Move desugaring of super calls with trailing spread to one runtime call.

Move desugaring of super calls with trailing spread to one runtime call.

Unfortunately we have to split this up into two cases: those with exactly one spread argument as the final argument, and all others, due to any side-effects of evaluation being visible.

This is in preparation for a new bytecode which handles super calls.

BUG=v8:5659
Committed: https://crrev.com/a328143eb33f225488c6efb81f7c514cbb8b19be
Cr-Commit-Position: refs/heads/master@{#41415}

[petermarshall, 2016-11-29 12:33:42]

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

[commit-bot: I haz the power, 2016-11-29 12:33: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-11-29 13:00:21]

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

[commit-bot: I haz the power, 2016-11-29 13:00:22]

Dry run: This issue passed the CQ dry run.

[petermarshall, 2016-11-29 13:04:47]

Description was changed from

==========
Move desugaring of super calls with trailing spread to one runtime call.

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

to

==========
Move desugaring of super calls with trailing spread to one runtime call.

Unfortunately we have to split this up into two cases: those with exactly one
spread argument as the final argument, and all others, due to any side-effects
of evaluation being visible.

This is in preparation for a new bytecode which handles super calls.

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

[petermarshall, 2016-11-29 13:08:01]

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

[petermarshall, 2016-11-29 13:08:02]

PTAL :)

[Benedikt Meurer, 2016-11-29 13:15:40]

LGTM once comments/nits addressed.

https://codereview.chromium.org/2540593003/diff/20001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (right):

https://codereview.chromium.org/2540593003/diff/20001/src/ast/ast-numbering.c...
src/ast/ast-numbering.cc:397: void AstNumberingVisitor::VisitSpread(Spread*
node) {
Don't we need a DisableFullCodegenAndCrankshaft() call in here?

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime-arr...
File src/runtime/runtime-array.cc (right):

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime-arr...
src/runtime/runtime-array.cc:640: bool mustIterate(Isolate* isolate,
Handle<Object> spread) {
Nit: CamlCase.

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime-arr...
src/runtime/runtime-array.cc:710: spread_array->length()->ToArrayIndex(&length);
Nit: CHECK the result.

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime.h
File src/runtime/runtime.h (right):

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime.h#n...
src/runtime/runtime.h:61: F(SpreadIterablePrepareVarArgs, -1, 1)
Nit: Can we use "Varargs" instead of "VarArgs"?

[petermarshall, 2016-11-29 13:32:27]

https://codereview.chromium.org/2540593003/diff/20001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (right):

https://codereview.chromium.org/2540593003/diff/20001/src/ast/ast-numbering.c...
src/ast/ast-numbering.cc:397: void AstNumberingVisitor::VisitSpread(Spread*
node) {
On 2016/11/29 at 13:15:40, Benedikt Meurer wrote:
> Don't we need a DisableFullCodegenAndCrankshaft() call in here?

Hmmm currently the spread only ends up in the AST if it is a child of a
super-call node, meaning DisableFullCodegenAndCrankshaft() has already been
called, but it isn't possible to assert that here I don't think. Should we just
call it here again anyway, to be sure? Problem being that it could potentially
mask bugs in the future if a spread ends up in the AST when it shouldn't be.

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime-arr...
File src/runtime/runtime-array.cc (right):

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime-arr...
src/runtime/runtime-array.cc:640: bool mustIterate(Isolate* isolate,
Handle<Object> spread) {
On 2016/11/29 at 13:15:40, Benedikt Meurer wrote:
> Nit: CamlCase.

Done

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime-arr...
src/runtime/runtime-array.cc:710: spread_array->length()->ToArrayIndex(&length);
On 2016/11/29 at 13:15:40, Benedikt Meurer wrote:
> Nit: CHECK the result.

Done

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime.h
File src/runtime/runtime.h (right):

https://codereview.chromium.org/2540593003/diff/20001/src/runtime/runtime.h#n...
src/runtime/runtime.h:61: F(SpreadIterablePrepareVarArgs, -1, 1)
On 2016/11/29 at 13:15:40, Benedikt Meurer wrote:
> Nit: Can we use "Varargs" instead of "VarArgs"?

Done

[Benedikt Meurer, 2016-11-29 13:36:56]

LGTM

https://codereview.chromium.org/2540593003/diff/20001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (right):

https://codereview.chromium.org/2540593003/diff/20001/src/ast/ast-numbering.c...
src/ast/ast-numbering.cc:397: void AstNumberingVisitor::VisitSpread(Spread*
node) {
I see, makes sense. Let's leave it this way.

[petermarshall, 2016-11-29 13:38:12]

The CQ bit was checked by petermarshall@chromium.org

[commit-bot: I haz the power, 2016-11-29 13:38:19]

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-11-29 13:41:08]

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

[commit-bot: I haz the power, 2016-11-29 13:41:08]

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

[petermarshall, 2016-11-29 13:43:21]

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

[petermarshall, 2016-11-29 13:43:22]

Daniel, could you PTAL for /parsing/*?

[vogelheim, 2016-11-29 14:55:19]

lgtm for src/parsing/...


Generally speaking I find this code quite confusing:

It seems that previously, calling PrepareSpreadArguments(.) was the job of the
parsing part of the parser, and the AST building part of the parser could rely
on whatever work this actually does being done.

Now, in the case of SpreadCall PrepareSpreadArguments is done in AST building,
but for SpreadCallNew it's still done in parsing proper. The code might be
easier to read if this is consistently done in AST building. In that case,
PrepareSpreadArguments could be removed entirely from ParserBase and PreParser.

[rmcilroy, 2016-11-29 15:51:55]

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

[rmcilroy, 2016-11-29 15:51:56]

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2491:
register_allocator()->NewRegisterList(args->length());
These are only used in the else branch, please move them there.

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2501:
register_allocator()->NewRegisterList(4);
You can move this down to the first point you use it (above
builder()->StoreAccumulatorInRegister(reflect_construct_args[2]) - this will
avoid keeping these registers allocated while visiting the arguments.

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2503: // Deal with the spread arg
fullstop on comments

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2512: // visit args at 0 --> length -2
inclusive
And capitals

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2512: // visit args at 0 --> length -2
inclusive
length -2 -> length - 2

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2519: spread_prepare_args[args->length() -
1]);
Is this not assuming the spread arg is at the end of the arguments? If this is
the case, why do we need to loop through all the arguments to check if there is
a spread at line 2594?

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2523: }
This seems a lot of special casing for either 1 spread argument or multiple
spreads - can't the runtime function just deal with the case were you only have
one spread arg or multiple instead of doing something different for both cases?

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2524: 
Could the code dealing with spread arguments be moved to a seperate function
(e.g., VisitSpreadArguments or similar)?

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2533: reflect_construct_args);
Please add tests for this in test-bytecode-generator for bytecode expectations.

[petermarshall, 2016-11-30 13:11:09]

Thanks for the comments Daniel and Ross, I think it is looking much cleaner now.
Discussed offline with Daniel how to make the use of PrepareSpreadArguments
consistent and have implemented that here. Ross - anything more to look at from
your side?

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2491:
register_allocator()->NewRegisterList(args->length());
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> These are only used in the else branch, please move them there.

Done

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2501:
register_allocator()->NewRegisterList(4);
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> You can move this down to the first point you use it (above
builder()->StoreAccumulatorInRegister(reflect_construct_args[2]) - this will
avoid keeping these registers allocated while visiting the arguments.

Done

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2503: // Deal with the spread arg
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> fullstop on comments

Done

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2512: // visit args at 0 --> length -2
inclusive
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> And capitals

Done

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2519: spread_prepare_args[args->length() -
1]);
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> Is this not assuming the spread arg is at the end of the arguments? If this is
the case, why do we need to loop through all the arguments to check if there is
a spread at line 2594?

There are two cases here - with and without spread. With spread must call into
Reflect.Construct, without can just emit the New bytecode with the args it was
passed.

It gets kind of confusing because we only create SuperCall nodes if the spread
is the last argument. So a spread being present implies that it is the last
argument.

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2523: }
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> This seems a lot of special casing for either 1 spread argument or multiple
spreads - can't the runtime function just deal with the case were you only have
one spread arg or multiple instead of doing something different for both cases?

Yeah that is definitely much simpler, done.

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2524: 
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> Could the code dealing with spread arguments be moved to a seperate function
(e.g., VisitSpreadArguments or similar)?

Yep much simpler, done

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2533: reflect_construct_args);
On 2016/11/29 at 15:51:56, rmcilroy wrote:
> Please add tests for this in test-bytecode-generator for bytecode
expectations.

Done

[rmcilroy, 2016-11-30 14:35:32]

Looks good. A couple of last comments, all of which are nits apart from
DisableFullCodegenAndCrankshaft for spread which I think you need to fix.

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2519: spread_prepare_args[args->length() -
1]);
On 2016/11/30 13:11:09, petermarshall wrote:
> On 2016/11/29 at 15:51:56, rmcilroy wrote:
> > Is this not assuming the spread arg is at the end of the arguments? If this
is
> the case, why do we need to loop through all the arguments to check if there
is
> a spread at line 2594?
> 
> There are two cases here - with and without spread. With spread must call into
> Reflect.Construct, without can just emit the New bytecode with the args it was
> passed.
> 
> It gets kind of confusing because we only create SuperCall nodes if the spread
> is the last argument. So a spread being present implies that it is the last
> argument.

Right, My point was more that if the spread is only present on the last argument
(if it exists) means the for loop on line 2494 is unnecessary, just check check:
  if (args->at(args->length() - 1)->IsSpread() {
    ... Do REFLECT_CONSTRUCT_INDEX code
  } else {
    // DCHECK none of the other arguments are spread (if necessary)
    .. Do normal new case 
  }

https://codereview.chromium.org/2540593003/diff/140001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (right):

https://codereview.chromium.org/2540593003/diff/140001/src/ast/ast-numbering....
src/ast/ast-numbering.cc:398: IncrementNodeCount();
I think this should this disable Fullcodegen and Crankshaft
(DisableFullCodegenAndCrankshaft) since we only have bytecode graph builder
implementations of it?

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

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2497: // Deal with the spread arg.
nit - Prepare the spread arguments. ?

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2502: Runtime::FunctionId function_id =
Runtime::kSpreadIterablePrepareVarargs;
nit - no need for the function_id variable now

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2505: // Do ReflectConstruct.
nit - Call ReflectConstruct to do the actual super constructor call.  ?

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2509: Register receiver =
reflect_construct_args[0];
nit - I'd prefer that the code just consistently uses reflect_construct_args[0],
if it isn't going to be used for anything other than the runtime call.

Also, you coul use the builder chaining for builder calls :

 builder()->StoreAccumulatorInRegister(reflect_construct_args[2])
    .LoadUndefined()
    .StoreAccumulatorInRegister(reflect_construct_args[0])
    .MoveRegister(constructor, reflect_construct_args[1]);

[petermarshall, 2016-11-30 15:24:48]

Thanks =]

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/2540593003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:2519: spread_prepare_args[args->length() -
1]);
On 2016/11/30 at 14:35:32, rmcilroy wrote:
> On 2016/11/30 13:11:09, petermarshall wrote:
> > On 2016/11/29 at 15:51:56, rmcilroy wrote:
> > > Is this not assuming the spread arg is at the end of the arguments? If
this is
> > the case, why do we need to loop through all the arguments to check if there
is
> > a spread at line 2594?
> > 
> > There are two cases here - with and without spread. With spread must call
into
> > Reflect.Construct, without can just emit the New bytecode with the args it
was
> > passed.
> > 
> > It gets kind of confusing because we only create SuperCall nodes if the
spread
> > is the last argument. So a spread being present implies that it is the last
> > argument.
> 
> Right, My point was more that if the spread is only present on the last
argument (if it exists) means the for loop on line 2494 is unnecessary, just
check check:
>   if (args->at(args->length() - 1)->IsSpread() {
>     ... Do REFLECT_CONSTRUCT_INDEX code
>   } else {
>     // DCHECK none of the other arguments are spread (if necessary)
>     .. Do normal new case 
>   }

Ahhhh I see your point, that is much nicer. Need to check that args isn't empty
too. Also there is a handy last() function.

https://codereview.chromium.org/2540593003/diff/140001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (right):

https://codereview.chromium.org/2540593003/diff/140001/src/ast/ast-numbering....
src/ast/ast-numbering.cc:398: IncrementNodeCount();
On 2016/11/30 at 14:35:32, rmcilroy wrote:
> I think this should this disable Fullcodegen and Crankshaft
(DisableFullCodegenAndCrankshaft) since we only have bytecode graph builder
implementations of it?

I think we are technically safe because a spread only shows up in the AST as a
child of a SuperCallReference, which disables them already, although we don't
have a way for the type system or DCHECKs to enforce this so it could break
weirdly in the future. I disabled it here anyway and left a comment.

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

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2497: // Deal with the spread arg.
On 2016/11/30 at 14:35:32, rmcilroy wrote:
> nit - Prepare the spread arguments. ?

Done

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2502: Runtime::FunctionId function_id =
Runtime::kSpreadIterablePrepareVarargs;
On 2016/11/30 at 14:35:32, rmcilroy wrote:
> nit - no need for the function_id variable now

Done

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2505: // Do ReflectConstruct.
On 2016/11/30 at 14:35:32, rmcilroy wrote:
> nit - Call ReflectConstruct to do the actual super constructor call.  ?

Done.

https://codereview.chromium.org/2540593003/diff/140001/src/interpreter/byteco...
src/interpreter/bytecode-generator.cc:2509: Register receiver =
reflect_construct_args[0];
On 2016/11/30 at 14:35:32, rmcilroy wrote:
> nit - I'd prefer that the code just consistently uses
reflect_construct_args[0], if it isn't going to be used for anything other than
the runtime call.
> 
> Also, you coul use the builder chaining for builder calls :
> 
>  builder()->StoreAccumulatorInRegister(reflect_construct_args[2])
>     .LoadUndefined()
>     .StoreAccumulatorInRegister(reflect_construct_args[0])
>     .MoveRegister(constructor, reflect_construct_args[1]);

Done

[rmcilroy, 2016-11-30 15:45:04]

LGTM, thanks.

https://codereview.chromium.org/2540593003/diff/140001/src/ast/ast-numbering.cc
File src/ast/ast-numbering.cc (right):

https://codereview.chromium.org/2540593003/diff/140001/src/ast/ast-numbering....
src/ast/ast-numbering.cc:398: IncrementNodeCount();
On 2016/11/30 15:24:48, petermarshall wrote:
> On 2016/11/30 at 14:35:32, rmcilroy wrote:
> > I think this should this disable Fullcodegen and Crankshaft
> (DisableFullCodegenAndCrankshaft) since we only have bytecode graph builder
> implementations of it?
> 
> I think we are technically safe because a spread only shows up in the AST as a
> child of a SuperCallReference, which disables them already, although we don't
> have a way for the type system or DCHECKs to enforce this so it could break
> weirdly in the future. I disabled it here anyway and left a comment.

Ahh makes sense, thanks.

[petermarshall, 2016-11-30 15:53:42]

The CQ bit was checked by petermarshall@chromium.org

[petermarshall, 2016-11-30 15:53:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, vogelheim@chromium.org
Link to the patchset: https://codereview.chromium.org/2540593003/#ps180001
(title: "Clean up spread recognition in bytecodegen")

[commit-bot: I haz the power, 2016-11-30 15:53:52]

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-11-30 16:33:38]

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

[commit-bot: I haz the power, 2016-11-30 16:33:39]

Try jobs failed on following builders:
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/17052)
  v8_linux_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)

[petermarshall, 2016-12-01 09:10:46]

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

[commit-bot: I haz the power, 2016-12-01 09:10:54]

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-01 09:39:22]

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

[commit-bot: I haz the power, 2016-12-01 09:39:23]

Dry run: This issue passed the CQ dry run.

[petermarshall, 2016-12-01 09:40:24]

The CQ bit was checked by petermarshall@chromium.org

[petermarshall, 2016-12-01 09:40:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, rmcilroy@chromium.org, vogelheim@chromium.org
Link to the patchset: https://codereview.chromium.org/2540593003/#ps200001
(title: "fix potential evaluation order issue")

[commit-bot: I haz the power, 2016-12-01 09:40:31]

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-01 09:42:07]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1480585224922190,
"parent_rev": "b05c11193e8a7faf37b39712ce95d5fb6c440698", "commit_rev":
"9107f4b6912d3fd407f543acd664789c3137ea60"}

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

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-12-01 09:42:47]

Description was changed from

==========
Move desugaring of super calls with trailing spread to one runtime call.

Unfortunately we have to split this up into two cases: those with exactly one
spread argument as the final argument, and all others, due to any side-effects
of evaluation being visible.

This is in preparation for a new bytecode which handles super calls.

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

to

==========
Move desugaring of super calls with trailing spread to one runtime call.

Unfortunately we have to split this up into two cases: those with exactly one
spread argument as the final argument, and all others, due to any side-effects
of evaluation being visible.

This is in preparation for a new bytecode which handles super calls.

BUG=v8:5659

Committed: https://crrev.com/a328143eb33f225488c6efb81f7c514cbb8b19be
Cr-Commit-Position: refs/heads/master@{#41415}
==========

[commit-bot: I haz the power, 2016-12-01 09:42:48]

Patchset 11 (id:??) landed as
https://crrev.com/a328143eb33f225488c6efb81f7c514cbb8b19be
Cr-Commit-Position: refs/heads/master@{#41415}

[Michael Achenbach, 2016-12-01 11:15:25]

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

[Michael Achenbach, 2016-12-01 11:15:26]

This might break:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%...

Could you take a look if it repro's?

[petermarshall, 2016-12-01 11:46:20]

On 2016/12/01 at 11:15:26, machenbach wrote:
> This might break:
>
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%...
> 
> Could you take a look if it repro's?

OK - taking a look

[petermarshall, 2016-12-01 15:41:52]

On 2016/12/01 at 11:46:20, petermarshall wrote:
> On 2016/12/01 at 11:15:26, machenbach wrote:
> > This might break:
> >
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%...
> > 
> > Could you take a look if it repro's?
> 
> OK - taking a look

Could repro locally.
It looks like this CL uncovers a problem with a WASM CL - have reverted it:
https://codereview.chromium.org/2538183003/

[Michael Achenbach, 2016-12-02 08:07:19]

On 2016/12/01 15:41:52, petermarshall wrote:
> On 2016/12/01 at 11:46:20, petermarshall wrote:
> > On 2016/12/01 at 11:15:26, machenbach wrote:
> > > This might break:
> > >
>
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%...
> > > 
> > > Could you take a look if it repro's?
> > 
> > OK - taking a look
> 
> Could repro locally.
> It looks like this CL uncovers a problem with a WASM CL - have reverted it:
> https://codereview.chromium.org/2538183003/

Looks like the revert didn't stick yet...