[parsing] inline ArrayLiteral creation for spread calls

Instead of using runtime calls to generate the Array Literal passed to
%reflect_call / %reflect_construct, we create an ArrayLiteral from the
list of arguments, and perform spreads using the interpreter mechanism for
spreading in ArrayLiterals (thus, the spreading becomes inline). This
array literal is still passed to %reflect_call / %reflect_construct as
before.

This cuts the runtime for bench-spread-call.js -> testSpread roughly in
half, and will likely improve further once
https://chromium-review.googlesource.com/c/v8/v8/+/915364 has landed.

BUG=v8:7446
R=neis@chromium.org, adamk@chromium.org

Change-Id: I74a6acd3a60aad422e4ac575275c7b567659d8ad
Reviewed-on: https://chromium-review.googlesource.com/939587
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51678}

src/contexts.h

@@ -57,7 +57,6 @@ enum ContextLookupFlags {
   V(REFLECT_CONSTRUCT_INDEX, JSFunction, reflect_construct)             \
   V(REFLECT_DEFINE_PROPERTY_INDEX, JSFunction, reflect_define_property) \
   V(REFLECT_DELETE_PROPERTY_INDEX, JSFunction, reflect_delete_property) \
-  V(SPREAD_ARGUMENTS_INDEX, JSFunction, spread_arguments)               \
   V(SPREAD_ITERABLE_INDEX, JSFunction, spread_iterable)                 \
   V(MATH_FLOOR_INDEX, JSFunction, math_floor)                           \
   V(MATH_POW_INDEX, JSFunction, math_pow)                               \

src/js/spread.js

@@ -12,22 +12,6 @@ var InternalArray = utils.InternalArray;
 
 // -------------------------------------------------------------------
 
-function SpreadArguments() {
-  var count = arguments.length;
-  var args = new InternalArray();
-
-  for (var i = 0; i < count; ++i) {
-    var array = arguments[i];
-    var length = array.length;
-    for (var j = 0; j < length; ++j) {
-      args.push(array[j]);
-    }
-  }
-
-  return args;
-}
-
-
 function SpreadIterable(collection) {
   if (IS_NULL_OR_UNDEFINED(collection)) {
     throw %make_type_error(kNotIterable, collection);
@@ -44,7 +28,6 @@ function SpreadIterable(collection) {
 // Exports
 
 %InstallToContext([
-  "spread_arguments", SpreadArguments,
   "spread_iterable", SpreadIterable,
 ]);
 

src/parsing/parser.cc

@@ -3564,96 +3564,52 @@ bool OnlyLastArgIsSpread(ZoneList<Expression*>* args) {
 
 }  // namespace
 
-ZoneList<Expression*>* Parser::PrepareSpreadArguments(
+ArrayLiteral* Parser::ArrayLiteralFromListWithSpread(
     ZoneList<Expression*>* list) {
-  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
-  if (list->length() == 1) {
-    // Spread-call with single spread argument produces an InternalArray
-    // containing the values from the array.
-    //
-    // Function is called or constructed with the produced array of arguments
-    //
-    // EG: Apply(Func, Spread(spread0))
-    ZoneList<Expression*>* spread_list =
-        new (zone()) ZoneList<Expression*>(0, zone());
-    spread_list->Add(list->at(0)->AsSpread()->expression(), zone());
-    args->Add(factory()->NewCallRuntime(Runtime::kSpreadIterablePrepare,
-                                        spread_list, kNoSourcePosition),
-              zone());
-    return args;
-  } else {
-    // Spread-call with multiple arguments produces array literals for each
-    // sequences of unspread arguments, and converts each spread iterable to
-    // an Internal array. Finally, all of these produced arrays are flattened
-    // into a single InternalArray, containing the arguments for the call.
-    //
-    // EG: Apply(Func, Flatten([unspread0, unspread1], Spread(spread0),
-    //                         Spread(spread1), [unspread2, unspread3]))
-    int i = 0;
-    int n = list->length();
-    while (i < n) {
-      if (!list->at(i)->IsSpread()) {
-        ZoneList<Expression*>* unspread =
-            new (zone()) ZoneList<Expression*>(1, zone());
-
-        // Push array of unspread parameters
-        while (i < n && !list->at(i)->IsSpread()) {
-          unspread->Add(list->at(i++), zone());
-        }
-        args->Add(factory()->NewArrayLiteral(unspread, kNoSourcePosition),
-                  zone());
-
-        if (i == n) break;
-      }
+  // If there's only a single spread argument, a fast path using CallWithSpread
+  // is taken.
+  DCHECK_LT(1, list->length());
 
-      // Push eagerly spread argument
-      ZoneList<Expression*>* spread_list =
-          new (zone()) ZoneList<Expression*>(1, zone());
-      spread_list->Add(list->at(i++)->AsSpread()->expression(), zone());
-      args->Add(factory()->NewCallRuntime(Context::SPREAD_ITERABLE_INDEX,
-                                          spread_list, kNoSourcePosition),
-                zone());
-    }
-
-    list = new (zone()) ZoneList<Expression*>(1, zone());
-    list->Add(factory()->NewCallRuntime(Context::SPREAD_ARGUMENTS_INDEX, args,
-                                        kNoSourcePosition),
-              zone());
-    return list;
+  // The arguments of the spread call become a single ArrayLiteral.
+  int first_spread = 0;
+  for (; first_spread < list->length() && !list->at(first_spread)->IsSpread();
+       ++first_spread) {
   }
-  UNREACHABLE();
+
+  DCHECK_LT(first_spread, list->length());
+  return factory()->NewArrayLiteral(list, first_spread, kNoSourcePosition);
 }
 
 Expression* Parser::SpreadCall(Expression* function,
-                               ZoneList<Expression*>* args, int pos,
+                               ZoneList<Expression*>* args_list, int pos,
                                Call::PossiblyEval is_possibly_eval) {
   // Handle this case in BytecodeGenerator.
-  if (OnlyLastArgIsSpread(args)) {
-    return factory()->NewCall(function, args, pos);
+  if (OnlyLastArgIsSpread(args_list)) {
+    return factory()->NewCall(function, args_list, pos);
   }
 
   if (function->IsSuperCallReference()) {
     // Super calls
     // $super_constructor = %_GetSuperConstructor(<this-function>)
     // %reflect_construct($super_constructor, args, new.target)
-
-    args = PrepareSpreadArguments(args);
+    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
     ZoneList<Expression*>* tmp = new (zone()) ZoneList<Expression*>(1, zone());
     tmp->Add(function->AsSuperCallReference()->this_function_var(), zone());
-    Expression* super_constructor = factory()->NewCallRuntime(
-        Runtime::kInlineGetSuperConstructor, tmp, pos);
-    args->InsertAt(0, super_constructor, zone());
+    args->Add(factory()->NewCallRuntime(Runtime::kInlineGetSuperConstructor,
+                                        tmp, pos),
+              zone());
+    args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
     args->Add(function->AsSuperCallReference()->new_target_var(), zone());
     return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args,
                                      pos);
   } else {
-    args = PrepareSpreadArguments(args);
+    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(3, zone());
     if (function->IsProperty()) {
       // Method calls
       if (function->AsProperty()->IsSuperAccess()) {
         Expression* home = ThisExpression(kNoSourcePosition);
-        args->InsertAt(0, function, zone());
-        args->InsertAt(1, home, zone());
+        args->Add(function, zone());
+        args->Add(home, zone());
       } else {
         Variable* temp = NewTemporary(ast_value_factory()->empty_string());
         VariableProxy* obj = factory()->NewVariableProxy(temp);
@@ -3662,28 +3618,29 @@ Expression* Parser::SpreadCall(Expression* function,
             kNoSourcePosition);
         function = factory()->NewProperty(
             assign_obj, function->AsProperty()->key(), kNoSourcePosition);
-        args->InsertAt(0, function, zone());
+        args->Add(function, zone());
         obj = factory()->NewVariableProxy(temp);
-        args->InsertAt(1, obj, zone());
+        args->Add(obj, zone());
       }
     } else {
       // Non-method calls
-      args->InsertAt(0, function, zone());
-      args->InsertAt(1, factory()->NewUndefinedLiteral(kNoSourcePosition),
-                     zone());
+      args->Add(function, zone());
+      args->Add(factory()->NewUndefinedLiteral(kNoSourcePosition), zone());
     }
+    args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
     return factory()->NewCallRuntime(Context::REFLECT_APPLY_INDEX, args, pos);
   }
 }
 
 Expression* Parser::SpreadCallNew(Expression* function,
-                                  ZoneList<Expression*>* args, int pos) {
-  if (OnlyLastArgIsSpread(args)) {
+                                  ZoneList<Expression*>* args_list, int pos) {
+  if (OnlyLastArgIsSpread(args_list)) {
     // Handle in BytecodeGenerator.
-    return factory()->NewCallNew(function, args, pos);
+    return factory()->NewCallNew(function, args_list, pos);
   }
-  args = PrepareSpreadArguments(args);
-  args->InsertAt(0, function, zone());
+  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
+  args->Add(function, zone());
+  args->Add(ArrayLiteralFromListWithSpread(args_list), zone());
 
   return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
 }

src/parsing/parser.h

@@ -502,7 +502,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   Expression* CloseTemplateLiteral(TemplateLiteralState* state, int start,
                                    Expression* tag);
 
-  ZoneList<Expression*>* PrepareSpreadArguments(ZoneList<Expression*>* list);
+  ArrayLiteral* ArrayLiteralFromListWithSpread(ZoneList<Expression*>* list);
   Expression* SpreadCall(Expression* function, ZoneList<Expression*>* args,
                          int pos, Call::PossiblyEval is_possibly_eval);
   Expression* SpreadCallNew(Expression* function, ZoneList<Expression*>* args,

src/runtime/runtime-array.cc

@@ -794,23 +794,5 @@ RUNTIME_FUNCTION(Runtime_ArrayIndexOf) {
   return Smi::FromInt(-1);
 }
 
-
-RUNTIME_FUNCTION(Runtime_SpreadIterablePrepare) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(Object, spread, 0);
-
-  // Iterate over the spread if we need to.
-  if (spread->IterationHasObservableEffects()) {
-    Handle<JSFunction> spread_iterable_function = isolate->spread_iterable();
-    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-        isolate, spread,
-        Execution::Call(isolate, spread_iterable_function,
-                        isolate->factory()->undefined_value(), 1, &spread));
-  }
-
-  return *spread;
-}
-
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -51,8 +51,7 @@ namespace internal {
   F(ArrayIsArray, 1, 1)                  \
   F(ArraySpeciesConstructor, 1, 1)       \
   F(ArrayIncludes_Slow, 3, 1)            \
-  F(ArrayIndexOf, 3, 1)                  \
-  F(SpreadIterablePrepare, 1, 1)
+  F(ArrayIndexOf, 3, 1)
 
 #define FOR_EACH_INTRINSIC_ATOMICS(F)           \
   F(AtomicsExchange, 3, 1)                      \

test/cctest/compiler/test-run-bytecode-graph-builder.cc

@@ -618,7 +618,7 @@ TEST(BytecodeGraphBuilderCallRuntime) {
        {factory->true_value(), BytecodeGraphTester::NewObject("[1, 2, 3]")}},
       {"function f(arg0) { return %Add(arg0, 2) }\nf(1)",
        {factory->NewNumberFromInt(5), factory->NewNumberFromInt(3)}},
-      {"function f(arg0) { return %spread_arguments(arg0).length }\nf([])",
+      {"function f(arg0) { return %spread_iterable(arg0).length }\nf([])",
        {factory->NewNumberFromInt(3),
         BytecodeGraphTester::NewObject("[1, 2, 3]")}},
   };

test/cctest/interpreter/bytecode_expectations/CallAndSpread.golden

@@ -65,9 +65,9 @@ handlers: [
 snippet: "
   Math.max(0, ...[1, 2, 3], 4);
 "
-frame size: 6
+frame size: 11
 parameter count: 1
-bytecode array length: 51
+bytecode array length: 109
 bytecodes: [
   /*   30 E> */ B(StackCheck),
   /*   34 S> */ B(LdaGlobal), U8(0), U8(0),
@@ -75,16 +75,34 @@ bytecodes: [
                 B(LdaNamedProperty), R(0), U8(1), U8(2),
                 B(Star), R(1),
                 B(CreateArrayLiteral), U8(2), U8(4), U8(37),
-                B(Star), R(3),
-                B(CreateArrayLiteral), U8(3), U8(5), U8(37),
-                B(Star), R(4),
-                B(CallJSRuntime), U8(%spread_iterable), R(4), U8(1),
                 B(Star), R(4),
-                B(CreateArrayLiteral), U8(4), U8(6), U8(37),
-                B(Star), R(5),
-                B(CallJSRuntime), U8(%spread_arguments), R(3), U8(3),
-                B(Star), R(3),
+  /*   49 S> */ B(CreateArrayLiteral), U8(3), U8(5), U8(37),
+                B(Star), R(9),
+                B(LdaNamedProperty), R(9), U8(4), U8(6),
+                B(Star), R(10),
+                B(CallProperty0), R(10), R(9), U8(8),
                 B(Mov), R(0), R(2),
+                B(Mov), R(4), R(5),
+                B(JumpIfJSReceiver), U8(7),
+                B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
+                B(Star), R(8),
+                B(LdaNamedProperty), R(8), U8(5), U8(10),
+                B(Star), R(7),
+                B(CallProperty0), R(7), R(8), U8(12),
+                B(Star), R(6),
+                B(JumpIfJSReceiver), U8(7),
+                B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(6), U8(1),
+                B(LdaNamedProperty), R(6), U8(6), U8(14),
+                B(JumpIfToBooleanTrue), U8(16),
+                B(LdaNamedProperty), R(6), U8(7), U8(16),
+                B(Star), R(6),
+                B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
+                B(JumpLoop), U8(30), I8(0),
+                B(LdaSmi), I8(4),
+                B(Star), R(6),
+                B(Mov), R(4), R(5),
+                B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
+                B(Mov), R(5), R(3),
                 B(CallJSRuntime), U8(%reflect_apply), R(1), U8(3),
                 B(LdaUndefined),
   /*   64 S> */ B(Return),
@@ -94,7 +112,10 @@ constant pool: [
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["max"],
   TUPLE2_TYPE,
   TUPLE2_TYPE,
-  TUPLE2_TYPE,
+  SYMBOL_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
 ]
 handlers: [
 ]

test/cctest/interpreter/bytecode_expectations/NewAndSpread.golden

@@ -84,9 +84,9 @@ snippet: "
   class A { constructor(...args) { this.args = args; } }
   new A(0, ...[1, 2, 3], 4);
 "
-frame size: 6
+frame size: 11
 parameter count: 1
-bytecode array length: 66
+bytecode array length: 124
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(LdaTheHole),
@@ -101,15 +101,33 @@ bytecodes: [
                 B(Mov), R(4), R(0),
                 B(Mov), R(0), R(1),
   /*   89 S> */ B(CreateArrayLiteral), U8(2), U8(1), U8(37),
-                B(Star), R(3),
-                B(CreateArrayLiteral), U8(3), U8(2), U8(37),
-                B(Star), R(4),
-                B(CallJSRuntime), U8(%spread_iterable), R(4), U8(1),
                 B(Star), R(4),
-                B(CreateArrayLiteral), U8(4), U8(3), U8(37),
-                B(Star), R(5),
-                B(CallJSRuntime), U8(%spread_arguments), R(3), U8(3),
-                B(Star), R(3),
+  /*  101 S> */ B(CreateArrayLiteral), U8(3), U8(2), U8(37),
+                B(Star), R(9),
+                B(LdaNamedProperty), R(9), U8(4), U8(3),
+                B(Star), R(10),
+                B(CallProperty0), R(10), R(9), U8(5),
+                B(Mov), R(4), R(5),
+                B(JumpIfJSReceiver), U8(7),
+                B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
+                B(Star), R(8),
+                B(LdaNamedProperty), R(8), U8(5), U8(7),
+                B(Star), R(7),
+                B(CallProperty0), R(7), R(8), U8(9),
+                B(Star), R(6),
+                B(JumpIfJSReceiver), U8(7),
+                B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(6), U8(1),
+                B(LdaNamedProperty), R(6), U8(6), U8(11),
+                B(JumpIfToBooleanTrue), U8(16),
+                B(LdaNamedProperty), R(6), U8(7), U8(13),
+                B(Star), R(6),
+                B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
+                B(JumpLoop), U8(30), I8(0),
+                B(LdaSmi), I8(4),
+                B(Star), R(6),
+                B(Mov), R(4), R(5),
+                B(CallRuntime), U16(Runtime::kAppendElement), R(5), U8(2),
+                B(Mov), R(5), R(3),
                 B(CallJSRuntime), U8(%reflect_construct), R(2), U8(2),
                 B(LdaUndefined),
   /*  116 S> */ B(Return),
@@ -119,7 +137,10 @@ constant pool: [
   SHARED_FUNCTION_INFO_TYPE,
   TUPLE2_TYPE,
   TUPLE2_TYPE,
-  TUPLE2_TYPE,
+  SYMBOL_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
 ]
 handlers: [
 ]

test/cctest/interpreter/bytecode_expectations/SuperCallAndSpread.golden

@@ -91,9 +91,9 @@ snippet: "
     test = new B(1, 2, 3).constructor;
   })();
 "
-frame size: 8
+frame size: 13
 parameter count: 1
-bytecode array length: 60
+bytecode array length: 121
 bytecodes: [
                 B(CreateRestParameter),
                 B(Star), R(2),
@@ -103,13 +103,32 @@ bytecodes: [
   /*  140 S> */ B(CallRuntime), U16(Runtime::k_GetSuperConstructor), R(closure), U8(1),
                 B(Star), R(4),
                 B(CreateArrayLiteral), U8(0), U8(0), U8(37),
-                B(Star), R(5),
-  /*  152 E> */ B(CallJSRuntime), U8(%spread_iterable), R(2), U8(1),
-                B(Star), R(6),
-                B(CreateArrayLiteral), U8(1), U8(1), U8(37),
-                B(Star), R(7),
-                B(CallJSRuntime), U8(%spread_arguments), R(5), U8(3),
-                B(Star), R(5),
+  /*  152 S> */ B(Star), R(6),
+                B(LdaNamedProperty), R(2), U8(1), U8(1),
+                B(Star), R(12),
+                B(CallProperty0), R(12), R(2), U8(3),
+                B(Mov), R(2), R(11),
+                B(Mov), R(6), R(7),
+                B(JumpIfJSReceiver), U8(7),
+                B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
+                B(Star), R(10),
+                B(LdaNamedProperty), R(10), U8(2), U8(5),
+                B(Star), R(9),
+                B(CallProperty0), R(9), R(10), U8(7),
+                B(Star), R(8),
+                B(JumpIfJSReceiver), U8(7),
+                B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(8), U8(1),
+                B(LdaNamedProperty), R(8), U8(3), U8(9),
+                B(JumpIfToBooleanTrue), U8(16),
+                B(LdaNamedProperty), R(8), U8(4), U8(11),
+                B(Star), R(8),
+                B(CallRuntime), U16(Runtime::kAppendElement), R(7), U8(2),
+                B(JumpLoop), U8(30), I8(0),
+                B(LdaSmi), I8(1),
+                B(Star), R(8),
+                B(Mov), R(6), R(7),
+                B(CallRuntime), U16(Runtime::kAppendElement), R(7), U8(2),
+                B(Mov), R(7), R(5),
                 B(Mov), R(0), R(6),
   /*  140 E> */ B(CallJSRuntime), U8(%reflect_construct), R(4), U8(3),
                 B(Star), R(4),
@@ -122,7 +141,10 @@ bytecodes: [
 ]
 constant pool: [
   TUPLE2_TYPE,
-  TUPLE2_TYPE,
+  SYMBOL_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],
 ]
 handlers: [
 ]