[turbofan]: Convert StringFromCharCode to var-args style TF builtin

[turbofan]: Convert StringFromCharCode to var-args style TF builtin

Committed: https://crrev.com/57f8e38ecb786caba82a1c7f46b3a4cf2bb687cf
Cr-Commit-Position: refs/heads/master@{#40814}

[danno, 2016-10-28 20:25:39]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 20:25:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 20:30:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 20:30:21]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)

[danno, 2016-10-28 20:53:38]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 20:53:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 20:59:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 20:59:51]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[danno, 2016-10-30 21:47:19]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-30 21:47:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-30 21:54:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-30 21:54:18]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[danno, 2016-11-03 10:08:22]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 10:08:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 10:35:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 10:35:46]

Dry run: This issue passed the CQ dry run.

[danno, 2016-11-03 13:02:20]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 13:02:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-03 13:28:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-03 13:28:33]

Dry run: This issue passed the CQ dry run.

[danno, 2016-11-03 13:29:54]

danno@chromium.org changed reviewers:
+ bmeurer@chromium.org, ishell@chromium.org

[danno, 2016-11-03 13:29:55]

PTAL

[Igor Sheludko, 2016-11-04 12:55:58]

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
File src/builtins/builtins-string.cc (left):

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:441:
assembler->SmiConstant(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR))),
Wow! What a hack! :)

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
File src/builtins/builtins-string.cc (right):

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:459: Variable max_index(assembler,
MachineRepresentation::kWord32);
Why is max_index not of an intptr_t type? I suppose we don't want to generate
sign-extension instructions.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:463: // codes. Stope if any of the conversions
generates a code that doesn't fit
s/Stope/Stop/

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:481: CodeStubAssembler::INTPTR_PARAMETERS,
Parameter mode here is not in sync with max_index definition.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:499: Node* zero = assembler->Int32Constant(0);
Not an intptr_t zero.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:503: CodeStubAssembler::INTPTR_PARAMETERS);
Same here.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:508: CodeStubAssembler::INTPTR_PARAMETERS,
And here.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:528: CodeStubAssembler::INTPTR_PARAMETERS,
Same here.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:535: max_index.value());
Same here (default parameter mode is INTPTR_PARAMETERS).

[Benedikt Meurer, 2016-11-07 06:36:34]

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins....
src/builtins/builtins.h:624: TFJ(StringFromCharCode, -1)                        
                        \
Can we use SharedFunctionInfo::kDontAdaptArgumentsSentinel here to avoid the
magic -1 here?

[danno, 2016-11-07 16:26:27]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-07 16:26:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[danno, 2016-11-07 16:43:52]

Feedback addressed. Please take another look.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
File src/builtins/builtins-string.cc (left):

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:441:
assembler->SmiConstant(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR))),
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Wow! What a hack! :)

Acknowledged.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
File src/builtins/builtins-string.cc (right):

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:459: Variable max_index(assembler,
MachineRepresentation::kWord32);
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Why is max_index not of an intptr_t type? I suppose we don't want to generate
> sign-extension instructions.

Done. Made it an IntPtr.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:463: // codes. Stope if any of the conversions
generates a code that doesn't fit
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> s/Stope/Stop/

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:481: CodeStubAssembler::INTPTR_PARAMETERS,
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Parameter mode here is not in sync with max_index definition.

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:499: Node* zero = assembler->Int32Constant(0);
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Not an intptr_t zero.

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:503: CodeStubAssembler::INTPTR_PARAMETERS);
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Same here.

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:508: CodeStubAssembler::INTPTR_PARAMETERS,
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> And here.

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:528: CodeStubAssembler::INTPTR_PARAMETERS,
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Same here.

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins-...
src/builtins/builtins-string.cc:535: max_index.value());
On 2016/11/04 12:55:58, Igor Sheludko wrote:
> Same here (default parameter mode is INTPTR_PARAMETERS).

Done.

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2448993002/diff/200001/src/builtins/builtins....
src/builtins/builtins.h:624: TFJ(StringFromCharCode, -1)                        
                        \
On 2016/11/07 06:36:34, Benedikt Meurer wrote:
> Can we use SharedFunctionInfo::kDontAdaptArgumentsSentinel here to avoid the
> magic -1 here?

Done.

[danno, 2016-11-07 16:46:27]

The CQ bit was checked by danno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-07 16:46:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 17:20:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-07 17:20:07]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-11-07 17:28:18]

lgtm

[Benedikt Meurer, 2016-11-07 18:35:26]

lgtm

[danno, 2016-11-07 21:23:32]

The CQ bit was checked by danno@chromium.org

[commit-bot: I haz the power, 2016-11-07 21:23:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-07 21:26:25]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-11-17 22:25:11]

Description was changed from

==========
[turbofan]: Convert StringFromCharCode to var-args style TF builtin
==========

to

==========
[turbofan]: Convert StringFromCharCode to var-args style TF builtin

Committed: https://crrev.com/57f8e38ecb786caba82a1c7f46b3a4cf2bb687cf
Cr-Commit-Position: refs/heads/master@{#40814}
==========

[commit-bot: I haz the power, 2016-11-17 22:25:15]

Patchset 13 (id:??) landed as
https://crrev.com/57f8e38ecb786caba82a1c7f46b3a4cf2bb687cf
Cr-Commit-Position: refs/heads/master@{#40814}