[es6] Tail calls support.

[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen, Crankshaft and Turbofan.
When debugger is active tail calls are disabled.

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
TBR=rossberg@chromium.org

Committed: https://crrev.com/6131ab1edd6e78be01ac90b8f0b0f4f27f308071
Cr-Commit-Position: refs/heads/master@{#33509}

[Benedikt Meurer, 2016-01-21 05:20:48]

bmeurer@chromium.org changed reviewers:
+ bmeurer@chromium.org

[Benedikt Meurer, 2016-01-21 05:20:49]

Awesome Igor! I love the idea of putting that into the Call builtin.
Not much to add to this, except one nit and the debugger/devtools thing.

https://codereview.chromium.org/1609893003/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/1609893003/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:2145: __ DropCurrentJSFrame(rax, rbx, rcx, r8);
We need to check whether the debugger is active, and if so don't eliminate the
frame(s), but do a normal call. See Yang's tail call+debugger document for
background.

https://codereview.chromium.org/1609893003/diff/1/src/x64/macro-assembler-x64.h
File src/x64/macro-assembler-x64.h (right):

https://codereview.chromium.org/1609893003/diff/1/src/x64/macro-assembler-x64...
src/x64/macro-assembler-x64.h:368: void DropCurrentJSFrame(Register args,
Register scratch1, Register scratch2,
Nit: The name is a bit misleading, since this not only drops the "current JS
frame", but might also remove the arguments adaptor frame. I don't have a great
suggestion yet, tho.

[Igor Sheludko, 2016-01-21 12:48:00]

Description was changed from

==========
WIP: Tail calls support.

TailCallMode enum moved to globals.h


Adding --harmony-tailcalls flag

BUG=
==========

to

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
==========

[Igor Sheludko, 2016-01-21 14:29:28]

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

[commit-bot: I haz the power, 2016-01-21 14:29:40]

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

[commit-bot: I haz the power, 2016-01-21 14:34:28]

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

[commit-bot: I haz the power, 2016-01-21 14:34:29]

Dry run: Try jobs failed on following builders:
  v8_win_compile_dbg on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/1...)
  v8_win_nosnap_shared_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_compil...)

[Igor Sheludko, 2016-01-21 14:36:09]

Description was changed from

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
==========

to

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
==========

[Igor Sheludko, 2016-01-21 14:36:10]

Description was changed from

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
==========

to

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
==========

[Igor Sheludko, 2016-01-21 14:37:08]

PTAL. I think x64 is now complete. Starting ports.

https://codereview.chromium.org/1609893003/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/1609893003/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:2145: __ DropCurrentJSFrame(rax, rbx, rcx, r8);
On 2016/01/21 05:20:49, Benedikt Meurer wrote:
> We need to check whether the debugger is active, and if so don't eliminate the
> frame(s), but do a normal call. See Yang's tail call+debugger document for
> background.

Done.

https://codereview.chromium.org/1609893003/diff/1/src/x64/macro-assembler-x64.h
File src/x64/macro-assembler-x64.h (right):

https://codereview.chromium.org/1609893003/diff/1/src/x64/macro-assembler-x64...
src/x64/macro-assembler-x64.h:368: void DropCurrentJSFrame(Register args,
Register scratch1, Register scratch2,
On 2016/01/21 05:20:49, Benedikt Meurer wrote:
> Nit: The name is a bit misleading, since this not only drops the "current JS
> frame", but might also remove the arguments adaptor frame. I don't have a
great
> suggestion yet, tho.

PrepareForTailCall is a better name. Done.

[Benedikt Meurer, 2016-01-22 05:23:12]

https://codereview.chromium.org/1609893003/diff/40001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/1609893003/diff/40001/src/x64/builtins-x64.cc...
src/x64/builtins-x64.cc:2372: void
Builtins::Generate_CallBoundFunctionImpl(MacroAssembler* masm,
Nit: No need to add Impl suffix here.

https://codereview.chromium.org/1609893003/diff/40001/src/x64/builtins-x64.cc...
src/x64/builtins-x64.cc:2380: if (tail_call_mode == TailCallMode::kAllow) {
You don't need to do this here. Just pick the right Call builtin at the end,
depending on tail_call_mode, which will then deconstruct the frame properly,
i.e.

__ Load(rcx, ExternalReference(tail_call_mode == TailCallMode::kAllow ?
Builtins::kTailCall_ReceiverIsAny : Builtins::kCall_ReceiverIsAny,
masm->isolate()));

should do the trick.

[Igor Sheludko, 2016-01-22 09:48:51]

Thanks!

https://codereview.chromium.org/1609893003/diff/40001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/1609893003/diff/40001/src/x64/builtins-x64.cc...
src/x64/builtins-x64.cc:2372: void
Builtins::Generate_CallBoundFunctionImpl(MacroAssembler* masm,
On 2016/01/22 05:23:12, Benedikt Meurer wrote:
> Nit: No need to add Impl suffix here.

We have to use a different name here otherwise the compiler will complain about
ambiguity of the &Generate_CallBoundFunction that we have in our macros.

https://codereview.chromium.org/1609893003/diff/40001/src/x64/builtins-x64.cc...
src/x64/builtins-x64.cc:2380: if (tail_call_mode == TailCallMode::kAllow) {
On 2016/01/22 05:23:12, Benedikt Meurer wrote:
> You don't need to do this here. Just pick the right Call builtin at the end,
> depending on tail_call_mode, which will then deconstruct the frame properly,
> i.e.
> 
> __ Load(rcx, ExternalReference(tail_call_mode == TailCallMode::kAllow ?
> Builtins::kTailCall_ReceiverIsAny : Builtins::kCall_ReceiverIsAny,
> masm->isolate()));
> 
> should do the trick.

If we do like you suggested that would mean that we will copy the bound
arguments twice. Probably we don't want that.

[Benedikt Meurer, 2016-01-22 10:14:45]

I see. I'm fine with this approach as baseline. Let's see how the other
architectures deal with tail calls.

LGTM on the x64 port.

[Igor Sheludko, 2016-01-25 17:37:04]

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

[commit-bot: I haz the power, 2016-01-25 17:37:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1609893003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1609893003/80001

[commit-bot: I haz the power, 2016-01-25 17:40:33]

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

[commit-bot: I haz the power, 2016-01-25 17:40:33]

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

[Igor Sheludko, 2016-01-25 21:55:15]

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

[commit-bot: I haz the power, 2016-01-25 21:55:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1609893003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1609893003/100001

[commit-bot: I haz the power, 2016-01-25 22:22:38]

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

[commit-bot: I haz the power, 2016-01-25 22:22:39]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-01-25 22:26:16]

All the other ports are done. PTAL.

[Benedikt Meurer, 2016-01-26 10:05:06]

Still LGTM :-)

[Igor Sheludko, 2016-01-26 10:13:06]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2016-01-26 10:13:20]

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

[commit-bot: I haz the power, 2016-01-26 10:21:02]

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

[commit-bot: I haz the power, 2016-01-26 10:21:02]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/10144)

[Igor Sheludko, 2016-01-26 10:47:49]

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

[Igor Sheludko, 2016-01-26 10:50:43]

Andreas, PTAL parsing part.

[Igor Sheludko, 2016-01-26 11:05:06]

Description was changed from

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
==========

to

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
TBR=rossberg@chromium.org
==========

[Igor Sheludko, 2016-01-26 11:05:26]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2016-01-26 11:05:33]

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

[commit-bot: I haz the power, 2016-01-26 11:07:22]

Description was changed from

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
TBR=rossberg@chromium.org
==========

to

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
TBR=rossberg@chromium.org
==========

[commit-bot: I haz the power, 2016-01-26 11:07:23]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-01-26 11:07:50]

Description was changed from

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled. 

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
TBR=rossberg@chromium.org
==========

to

==========
[es6] Tail calls support.

This CL implements PrepareForTailCall() mentioned in ES6 spec for full codegen,
Crankshaft and Turbofan.
When debugger is active tail calls are disabled.

Tail calling can be enabled by --harmony-tailcalls flag.

BUG=v8:4698
LOG=Y
TBR=rossberg@chromium.org

Committed: https://crrev.com/6131ab1edd6e78be01ac90b8f0b0f4f27f308071
Cr-Commit-Position: refs/heads/master@{#33509}
==========

[commit-bot: I haz the power, 2016-01-26 11:07:51]

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