Add fast paths for native RegExps in ES2015 subclass-aware code

Add fast paths for native RegExps in ES2015 subclass-aware code

This patch adds fast paths for @@replace and @@split that call into
the old, side-effect free (and faster) versions of those builtin
operations when possible (roughly, when the receiver is a RegExp
and the 'exec' method is untampered). Also add some micro-optimizations
that further improve performance.

Taken altogether, this takes us from a ~70% regression on the Octane
RegExp benchmark to a ~9% regression.

The test262.status lines for RegExps has been reorganized to make it
clearer the status of individual tests. More work will likely be
required to increase spec compliance before the --harmony-regexp-exec
flag is shipped; some of that work is happening on the spec side.

BUG=v8:4602
LOG=y

Committed: https://crrev.com/360586f2f91973b8fc5a20dc1073a43acdce10a5
Cr-Commit-Position: refs/heads/master@{#35118}

[adamk, 2016-03-28 22:46:32]

adamk@chromium.org changed reviewers:
+ littledan@chromium.org, yangguo@chromium.org

[Dan Ehrenberg, 2016-03-28 22:56:09]

lgtm

+100 Nice clean change

https://codereview.chromium.org/1836123002/diff/1/src/js/harmony-unicode-rege...
File src/js/harmony-unicode-regexps.js (right):

https://codereview.chromium.org/1836123002/diff/1/src/js/harmony-unicode-rege...
src/js/harmony-unicode-regexps.js:34: return !!REGEXP_UNICODE(this);
While making a change here, maybe change to TO_BOOLEAN, here and in the other
flag getters?

https://codereview.chromium.org/1836123002/diff/1/src/js/regexp.js
File src/js/regexp.js (right):

https://codereview.chromium.org/1836123002/diff/1/src/js/regexp.js#newcode908
src/js/regexp.js:908: // on RegExp.prototype.
I believe there is also a shortcut with respect to lastIndex; yangguo@ could say
more.

[adamk, 2016-03-28 23:14:37]

https://codereview.chromium.org/1836123002/diff/1/src/js/harmony-unicode-rege...
File src/js/harmony-unicode-regexps.js (right):

https://codereview.chromium.org/1836123002/diff/1/src/js/harmony-unicode-rege...
src/js/harmony-unicode-regexps.js:34: return !!REGEXP_UNICODE(this);
On 2016/03/28 22:56:09, Dan Ehrenberg wrote:
> While making a change here, maybe change to TO_BOOLEAN, here and in the other
> flag getters?

Done.

https://codereview.chromium.org/1836123002/diff/1/src/js/regexp.js
File src/js/regexp.js (right):

https://codereview.chromium.org/1836123002/diff/1/src/js/regexp.js#newcode908
src/js/regexp.js:908: // on RegExp.prototype.
On 2016/03/28 22:56:09, Dan Ehrenberg wrote:
> I believe there is also a shortcut with respect to lastIndex; yangguo@ could
say
> more.

Added more to this comment.

[Dan Ehrenberg, 2016-03-28 23:22:02]

lgtm

[adamk, 2016-03-29 00:36:01]

Yang, would love another set of eyes on this in case you have concerns (in
particular, the lastIndex thing, or other bits you see missing that could make
the fast path more correct).

[Yang, 2016-03-29 12:19:04]

IIRC the existing implementation already violates the spec wrt updating
lastIndex inside exec when we go into a fast path. Not sure what we want to do
about that.

Rest LGTM, though I'm a bit skeptical about the generous use of force inline.

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js
File src/js/regexp.js (right):

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js#newcod...
src/js/regexp.js:886: %SetForceInlineFlag(SetAdvancedStringIndex);
I see this is only used as a helper. Can we write this as macro instead? That
way we don't have to allocate a function object and have it inlined in
non-optimized code as well.

Same goes for AdvanceStringIndex above.

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js#newcod...
src/js/regexp.js:1080: %SetForceInlineFlag(RegExpGetFlags);
I can see the need for individual flag getters below, if they are used in other
regexp functions. But do we need it for .flags() as well?

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js#newcod...
src/js/regexp.js:1125: %SetForceInlineFlag(RegExpGetMultiline);
Is this only for symmetry? I don't think any of the subclass functions use this.

[adamk, 2016-03-29 20:19:20]

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js
File src/js/regexp.js (right):

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js#newcod...
src/js/regexp.js:886: %SetForceInlineFlag(SetAdvancedStringIndex);
On 2016/03/29 12:19:04, Yang wrote:
> I see this is only used as a helper. Can we write this as macro instead? That
> way we don't have to allocate a function object and have it inlined in
> non-optimized code as well.
> 
> Same goes for AdvanceStringIndex above.

For this patch, it turns out we don't need this stuff anyway (since the fast
path never calls these functions). Macro-ifying them is going to be somewhat
ugly, but that can be handled later if need be.

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js#newcod...
src/js/regexp.js:1080: %SetForceInlineFlag(RegExpGetFlags);
On 2016/03/29 12:19:04, Yang wrote:
> I can see the need for individual flag getters below, if they are used in
other
> regexp functions. But do we need it for .flags() as well?

No, flags barely shows up on the profile, these got added for symmetry. Happy to
remove them. I agree that I got a little bit over-zealous with the
%SetForceInlineFlag...

https://codereview.chromium.org/1836123002/diff/20001/src/js/regexp.js#newcod...
src/js/regexp.js:1125: %SetForceInlineFlag(RegExpGetMultiline);
On 2016/03/29 12:19:04, Yang wrote:
> Is this only for symmetry? I don't think any of the subclass functions use
this.

Removed, as noted. The only getters that still have them are "global", "sticky",
and "unicode".

[adamk, 2016-03-29 20:19:34]

The CQ bit was checked by adamk@chromium.org

[adamk, 2016-03-29 20:19:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1836123002/#ps40001
(title: "Less zealous inlining")

[commit-bot: I haz the power, 2016-03-29 20:19:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1836123002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1836123002/40001

[commit-bot: I haz the power, 2016-03-29 20:41:46]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-29 20:43:02]

Description was changed from

==========
Add fast paths for native RegExps in ES2015 subclass-aware code

This patch adds fast paths for @@replace and @@split that call into
the old, side-effect free (and faster) versions of those builtin
operations when possible (roughly, when the receiver is a RegExp
and the 'exec' method is untampered). Also add some micro-optimizations
that further improve performance.

Taken altogether, this takes us from a ~70% regression on the Octane
RegExp benchmark to a ~9% regression.

The test262.status lines for RegExps has been reorganized to make it
clearer the status of individual tests. More work will likely be
required to increase spec compliance before the --harmony-regexp-exec
flag is shipped; some of that work is happening on the spec side.

BUG=v8:4602
LOG=y
==========

to

==========
Add fast paths for native RegExps in ES2015 subclass-aware code

This patch adds fast paths for @@replace and @@split that call into
the old, side-effect free (and faster) versions of those builtin
operations when possible (roughly, when the receiver is a RegExp
and the 'exec' method is untampered). Also add some micro-optimizations
that further improve performance.

Taken altogether, this takes us from a ~70% regression on the Octane
RegExp benchmark to a ~9% regression.

The test262.status lines for RegExps has been reorganized to make it
clearer the status of individual tests. More work will likely be
required to increase spec compliance before the --harmony-regexp-exec
flag is shipped; some of that work is happening on the spec side.

BUG=v8:4602
LOG=y

Committed: https://crrev.com/360586f2f91973b8fc5a20dc1073a43acdce10a5
Cr-Commit-Position: refs/heads/master@{#35118}
==========

[commit-bot: I haz the power, 2016-03-29 20:43:03]

Patchset 3 (id:??) landed as
https://crrev.com/360586f2f91973b8fc5a20dc1073a43acdce10a5
Cr-Commit-Position: refs/heads/master@{#35118}