[Turbofan] Implement super calls with spread bytecode in assembly code.

[Turbofan] Implement super calls with spread bytecode in assembly code.

BUG=v8:5659
Review-Url: https://codereview.chromium.org/2571563004
Cr-Commit-Position: refs/heads/master@{#42444}
Committed: https://chromium.googlesource.com/v8/v8/+/0b8200c7664089b101fe4d1ec2695af885929dbe

[petermarshall, 2016-12-13 15:17:25]

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

[commit-bot: I haz the power, 2016-12-13 15:17:32]

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-12-13 15:21:22]

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

[commit-bot: I haz the power, 2016-12-13 15:21:23]

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_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  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/17718)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)

[petermarshall, 2016-12-14 13:20:51]

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

[commit-bot: I haz the power, 2016-12-14 13:20: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-12-14 13:25:06]

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

[commit-bot: I haz the power, 2016-12-14 13:25:07]

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_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_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/17754)

[petermarshall, 2016-12-14 13:59:44]

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

[commit-bot: I haz the power, 2016-12-14 13: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, 2016-12-14 14:02:59]

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

[commit-bot: I haz the power, 2016-12-14 14:03:00]

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_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)

[petermarshall, 2016-12-15 09:14:29]

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

[commit-bot: I haz the power, 2016-12-15 09:14:42]

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

[petermarshall, 2016-12-15 09:15:34]

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

[petermarshall, 2016-12-15 09:15:34]

I've just implemented this for x64 so far, could you PTAL before I go any
further =]

[commit-bot: I haz the power, 2016-12-15 09:18:27]

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

[commit-bot: I haz the power, 2016-12-15 09:18:28]

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_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)

[Benedikt Meurer, 2016-12-15 11:59:52]

Looks good as a first step.

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
File src/builtins/x64/builtins-x64.cc (right):

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:2896: Label target_not_constructor;
These checks must not be performed before the evaluation of the spread. So just
let the Construct builtin deal with this.

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:2904: Label new_target_not_constructor;
Same for new.target

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:2936: __ addp(rax, r9);
You can combine these addp and subp into a single leap

[petermarshall, 2016-12-15 13:36:10]

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

[commit-bot: I haz the power, 2016-12-15 13:36:20]

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

[petermarshall, 2016-12-15 13:37:27]

Thanks!

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
File src/builtins/x64/builtins-x64.cc (right):

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:2896: Label target_not_constructor;
On 2016/12/15 at 11:59:52, Benedikt Meurer wrote:
> These checks must not be performed before the evaluation of the spread. So
just let the Construct builtin deal with this.

Yep cool

https://codereview.chromium.org/2571563004/diff/120001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:2936: __ addp(rax, r9);
On 2016/12/15 at 11:59:52, Benedikt Meurer wrote:
> You can combine these addp and subp into a single leap

I always thought leap was a funny instruction name, it sounds like a jump
instruction to somewhere very far away =P. That objection aside, SGTM.

[commit-bot: I haz the power, 2016-12-15 13:39:50]

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

[commit-bot: I haz the power, 2016-12-15 13:39:51]

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_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/14242)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/19556)

[petermarshall, 2016-12-21 09:02:31]

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

[commit-bot: I haz the power, 2016-12-21 09:02:38]

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

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

[commit-bot: I haz the power, 2016-12-21 09:04:30]

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_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_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_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18212)
  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/18275)
  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_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/18185)
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/19839)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/19919)

[petermarshall, 2016-12-21 09:32:50]

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

[commit-bot: I haz the power, 2016-12-21 09:33:02]

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-12-21 09:37:09]

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

[commit-bot: I haz the power, 2016-12-21 09:37:10]

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

[petermarshall, 2016-12-21 10:26:13]

Benedikt, could you take a look at src/builtins/x64/builtins-x64.cc as a final
glance before I port over to other archs? Thanks =]

[Benedikt Meurer, 2016-12-21 11:51:34]

Looks good to me :-)

[petermarshall, 2016-12-21 14:57:06]

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

[commit-bot: I haz the power, 2016-12-21 14:57: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, 2016-12-21 15:01:23]

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

[commit-bot: I haz the power, 2016-12-21 15:01:24]

Dry run: Try jobs failed on following builders:
  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_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

[rmcilroy, 2017-01-03 11:28:58]

Description was changed from

==========
[Turbofan] Implement super calls with spread in x64.

BUG=v8:5659
==========

to

==========
[Turbofan] Implement super calls with spread in x64.

BUG=v8:5659
==========

[rmcilroy, 2017-01-03 11:28:58]

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

[rmcilroy, 2017-01-03 11:28:58]

rmcilroy@chromium.org changed required reviewers:
+ rmcilroy@chromium.org

[petermarshall, 2017-01-09 09:28:30]

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

[commit-bot: I haz the power, 2017-01-09 09:28: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-01-09 09:30:10]

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

[commit-bot: I haz the power, 2017-01-09 09:30:10]

Dry run: Try jobs failed on following builders:
  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_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_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_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18727)
  v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/18790)
  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_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_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/31814)
  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/20429)

[petermarshall, 2017-01-09 09:50:31]

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

[commit-bot: I haz the power, 2017-01-09 09:50:37]

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-01-09 09:54:26]

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

[commit-bot: I haz the power, 2017-01-09 09:54:27]

Dry run: Try jobs failed on following builders:
  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_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/15156)

[petermarshall, 2017-01-09 15:14:32]

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

[commit-bot: I haz the power, 2017-01-09 15:14:38]

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-01-09 15:18:28]

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

[commit-bot: I haz the power, 2017-01-09 15:18:29]

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

[petermarshall, 2017-01-11 10:41:02]

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

[commit-bot: I haz the power, 2017-01-11 10:41:13]

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-01-11 10:45:03]

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

[commit-bot: I haz the power, 2017-01-11 10:45:05]

Dry run: Try jobs failed on following builders:
  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_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...)

[petermarshall, 2017-01-11 10:52:58]

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

[commit-bot: I haz the power, 2017-01-11 10:53:04]

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-01-11 10:57:58]

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

[commit-bot: I haz the power, 2017-01-11 10:57:59]

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

[petermarshall, 2017-01-11 13:53:29]

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

[commit-bot: I haz the power, 2017-01-11 13:53:34]

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-01-11 13:59:42]

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

[commit-bot: I haz the power, 2017-01-11 13:59:43]

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

[petermarshall, 2017-01-11 14:38:49]

Description was changed from

==========
[Turbofan] Implement super calls with spread in x64.

BUG=v8:5659
==========

to

==========
[Turbofan] Implement super calls with spread bytecode in assembly code.

BUG=v8:5659
==========

[petermarshall, 2017-01-11 14:40:23]

mips-team, could you please port the builtins code. Thanks!

[petermarshall, 2017-01-11 15:06:31]

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

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

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

[commit-bot: I haz the power, 2017-01-11 15:16:09]

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

[rmcilroy, 2017-01-11 15:24:45]

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
File src/builtins/x64/builtins-x64.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:917: void
Builtins::Generate_InterpreterPushArgsAndConstructWithSpread(
High level question - this seems to be functionally identical to
Generate_InterpreterPushArgsAndConstructImpl other than the builtin being
called. Could we adapt that to be used for this (e.g., replace the CallableType
argument with another enum which differentiates between JSFunction / Any and
Spread ?  We could possibly also roll up InterpreterPushArgsAndConstructArray
into the same function (beyond the scope of this CL though). WDYT?

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/byteco...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/byteco...
src/interpreter/bytecode-array-builder.h:248: // spread.
Please update comment

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
File src/interpreter/interpreter-assembler.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter-assembler.cc:842: Comment("call using
ConstructWithSpread");
Do we have any plans to add type feedback information for ConstructWithSpread?
If so, would it be possible to share the code with CallConstruct when it's added
(passing an appropriate Callable for the PushArgs variant to call)?

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
File src/interpreter/interpreter-assembler.h (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter-assembler.h:145: compiler::Node*
CallConstructWithSpread(compiler::Node* constructor,
Please add a comment.

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter.cc:2161: // NewWithSpread <first_arg> <arg_count>
Please update the comment and description.

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter.cc:2168: // Callable ic =
CodeFactory::ConstructWithSpread(isolate_);
leftover code?

[petermarshall, 2017-01-11 16:44:52]

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

[commit-bot: I haz the power, 2017-01-11 16:45:01]

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

[petermarshall, 2017-01-11 16:50:03]

Thanks Ross.

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
File src/builtins/x64/builtins-x64.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:917: void
Builtins::Generate_InterpreterPushArgsAndConstructWithSpread(
On 2017/01/11 at 15:24:44, rmcilroy wrote:
> High level question - this seems to be functionally identical to
Generate_InterpreterPushArgsAndConstructImpl other than the builtin being
called. Could we adapt that to be used for this (e.g., replace the CallableType
argument with another enum which differentiates between JSFunction / Any and
Spread ?  We could possibly also roll up InterpreterPushArgsAndConstructArray
into the same function (beyond the scope of this CL though). WDYT?

Yes good point they are identical. I implemented this with a boolean flag as an
added parameter rather than modify the CallableType because there are other
places in the code that use CallableType as well. Open to other suggestions of
how to implement it, but it already looks a lot better than before.

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/byteco...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/byteco...
src/interpreter/bytecode-array-builder.h:248: // spread.
On 2017/01/11 at 15:24:45, rmcilroy wrote:
> Please update comment

Done

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
File src/interpreter/interpreter-assembler.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter-assembler.cc:842: Comment("call using
ConstructWithSpread");
On 2017/01/11 at 15:24:45, rmcilroy wrote:
> Do we have any plans to add type feedback information for ConstructWithSpread?
If so, would it be possible to share the code with CallConstruct when it's added
(passing an appropriate Callable for the PushArgs variant to call)?

Yes we do plan to add type feedback information in a future CL. We haven't
planned out how the type feedback will work in detail yet, but I suspect it will
be very similar if not exactly to same as CallConstruct so yes we can just share
code between the two in the future.

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
File src/interpreter/interpreter-assembler.h (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter-assembler.h:145: compiler::Node*
CallConstructWithSpread(compiler::Node* constructor,
On 2017/01/11 at 15:24:45, rmcilroy wrote:
> Please add a comment.

Done

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
File src/interpreter/interpreter.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/interpreter/interp...
src/interpreter/interpreter.cc:2168: // Callable ic =
CodeFactory::ConstructWithSpread(isolate_);
On 2017/01/11 at 15:24:45, rmcilroy wrote:
> leftover code?

Yeah not needed, there is a similar line left in Interpreter::DoNew but not as a
commment, which I'll submit another CL to remove, given that it doesn't do
anything.

Updated the comment here too.

[commit-bot: I haz the power, 2017-01-11 17:15:54]

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

[commit-bot: I haz the power, 2017-01-11 17:15:56]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-12 10:40:38]

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
File src/builtins/x64/builtins-x64.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:917: void
Builtins::Generate_InterpreterPushArgsAndConstructWithSpread(
On 2017/01/11 16:50:03, petermarshall wrote:
> On 2017/01/11 at 15:24:44, rmcilroy wrote:
> > High level question - this seems to be functionally identical to
> Generate_InterpreterPushArgsAndConstructImpl other than the builtin being
> called. Could we adapt that to be used for this (e.g., replace the
CallableType
> argument with another enum which differentiates between JSFunction / Any and
> Spread ?  We could possibly also roll up InterpreterPushArgsAndConstructArray
> into the same function (beyond the scope of this CL though). WDYT?
> 
> Yes good point they are identical. I implemented this with a boolean flag as
an
> added parameter rather than modify the CallableType because there are other
> places in the code that use CallableType as well. Open to other suggestions of
> how to implement it, but it already looks a lot better than before.

Right, I wasn't suggesting to modifiy CallableType, but to have a new enum type
which is only used by this function (which includes JSFunction / Any and Spread
options).

[petermarshall, 2017-01-12 12:03:08]

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

[commit-bot: I haz the power, 2017-01-12 12:03:21]

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-01-12 12:40:01]

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

[commit-bot: I haz the power, 2017-01-12 12:40:02]

Dry run: This issue passed the CQ dry run.

[petermarshall, 2017-01-13 09:34:51]

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

[commit-bot: I haz the power, 2017-01-13 09:34: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-01-13 10:05:45]

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

[commit-bot: I haz the power, 2017-01-13 10:05:46]

Dry run: This issue passed the CQ dry run.

[petermarshall, 2017-01-16 16:01:35]

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

[commit-bot: I haz the power, 2017-01-16 16:01:42]

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

[petermarshall, 2017-01-16 16:06:06]

PTAL for interpreter/ updates =]

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
File src/builtins/x64/builtins-x64.cc (right):

https://codereview.chromium.org/2571563004/diff/420001/src/builtins/x64/built...
src/builtins/x64/builtins-x64.cc:917: void
Builtins::Generate_InterpreterPushArgsAndConstructWithSpread(
On 2017/01/12 at 10:40:38, rmcilroy wrote:
> Right, I wasn't suggesting to modifiy CallableType, but to have a new enum
type which is only used by this function (which includes JSFunction / Any and
Spread options).

Ah I misread it a bit. How does this look now? Can change the name of it if you
have something better.

[rmcilroy, 2017-01-16 16:28:02]

Interpreter LGTM with nits, thanks for the changes.

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-array-builder.h:249: BytecodeArrayBuilder&
NewWithSpread(Register constructor, RegisterList args);
I realize you didn't change this in this CL, but this should be up near the
New() function.

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2547:
builder()->NewWithSpread(constructor, args_regs);
nit - add a TODO to collect type on the feedback slot.

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2550:
builder()->SetExpressionPosition(expr);
nit - move setExpressionPosition to be outside the if (it should be set for both
new and NewWithSpread calls.

[commit-bot: I haz the power, 2017-01-16 16:31:13]

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

[commit-bot: I haz the power, 2017-01-16 16:31:14]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2017-01-16 18:08:01]

Awesome, Thanks! LGTM!

[petermarshall, 2017-01-17 14:46:47]

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

[commit-bot: I haz the power, 2017-01-17 14:46: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-01-17 15:21:07]

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

[commit-bot: I haz the power, 2017-01-17 15:21:08]

Dry run: This issue passed the CQ dry run.

[petermarshall, 2017-01-18 10:04:47]

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

[commit-bot: I haz the power, 2017-01-18 10:04:57]

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

[petermarshall, 2017-01-18 10:05:32]

Benedikt, could you PTAL at the MIPS64 port?

Then we are good to go!

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
File src/interpreter/bytecode-array-builder.h (right):

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-array-builder.h:249: BytecodeArrayBuilder&
NewWithSpread(Register constructor, RegisterList args);
On 2017/01/16 at 16:28:02, rmcilroy wrote:
> I realize you didn't change this in this CL, but this should be up near the
New() function.

Done

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2547:
builder()->NewWithSpread(constructor, args_regs);
On 2017/01/16 at 16:28:02, rmcilroy wrote:
> nit - add a TODO to collect type on the feedback slot.

Done

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2547:
builder()->NewWithSpread(constructor, args_regs);
On 2017/01/16 at 16:28:02, rmcilroy wrote:
> nit - add a TODO to collect type on the feedback slot.

Done

https://codereview.chromium.org/2571563004/diff/520001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2550:
builder()->SetExpressionPosition(expr);
On 2017/01/16 at 16:28:02, rmcilroy wrote:
> nit - move setExpressionPosition to be outside the if (it should be set for
both new and NewWithSpread calls.

Oops - done

[Benedikt Meurer, 2017-01-18 10:13:10]

LGTM on mips64.

[petermarshall, 2017-01-18 10:15:20]

The CQ bit was unchecked by petermarshall@chromium.org

[petermarshall, 2017-01-18 10:15:24]

The CQ bit was checked by petermarshall@chromium.org

[petermarshall, 2017-01-18 10:15:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2571563004/#ps580001
(title: "MIPS64 port")

[commit-bot: I haz the power, 2017-01-18 10:15:36]

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-01-18 10:34:26]

CQ is committing da patch.

Bot data: {"patchset_id": 580001, "attempt_start_ts": 1484734524021720,
"parent_rev": "2f3de27e1142aa0d5d1b5fe82c5c632786390c8a", "commit_rev":
"0b8200c7664089b101fe4d1ec2695af885929dbe"}

[commit-bot: I haz the power, 2017-01-18 10:34:31]

Description was changed from

==========
[Turbofan] Implement super calls with spread bytecode in assembly code.

BUG=v8:5659
==========

to

==========
[Turbofan] Implement super calls with spread bytecode in assembly code.

BUG=v8:5659

Review-Url: https://codereview.chromium.org/2571563004
Cr-Commit-Position: refs/heads/master@{#42444}
Committed:
https://chromium.googlesource.com/v8/v8/+/0b8200c7664089b101fe4d1ec2695af8859...
==========

[commit-bot: I haz the power, 2017-01-18 10:34:33]

Committed patchset #30 (id:580001) as
https://chromium.googlesource.com/v8/v8/+/0b8200c7664089b101fe4d1ec2695af8859...