[regexp] Port RegExpExecStub to CSA (mostly)

[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

Exit frame construction and the final call remain in RegExpExecStub.

BUG=v8:5339, v8:592
Review-Url: https://codereview.chromium.org/2738413002
Cr-Commit-Position: refs/heads/master@{#43844}
Committed: https://chromium.googlesource.com/v8/v8/+/5cc6189677066382dd206a68375b88e80ccd9afd

[jgruber, 2017-03-10 11:22:08]

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

[commit-bot: I haz the power, 2017-03-10 11:22:15]

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

[jgruber, 2017-03-10 11:25:22]

Description was changed from

==========
[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

BUG=v8:5339,v8:592
==========

to

==========
[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

Exit frame construction and the final call remain in RegExpExecStub.

BUG=v8:5339,v8:592
==========

[commit-bot: I haz the power, 2017-03-10 11:25:25]

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

[commit-bot: I haz the power, 2017-03-10 11:25:26]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/22303)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/3...)
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23959)

[jgruber, 2017-03-10 16:02:04]

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

[commit-bot: I haz the power, 2017-03-10 16:02:10]

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, 2017-03-10 16:05:17]

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

[commit-bot: I haz the power, 2017-03-10 16:05:18]

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/...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/22326)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/18781)

[jgruber, 2017-03-11 09:37:48]

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

[commit-bot: I haz the power, 2017-03-11 09:37:58]

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, 2017-03-11 09:40:48]

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

[commit-bot: I haz the power, 2017-03-11 09:40:49]

Dry run: Try jobs failed on following builders:
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/22239)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/24006)

[jgruber, 2017-03-11 10:03:40]

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

[commit-bot: I haz the power, 2017-03-11 10:03: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, 2017-03-11 10:06:01]

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

[commit-bot: I haz the power, 2017-03-11 10:06:02]

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

[jgruber, 2017-03-11 10:59:41]

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

[commit-bot: I haz the power, 2017-03-11 10:59:51]

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, 2017-03-11 11:02:11]

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

[commit-bot: I haz the power, 2017-03-11 11:02:12]

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

[jgruber, 2017-03-12 18:20:06]

jgruber@chromium.org changed reviewers:
+ bmeurer@chromium.org, yangguo@chromium.org

[jgruber, 2017-03-12 18:20:07]

I still need to do some cleanups and port remaining architectures, but if you're
interested in a preview PTAL.

[jgruber, 2017-03-13 07:49:41]

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

[commit-bot: I haz the power, 2017-03-13 07:49:48]

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, 2017-03-13 07:55:37]

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

[commit-bot: I haz the power, 2017-03-13 07:55:38]

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

[jgruber, 2017-03-13 08:19:45]

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

[commit-bot: I haz the power, 2017-03-13 08:19:52]

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, 2017-03-13 08:24:01]

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

[commit-bot: I haz the power, 2017-03-13 08:24:02]

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

[jgruber, 2017-03-13 08:40:55]

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

[commit-bot: I haz the power, 2017-03-13 08:40:57]

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, 2017-03-13 09:10:37]

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

[commit-bot: I haz the power, 2017-03-13 09:10:38]

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

[jgruber, 2017-03-13 11:57:54]

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

[commit-bot: I haz the power, 2017-03-13 11:57: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, 2017-03-13 12:19:55]

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

[commit-bot: I haz the power, 2017-03-13 12:19:56]

Dry run: This issue passed the CQ dry run.

[jgruber, 2017-03-13 12:27:38]

Ready to go. Some self-review comments inline.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
File src/builtins/builtins-regexp.cc (right):

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:593: match_indices = IrregexpExec(context,
regexp, string, var_lastindex.value(),
This will add even more to the snapshot size. It's probably time to convert some
inlined code into stub calls.

https://codereview.chromium.org/2738413002/diff/160001/src/code-stub-assemble...
File src/code-stub-assembler.cc (right):

https://codereview.chromium.org/2738413002/diff/160001/src/code-stub-assemble...
src/code-stub-assembler.cc:3281:
a->Branch(a->IsOneByteStringInstanceType(from_instance_type),
Drive-by simplification.

https://codereview.chromium.org/2738413002/diff/160001/src/code-stub-assemble...
src/code-stub-assembler.cc:3570: void
CodeStubAssembler::TryUnpackString(Variable* var_string,
String unpacking is getting messy, planning on refactoring this (and other use
sites) soon.

https://codereview.chromium.org/2738413002/diff/160001/src/code-stub-assemble...
src/code-stub-assembler.cc:5712: return Select(TaggedIsSmi(receiver),
Drive-by simplification.

[jgruber, 2017-03-14 08:59:21]

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

[Benedikt Meurer, 2017-03-14 10:46:07]

bmeurer@chromium.org changed required reviewers:
+ ishell@chromium.org, yangguo@chromium.org

[Benedikt Meurer, 2017-03-14 10:46:08]

LGTM from my side on the approach, haven't checked the registers in detail. :-)

[Igor Sheludko, 2017-03-15 00:39:04]

Nice cleanup! Looking good!

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
File src/builtins/builtins-regexp.cc (right):

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:388: Callable exec_callable =
CodeFactory::RegExpExec(isolate());
I'd rather name the stub and this function the other way round. What's left in
the stub is actually IrregexpExecStub (or CallIrrexpStub or IrregexpEntryStub)
while this whole function is what used to be RegExpExec code.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:394: // We expect exactly one result since we
force the called regexp to behave
s/we force/the stub forces/ or maybe add a comment above the call that the stub
internally enforces the lookup to be non-global.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:416: LoadFixedArrayElement(data,
JSRegExp::kIrregexpCaptureCountIndex);
Maybe
// Calculate number of capture registers (number_of_captures + 1) * 2

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:444: [this, static_offsets_vector_address,
match_info,
[=, &var_to_offset]

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:471: // If there is no pending exception, a
Please reformat the comment.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:475: Branch(IsTheHole(pending_exception),
&stack_overflow, &rethrow);
AFAICT the only non-gc-safe thing the Irregexp code does when it's called from
JS (is_direct = true) is calling a
NativeRegExpMacroAssembler::CheckStackGuardState() which in turn can only create
and throw a stack overflow exception (in direct mode). So I guess if you remove
the isolate->StackOverflow(); from CheckStackGuardState() and let this code
actually reach the &stack_overflow block then you can probably skip the exit
frame construction and call the Irregexp code directly from CSA as a C function.
And this hole check will then become an assert and &rethow case will not be
necessary anymore.
WDYT?

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:593: match_indices = IrregexpExec(context,
regexp, string, var_lastindex.value(),
On 2017/03/13 12:27:37, jgruber wrote:
> This will add even more to the snapshot size. It's probably time to convert
some
> inlined code into stub calls.

Why not leave this code in the stub in this CL?

https://codereview.chromium.org/2738413002/diff/160001/src/interface-descript...
File src/interface-descriptors.h (right):

https://codereview.chromium.org/2738413002/diff/160001/src/interface-descript...
src/interface-descriptors.h:648: class RegExpExecDescriptor : public
CallInterfaceDescriptor {
IrregexpExecDescriptor or something...

https://codereview.chromium.org/2738413002/diff/160001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (left):

https://codereview.chromium.org/2738413002/diff/160001/src/x64/code-stubs-x64...
src/x64/code-stubs-x64.cc:406: __ SmiToInteger32(rbx, FieldOperand(rax,
JSRegExp::kDataTagOffset));
Please also delete these unused offset constants from JSRegExp.

[jgruber, 2017-03-15 11:24:14]

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

[commit-bot: I haz the power, 2017-03-15 11:24:17]

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, 2017-03-15 11:27:07]

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

[commit-bot: I haz the power, 2017-03-15 11:27:08]

Dry run: Try jobs failed on following builders:
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/24242)

[jgruber, 2017-03-15 11:45:42]

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

[commit-bot: I haz the power, 2017-03-15 11:45:55]

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, 2017-03-15 11:49:24]

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

[commit-bot: I haz the power, 2017-03-15 11:49:25]

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

[jgruber, 2017-03-15 12:03:38]

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

[commit-bot: I haz the power, 2017-03-15 12:03:40]

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, 2017-03-15 12:08:30]

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

[commit-bot: I haz the power, 2017-03-15 12:08:31]

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

[jgruber, 2017-03-15 12:33:48]

Description was changed from

==========
[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

Exit frame construction and the final call remain in RegExpExecStub.

BUG=v8:5339,v8:592
==========

to

==========
[regexp] Port RegExpExecStub to CSA

This moves logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

BUG=v8:5339,v8:592
==========

[Igor Sheludko, 2017-03-15 13:08:26]

lgtm for landing the cl without CallCFunction9 changes for now.

[jgruber, 2017-03-15 13:40:06]

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

[commit-bot: I haz the power, 2017-03-15 13:40:15]

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

[jgruber, 2017-03-15 13:43:45]

As discussed offline, landing the partial port now so I can move ahead with
follow-up CLs.

The complete stub-removal is still blocked on arm/arm64 issues
(crbug.com/v8/6102). Will land that once those are solved.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
File src/builtins/builtins-regexp.cc (right):

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:388: Callable exec_callable =
CodeFactory::RegExpExec(isolate());
On 2017/03/15 00:39:04, Igor Sheludko wrote:
> I'd rather name the stub and this function the other way round. What's left in
> the stub is actually IrregexpExecStub (or CallIrrexpStub or IrregexpEntryStub)
> while this whole function is what used to be RegExpExec code.

Renamed the function RegExpExecInternal. The stub will be removed once issues on
arm/arm64 are fixed.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:394: // We expect exactly one result since we
force the called regexp to behave
On 2017/03/15 00:39:03, Igor Sheludko wrote:
> s/we force/the stub forces/ or maybe add a comment above the call that the
stub
> internally enforces the lookup to be non-global.

Done.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:416: LoadFixedArrayElement(data,
JSRegExp::kIrregexpCaptureCountIndex);
On 2017/03/15 00:39:04, Igor Sheludko wrote:
> Maybe
> // Calculate number of capture registers (number_of_captures + 1) * 2

Done.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:444: [this, static_offsets_vector_address,
match_info,
On 2017/03/15 00:39:04, Igor Sheludko wrote:
> [=, &var_to_offset]

Done.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:471: // If there is no pending exception, a
On 2017/03/15 00:39:03, Igor Sheludko wrote:
> Please reformat the comment.

Done.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:475: Branch(IsTheHole(pending_exception),
&stack_overflow, &rethrow);
On 2017/03/15 00:39:03, Igor Sheludko wrote:
> AFAICT the only non-gc-safe thing the Irregexp code does when it's called from
> JS (is_direct = true) is calling a
> NativeRegExpMacroAssembler::CheckStackGuardState() which in turn can only
create
> and throw a stack overflow exception (in direct mode). So I guess if you
remove
> the isolate->StackOverflow(); from CheckStackGuardState() and let this code
> actually reach the &stack_overflow block then you can probably skip the exit
> frame construction and call the Irregexp code directly from CSA as a C
function.
> And this hole check will then become an assert and &rethow case will not be
> necessary anymore.
> WDYT?

Great find. The initial goal was to completely remove the stub, awesome that we
can accomplish that. We're still blocked by issues on arm/arm64, but I'll remove
the stub once that's done.

https://codereview.chromium.org/2738413002/diff/160001/src/builtins/builtins-...
src/builtins/builtins-regexp.cc:593: match_indices = IrregexpExec(context,
regexp, string, var_lastindex.value(),
On 2017/03/15 00:39:03, Igor Sheludko wrote:
> On 2017/03/13 12:27:37, jgruber wrote:
> > This will add even more to the snapshot size. It's probably time to convert
> some
> > inlined code into stub calls.
> 
> Why not leave this code in the stub in this CL?

I have a follow-up CL that sets up a couple more stub calls for slow paths,
reducing snapshot size overall.

https://codereview.chromium.org/2738413002/diff/160001/src/interface-descript...
File src/interface-descriptors.h (right):

https://codereview.chromium.org/2738413002/diff/160001/src/interface-descript...
src/interface-descriptors.h:648: class RegExpExecDescriptor : public
CallInterfaceDescriptor {
On 2017/03/15 00:39:04, Igor Sheludko wrote:
> IrregexpExecDescriptor or something...

Left as-is for now since we'll remove this soon.

https://codereview.chromium.org/2738413002/diff/160001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (left):

https://codereview.chromium.org/2738413002/diff/160001/src/x64/code-stubs-x64...
src/x64/code-stubs-x64.cc:406: __ SmiToInteger32(rbx, FieldOperand(rax,
JSRegExp::kDataTagOffset));
On 2017/03/15 00:39:04, Igor Sheludko wrote:
> Please also delete these unused offset constants from JSRegExp.

Done.

[commit-bot: I haz the power, 2017-03-15 13:45:07]

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

[commit-bot: I haz the power, 2017-03-15 13:45:08]

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

[jgruber, 2017-03-15 13:48:42]

Description was changed from

==========
[regexp] Port RegExpExecStub to CSA

This moves logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

BUG=v8:5339,v8:592
==========

to

==========
[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

Exit frame construction and the final call remain in RegExpExecStub.

BUG=v8:5339,v8:592
==========

[jgruber, 2017-03-15 14:26:37]

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

[commit-bot: I haz the power, 2017-03-15 14:26:42]

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, 2017-03-15 14:28:15]

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

[commit-bot: I haz the power, 2017-03-15 14:28:17]

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/...)
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  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...)
  v8_linux64_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_verify_csa_rel_n...)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/22615)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)
  v8_linux_verify_csa_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_verify_csa_rel_ng/...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/36741)

[jgruber, 2017-03-15 14:30:41]

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

[commit-bot: I haz the power, 2017-03-15 14:30:53]

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, 2017-03-15 15:16:54]

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

[commit-bot: I haz the power, 2017-03-15 15:16:55]

Dry run: This issue passed the CQ dry run.

[jgruber, 2017-03-15 15:20:02]

The CQ bit was checked by jgruber@chromium.org

[jgruber, 2017-03-15 15:20:02]

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

[commit-bot: I haz the power, 2017-03-15 15:20:04]

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, 2017-03-15 15:20:06]

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

[commit-bot: I haz the power, 2017-03-15 15:20:08]

All required reviewers (with asterisk prefixes) have not yet approved this CL.

No L-G-T-M from a valid reviewer yet.
CQ run can only be started once the patch has received an L-G-T-M from a full
committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
full super star committer.
Committers are members of the group "project-v8-committers".
Note that this has nothing to do with OWNERS files.

[Yang, 2017-03-16 07:38:41]

On 2017/03/15 15:20:08, commit-bot: I haz the power wrote:
> All required reviewers (with asterisk prefixes) have not yet approved this CL.
> 
> No L-G-T-M from a valid reviewer yet.
> CQ run can only be started once the patch has received an L-G-T-M from a full
> committer.
> Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a
> full super star committer.
> Committers are members of the group "project-v8-committers".
> Note that this has nothing to do with OWNERS files.

lgtm. Very nice!

[Igor Sheludko, 2017-03-16 07:40:47]

lgtm

[jgruber, 2017-03-16 07:53:06]

The CQ bit was checked by jgruber@chromium.org

[commit-bot: I haz the power, 2017-03-16 07:53:14]

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, 2017-03-16 07:54:55]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1489650786983520,
"parent_rev": "9224d5d1bc61c5054ff79b9c5c700052b85c56d3", "commit_rev":
"5cc6189677066382dd206a68375b88e80ccd9afd"}

[commit-bot: I haz the power, 2017-03-16 07:55:03]

Description was changed from

==========
[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

Exit frame construction and the final call remain in RegExpExecStub.

BUG=v8:5339,v8:592
==========

to

==========
[regexp] Port RegExpExecStub to CSA (mostly)

This moves most of the logic contained in RegExpExecStub to CSA.  Benefits are
mostly easier readability and hackability, and removal of a large chunk of
platform-specific assembly.

Exit frame construction and the final call remain in RegExpExecStub.

BUG=v8:5339,v8:592

Review-Url: https://codereview.chromium.org/2738413002
Cr-Commit-Position: refs/heads/master@{#43844}
Committed:
https://chromium.googlesource.com/v8/v8/+/5cc6189677066382dd206a68375b88e80cc...
==========

[commit-bot: I haz the power, 2017-03-16 07:55:05]

Committed patchset #15 (id:280001) as
https://chromium.googlesource.com/v8/v8/+/5cc6189677066382dd206a68375b88e80cc...