[stubs] Port String.prototype.substring to TurboFan

[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415
Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Committed: https://crrev.com/e3222de330a854e153a7d4fa5020b443e9c0ac89
Cr-Original-Commit-Position: refs/heads/master@{#39717}
Cr-Commit-Position: refs/heads/master@{#39852}

[jgruber, 2016-09-23 14:06:03]

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

[commit-bot: I haz the power, 2016-09-23 14:06:19]

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

[jgruber, 2016-09-23 14:09:07]

jgruber@chromium.org changed reviewers:
+ ishell@chromium.org

[jgruber, 2016-09-23 14:09:08]

https://codereview.chromium.org/2358133004/diff/1/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2358133004/diff/1/src/builtins/builtins.h#new...
src/builtins/builtins.h:549: TFJ(StringPrototypeSubstring, 3)                   
                        \
Out of curiosity, why are we passing 'real_argc + 1' here?

[commit-bot: I haz the power, 2016-09-23 14:12:29]

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

[commit-bot: I haz the power, 2016-09-23 14:12:29]

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...)

[jgruber, 2016-09-26 08:00:47]

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

[commit-bot: I haz the power, 2016-09-26 08:00:56]

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-09-26 08:06:18]

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

[commit-bot: I haz the power, 2016-09-26 08:06: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...)

[jgruber, 2016-09-26 08:19:42]

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

[commit-bot: I haz the power, 2016-09-26 08:19:48]

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

[jgruber, 2016-09-26 08:37:13]

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

[commit-bot: I haz the power, 2016-09-26 08:37:15]

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-09-26 09:00:24]

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

[commit-bot: I haz the power, 2016-09-26 09:00:25]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-09-26 09:27:59]

lgtm

https://codereview.chromium.org/2358133004/diff/60001/src/builtins/builtins-s...
File src/builtins/builtins-string.cc (right):

https://codereview.chromium.org/2358133004/diff/60001/src/builtins/builtins-s...
src/builtins/builtins-string.cc:491: a->Branch(a->WordIsSmi(value_int),
&if_issmi, &if_isnotsmi);
I think we can avoid this second smi check here and in all the other places.
Just make the ToNumber() produce two outputs: if_smi and if_heapnumber.
See CSA::TryLookupProperty() that follows this approach already.

Maybe address that in a separate CL.

[jgruber, 2016-09-26 11:42:59]

Thanks for the review! I still renamed ToNumberBetweenZeroAnd to
ToSmiBetweenZeroAnd since that's what it actually does.

https://codereview.chromium.org/2358133004/diff/60001/src/builtins/builtins-s...
File src/builtins/builtins-string.cc (right):

https://codereview.chromium.org/2358133004/diff/60001/src/builtins/builtins-s...
src/builtins/builtins-string.cc:491: a->Branch(a->WordIsSmi(value_int),
&if_issmi, &if_isnotsmi);
On 2016/09/26 09:27:59, Igor Sheludko wrote:
> I think we can avoid this second smi check here and in all the other places.
> Just make the ToNumber() produce two outputs: if_smi and if_heapnumber.
> See CSA::TryLookupProperty() that follows this approach already.
> 
> Maybe address that in a separate CL.

Good idea, seems like every callsite of ToInteger immediately performs a smi
check.

[jgruber, 2016-09-26 11:44:00]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-09-26 11:44:00]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org
Link to the patchset: https://codereview.chromium.org/2358133004/#ps80001
(title: "Rename to ToSmiBetweenZeroAnd")

[commit-bot: I haz the power, 2016-09-26 11:44:07]

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-09-26 12:14:30]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-26 12:14:43]

Description was changed from

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415
==========

to

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}
==========

[commit-bot: I haz the power, 2016-09-26 12:14:44]

Patchset 5 (id:??) landed as
https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}

[Michael Hablich, 2016-09-26 13:16:02]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2369003002/ by hablich@chromium.org.

The reason for reverting is: Blocks roll:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/bui....

[jgruber, 2016-09-27 13:27:10]

Description was changed from

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}
==========

to

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}
==========

[jgruber, 2016-09-29 07:45:44]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2016-09-29 07:45:45]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org
Link to the patchset: https://codereview.chromium.org/2358133004/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-29 07:45:56]

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-09-29 08:16:08]

Description was changed from

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}
==========

to

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}
==========

[commit-bot: I haz the power, 2016-09-29 08:16:09]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-29 08:16:22]

Description was changed from

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Cr-Commit-Position: refs/heads/master@{#39717}
==========

to

==========
[stubs] Port String.prototype.substring to TurboFan

BUG=v8:5415

Committed: https://crrev.com/cc37dff7ba21345b3a867a86127a208e34a3f707
Committed: https://crrev.com/e3222de330a854e153a7d4fa5020b443e9c0ac89
Cr-Original-Commit-Position: refs/heads/master@{#39717}
Cr-Commit-Position: refs/heads/master@{#39852}
==========

[commit-bot: I haz the power, 2016-09-29 08:16:23]

Patchset 6 (id:??) landed as
https://crrev.com/e3222de330a854e153a7d4fa5020b443e9c0ac89
Cr-Commit-Position: refs/heads/master@{#39852}