[stubs] Port String.prototype.substr to TurboFan

[stubs] Port String.prototype.substr to TurboFan

BUG=v8:5415
Committed: https://crrev.com/7d26871d2d0109393b8a53e1a239ca028fe6f0f2
Cr-Commit-Position: refs/heads/master@{#39951}

[jgruber, 2016-09-26 12:02:25]

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

[commit-bot: I haz the power, 2016-09-26 12:02:35]

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 12:46:14]

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

[commit-bot: I haz the power, 2016-09-26 12:50:52]

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

[commit-bot: I haz the power, 2016-09-26 12:50:53]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-09-27 12:15:06]

https://codereview.chromium.org/2373493002/diff/1/src/builtins/builtins-strin...
File src/builtins/builtins-string.cc (right):

https://codereview.chromium.org/2373493002/diff/1/src/builtins/builtins-strin...
src/builtins/builtins-string.cc:625: Node* result =
a->CallRuntime(Runtime::kThrowIllegalOperation, context);
String.prototype.substr should not throw. I guess it the start index happened to
be that big the substr will return empty string. But we can't return from here,
because we have to call ToNumber() on the length value to follow the spec.

[jgruber, 2016-09-29 07:47:33]

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

[commit-bot: I haz the power, 2016-09-29 07:47:35]

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-29 07:48:49]

https://codereview.chromium.org/2373493002/diff/1/src/builtins/builtins-strin...
File src/builtins/builtins-string.cc (right):

https://codereview.chromium.org/2373493002/diff/1/src/builtins/builtins-strin...
src/builtins/builtins-string.cc:625: Node* result =
a->CallRuntime(Runtime::kThrowIllegalOperation, context);
On 2016/09/27 12:15:06, Igor Sheludko wrote:
> String.prototype.substr should not throw. I guess it the start index happened
to
> be that big the substr will return empty string. But we can't return from
here,
> because we have to call ToNumber() on the length value to follow the spec.

Thanks, good catch! Modified this to set var_start to string.length, which ends
up returning "" after the required calls to ToNumber.

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

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

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

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-09-29 10:59:45]

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

https://codereview.chromium.org/2373493002/diff/20001/src/builtins/builtins-s...
src/builtins/builtins-string.cc:918: var_start.Bind(string_length);
I think if the start index is < 0 then we should bind var_start to 0, like you
already do for the smi case.

https://codereview.chromium.org/2373493002/diff/20001/src/builtins/builtins-s...
src/builtins/builtins-string.cc:930:
var_length.Bind(a->Select(a->WordEqual(length, a->UndefinedConstant()),
Please check that we don't execute ToInteger(length) code in case of undefined
length in generated code.

[jgruber, 2016-09-29 12:12:39]

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

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

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-29 12:13:44]

Arrgh! Sorry about this, and thanks for being so thorough. Comments addressed,
and added a couple of tests for these edge cases. ptal.

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

https://codereview.chromium.org/2373493002/diff/20001/src/builtins/builtins-s...
src/builtins/builtins-string.cc:918: var_start.Bind(string_length);
On 2016/09/29 10:59:44, Igor Sheludko wrote:
> I think if the start index is < 0 then we should bind var_start to 0, like you
> already do for the smi case.

Done.

https://codereview.chromium.org/2373493002/diff/20001/src/builtins/builtins-s...
src/builtins/builtins-string.cc:930:
var_length.Bind(a->Select(a->WordEqual(length, a->UndefinedConstant()),
On 2016/09/29 10:59:44, Igor Sheludko wrote:
> Please check that we don't execute ToInteger(length) code in case of undefined
> length in generated code.

Done.

[jgruber, 2016-09-29 12:19:46]

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

[commit-bot: I haz the power, 2016-09-29 12:19:59]

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-29 12:48:33]

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

[commit-bot: I haz the power, 2016-09-29 12:48:34]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-10-04 11:30:28]

lgtm

[jgruber, 2016-10-04 11:30:59]

The CQ bit was checked by jgruber@chromium.org

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

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-04 12:00:21]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-04 12:00:37]

Description was changed from

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

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

to

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

BUG=v8:5415

Committed: https://crrev.com/7d26871d2d0109393b8a53e1a239ca028fe6f0f2
Cr-Commit-Position: refs/heads/master@{#39951}
==========

[commit-bot: I haz the power, 2016-10-04 12:00:38]

Patchset 4 (id:??) landed as
https://crrev.com/7d26871d2d0109393b8a53e1a239ca028fe6f0f2
Cr-Commit-Position: refs/heads/master@{#39951}