[builtins] Start refactoring the Apply builtin.

[builtins] Start refactoring the Apply builtin.

This splits the monolithic Apply builtin into several smaller builtins,
namely CallVargargs and ConstructVarargs, which accept a length and a
FixedArray of elements and deal with the actual stack manipulation, and
CallWithArrayLike / ConstructWithArrayLike that deal with getting the
elements from the receiver (for Function.prototype.apply, Reflect.apply
and Reflect.construct), which can now be written using the CSA.

The idea is that these builtins can be reused by TurboFan directly in
the future when we optimize apply better, and that we can also reuse the
core logic in the handling of spread calls/constructs.

R=petermarshall@chromium.org
BUG=v8:4587, v8:5269
Review-Url: https://codereview.chromium.org/2930623002
Cr-Commit-Position: refs/heads/master@{#45794}
Committed: https://chromium.googlesource.com/v8/v8/+/af76779aa3064a8d5ce8f7b88836110f3de4cfa0

[Benedikt Meurer, 2017-06-07 12:02:36]

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

[commit-bot: I haz the power, 2017-06-07 12:02:46]

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-06-07 12:32:54]

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

[commit-bot: I haz the power, 2017-06-07 12:32:56]

Dry run: This issue passed the CQ dry run.

[petermarshall, 2017-06-08 08:49:05]

LGTM with nits, thanks!

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
File src/builtins/builtins-call-gen.cc (right):

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
src/builtins/builtins-call-gen.cc:128:
var_length.Bind(LoadAndUntagToWord32ObjectField(arguments_list,
You can use LoadJSArrayLength

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
src/builtins/builtins-call-gen.cc:149: Node* arguments_list_prototype =
LoadMapPrototype =]

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
src/builtins/builtins-call-gen.cc:155: Node* protector_cell =
LoadRoot(Heap::kArrayProtectorRootIndex);
Might be worth adding 
DCHECK(isolate()->heap()->array_protector()->IsPropertyCell());
here

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins.h#new...
src/builtins/builtins.h:129: static void Generate_Varargs(MacroAssembler* masm,
Handle<Code> code);
'Varargs' is a bit confusing because there is no verb, what about 'PushVarargs'?

[Benedikt Meurer, 2017-06-08 18:02:42]

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

[commit-bot: I haz the power, 2017-06-08 18:02:51]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Benedikt Meurer, 2017-06-08 18:02:58]

Thanks, feedback addressed.

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
File src/builtins/builtins-call-gen.cc (right):

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
src/builtins/builtins-call-gen.cc:128:
var_length.Bind(LoadAndUntagToWord32ObjectField(arguments_list,
That doesn't untag.

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
src/builtins/builtins-call-gen.cc:149: Node* arguments_list_prototype =
On 2017/06/08 08:49:04, petermarshall wrote:
> LoadMapPrototype =]

Done.

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins-call-...
src/builtins/builtins-call-gen.cc:155: Node* protector_cell =
LoadRoot(Heap::kArrayProtectorRootIndex);
On 2017/06/08 08:49:04, petermarshall wrote:
> Might be worth adding 
> DCHECK(isolate()->heap()->array_protector()->IsPropertyCell());
> here

Done.

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins.h
File src/builtins/builtins.h (right):

https://codereview.chromium.org/2930623002/diff/1/src/builtins/builtins.h#new...
src/builtins/builtins.h:129: static void Generate_Varargs(MacroAssembler* masm,
Handle<Code> code);
Renamed to CallOrConstructVarargs and CallOrConstructForwardVarargs.

[Benedikt Meurer, 2017-06-08 18:03:03]

The CQ bit was unchecked by bmeurer@chromium.org

[Benedikt Meurer, 2017-06-08 18:03:13]

The CQ bit was checked by bmeurer@chromium.org

[Benedikt Meurer, 2017-06-08 18:03:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
petermarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/2930623002/#ps20001
(title: "Address feedback.")

[commit-bot: I haz the power, 2017-06-08 18:03:21]

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-06-08 18:32:00]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496944993568020,
"parent_rev": "ae947e26fe1a8c5ff490af93922f81c38f765f6f", "commit_rev":
"af76779aa3064a8d5ce8f7b88836110f3de4cfa0"}

[commit-bot: I haz the power, 2017-06-08 18:32:08]

Description was changed from

==========
[builtins] Start refactoring the Apply builtin.

This splits the monolithic Apply builtin into several smaller builtins,
namely CallVargargs and ConstructVarargs, which accept a length and a
FixedArray of elements and deal with the actual stack manipulation, and
CallWithArrayLike / ConstructWithArrayLike that deal with getting the
elements from the receiver (for Function.prototype.apply, Reflect.apply
and Reflect.construct), which can now be written using the CSA.

The idea is that these builtins can be reused by TurboFan directly in
the future when we optimize apply better, and that we can also reuse the
core logic in the handling of spread calls/constructs.

R=petermarshall@chromium.org
BUG=v8:4587,v8:5269
==========

to

==========
[builtins] Start refactoring the Apply builtin.

This splits the monolithic Apply builtin into several smaller builtins,
namely CallVargargs and ConstructVarargs, which accept a length and a
FixedArray of elements and deal with the actual stack manipulation, and
CallWithArrayLike / ConstructWithArrayLike that deal with getting the
elements from the receiver (for Function.prototype.apply, Reflect.apply
and Reflect.construct), which can now be written using the CSA.

The idea is that these builtins can be reused by TurboFan directly in
the future when we optimize apply better, and that we can also reuse the
core logic in the handling of spread calls/constructs.

R=petermarshall@chromium.org
BUG=v8:4587,v8:5269

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

[commit-bot: I haz the power, 2017-06-08 18:32:09]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/af76779aa3064a8d5ce8f7b88836110f3de...