[builtins] Port parameter and argument-related code stubs to CSA

[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inline
the implementation of these into the corresponding interpreter
byte codes.

BUG=v8:5269
LOG=N
R=ishell@chromium.org, rmcilroy@chromium.org

Review-Url: https://codereview.chromium.org/2645743002
Cr-Commit-Position: refs/heads/master@{#43002}
Committed: https://chromium.googlesource.com/v8/v8/+/c205c9b7ea717702d12855d133c05d2a4608abd4

[danno, 2017-01-25 13:48:25]

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

[commit-bot: I haz the power, 2017-01-25 13:48:30]

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-25 13:51:23]

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

[commit-bot: I haz the power, 2017-01-25 13:51:24]

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

[danno, 2017-01-25 13:54:29]

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

[commit-bot: I haz the power, 2017-01-25 13:54: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-25 13:57:10]

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

[commit-bot: I haz the power, 2017-01-25 13:57:10]

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

[danno, 2017-01-25 14:02:32]

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

[commit-bot: I haz the power, 2017-01-25 14:02:39]

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-25 14:08:08]

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

[commit-bot: I haz the power, 2017-01-25 14:08:09]

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

[danno, 2017-01-25 14:17:55]

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

[commit-bot: I haz the power, 2017-01-25 14:18:07]

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

[danno, 2017-01-25 14:18:52]

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

[commit-bot: I haz the power, 2017-01-25 14:18: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-01-25 14:52:42]

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

[commit-bot: I haz the power, 2017-01-25 14:52:43]

Dry run: This issue passed the CQ dry run.

[danno, 2017-01-27 15:32:57]

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

[commit-bot: I haz the power, 2017-01-27 15:32: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-01-27 16:29:47]

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

[commit-bot: I haz the power, 2017-01-27 16:29:48]

Dry run: This issue passed the CQ dry run.

[danno, 2017-01-27 16:34:44]

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

[commit-bot: I haz the power, 2017-01-27 16:34: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, 2017-01-27 17:08:01]

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

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

Dry run: This issue passed the CQ dry run.

[danno, 2017-01-30 15:51:19]

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

[commit-bot: I haz the power, 2017-01-30 15:51:22]

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-30 16:27:29]

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

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

Dry run: This issue passed the CQ dry run.

[danno, 2017-01-31 07:38:38]

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

[commit-bot: I haz the power, 2017-01-31 07:38:49]

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

[danno, 2017-01-31 07:42:53]

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

[commit-bot: I haz the power, 2017-01-31 07:42:58]

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

[danno, 2017-01-31 07:46:03]

Description was changed from

==========
[csa] Port FastNewStrictArguments to TF
==========

to

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inlines
the implementation of these into the corresponding interpreter
byte codes.
==========

[danno, 2017-01-31 07:46:04]

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

[danno, 2017-01-31 07:46:22]

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

[danno, 2017-01-31 07:46:23]

ptal

[danno, 2017-01-31 07:50:52]

Description was changed from

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inlines
the implementation of these into the corresponding interpreter
byte codes.
==========

to

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inlines
the implementation of these into the corresponding interpreter
byte codes.

BUG=v8:5269
LOG=N
R=ishell@chromium.org, rmcilroy@chromium.org
==========

[danno, 2017-01-31 07:51:17]

Description was changed from

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inlines
the implementation of these into the corresponding interpreter
byte codes.

BUG=v8:5269
LOG=N
R=ishell@chromium.org, rmcilroy@chromium.org
==========

to

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inline
the implementation of these into the corresponding interpreter
byte codes.

BUG=v8:5269
LOG=N
R=ishell@chromium.org, rmcilroy@chromium.org
==========

[commit-bot: I haz the power, 2017-01-31 08:08:50]

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

[commit-bot: I haz the power, 2017-01-31 08:08:51]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-01-31 20:49:00]

interpreter LGTM, didn't look at the rest.

[Igor Sheludko, 2017-02-06 20:17:03]

lgtm with nits:

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
File src/builtins/builtins-arguments.cc (right):

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:29: Variable argument_count(this, mode ==
SMI_PARAMETERS
Please introduce CSA::ParameterRepresentation(ParameterMode mode).

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:33: this, mode == SMI_PARAMETERS ?
MachineRepresentation::kTagged
Same here.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:58:
formal_parameter_count.Bind(parameter_count);
It looks like the formal_parameter_count computation code is the same for both
branches. Maybe you can put it before the check.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:67: argument_count.Bind(parameter_count);
And if you also bind this above then the no-adapter block doesn't even need to
be present.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:91: bool empty =
IsIntPtrOrSmiConstantZero(arguments_count);
DCHECK_IMPLIES(empty, parameter_map_count == nullptr);

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

https://codereview.chromium.org/2645743002/diff/300001/src/interface-descript...
src/interface-descriptors.h:468: class FastNewSloppyArgumentsDescriptor : public
CallInterfaceDescriptor {
I guess we can now have only one FastNewArgumentsDescriptor instead of three.

[danno, 2017-02-07 15:26:12]

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

[commit-bot: I haz the power, 2017-02-07 15:26:16]

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

[danno, 2017-02-07 15:30:20]

Feedback addressed, landing

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
File src/builtins/builtins-arguments.cc (right):

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:29: Variable argument_count(this, mode ==
SMI_PARAMETERS
On 2017/02/06 20:17:02, Igor Sheludko wrote:
> Please introduce CSA::ParameterRepresentation(ParameterMode mode).

Done.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:33: this, mode == SMI_PARAMETERS ?
MachineRepresentation::kTagged
On 2017/02/06 20:17:03, Igor Sheludko wrote:
> Same here.

Done.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:58:
formal_parameter_count.Bind(parameter_count);
On 2017/02/06 20:17:02, Igor Sheludko wrote:
> It looks like the formal_parameter_count computation code is the same for both
> branches. Maybe you can put it before the check.

Done.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:67: argument_count.Bind(parameter_count);
On 2017/02/06 20:17:03, Igor Sheludko wrote:
> And if you also bind this above then the no-adapter block doesn't even need to
> be present.

Done.

https://codereview.chromium.org/2645743002/diff/300001/src/builtins/builtins-...
src/builtins/builtins-arguments.cc:91: bool empty =
IsIntPtrOrSmiConstantZero(arguments_count);
On 2017/02/06 20:17:02, Igor Sheludko wrote:
> DCHECK_IMPLIES(empty, parameter_map_count == nullptr);

Done.

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

https://codereview.chromium.org/2645743002/diff/300001/src/interface-descript...
src/interface-descriptors.h:468: class FastNewSloppyArgumentsDescriptor : public
CallInterfaceDescriptor {
On 2017/02/06 20:17:03, Igor Sheludko wrote:
> I guess we can now have only one FastNewArgumentsDescriptor instead of three.

Done.

[commit-bot: I haz the power, 2017-02-07 15:53:45]

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

[commit-bot: I haz the power, 2017-02-07 15:53:46]

Dry run: This issue passed the CQ dry run.

[danno, 2017-02-07 16:01:09]

The CQ bit was checked by danno@chromium.org

[danno, 2017-02-07 16:01:10]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org,
rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/2645743002/#ps320001
(title: "Review feedback")

[commit-bot: I haz the power, 2017-02-07 16:01:15]

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-02-07 16:02:45]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1486483269414330,
"parent_rev": "4d8bde0cd64e5c2c525dde08f41c13579b04ad21", "commit_rev":
"c205c9b7ea717702d12855d133c05d2a4608abd4"}

[commit-bot: I haz the power, 2017-02-07 16:02:50]

Description was changed from

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inline
the implementation of these into the corresponding interpreter
byte codes.

BUG=v8:5269
LOG=N
R=ishell@chromium.org, rmcilroy@chromium.org
==========

to

==========
[builtins] Port parameter and argument-related code stubs to CSA

Includes the port of these three builtins: FastNewStrictArguments,
FastNewSloppyArguments and FastNewRestParameter. Also inline
the implementation of these into the corresponding interpreter
byte codes.

BUG=v8:5269
LOG=N
R=ishell@chromium.org, rmcilroy@chromium.org

Review-Url: https://codereview.chromium.org/2645743002
Cr-Commit-Position: refs/heads/master@{#43002}
Committed:
https://chromium.googlesource.com/v8/v8/+/c205c9b7ea717702d12855d133c05d2a460...
==========

[commit-bot: I haz the power, 2017-02-07 16:02:52]

Committed patchset #17 (id:320001) as
https://chromium.googlesource.com/v8/v8/+/c205c9b7ea717702d12855d133c05d2a460...