[interpreter] Apply Reflect.construct transform in BytecodeGenerator

Bug: v8:11573
Change-Id: Iab32d07443298bcd39c470ad92c5ce6db0a2b580
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2770603
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73550}

src/ast/ast.cc

@@ -891,6 +891,22 @@ bool CompareOperation::IsLiteralCompareNull(Expression** expr) {
          MatchLiteralCompareNull(right_, op(), left_, expr);
 }
 
+void CallBase::ComputeSpreadPosition() {
+  int arguments_length = arguments_.length();
+  int first_spread_index = 0;
+  for (; first_spread_index < arguments_length; first_spread_index++) {
+    if (arguments_.at(first_spread_index)->IsSpread()) break;
+  }
+  SpreadPosition position;
+  if (first_spread_index == arguments_length - 1) {
+    position = kHasFinalSpread;
+  } else {
+    DCHECK_LT(first_spread_index, arguments_length - 1);
+    position = kHasNonFinalSpread;
+  }
+  bit_field_ |= SpreadPositionField::encode(position);
+}
+
 Call::CallType Call::GetCallType() const {
   VariableProxy* proxy = expression()->AsVariableProxy();
   if (proxy != nullptr) {
@@ -935,22 +951,6 @@ Call::CallType Call::GetCallType() const {
   return OTHER_CALL;
 }
 
-void Call::ComputeSpreadPosition() {
-  int arguments_length = arguments_.length();
-  int first_spread_index = 0;
-  for (; first_spread_index < arguments_length; first_spread_index++) {
-    if (arguments_.at(first_spread_index)->IsSpread()) break;
-  }
-  SpreadPosition position;
-  if (first_spread_index == arguments_length - 1) {
-    position = kHasFinalSpread;
-  } else {
-    DCHECK_LT(first_spread_index, arguments_length - 1);
-    position = kHasNonFinalSpread;
-  }
-  bit_field_ |= SpreadPositionField::encode(position);
-}
-
 CaseClause::CaseClause(Zone* zone, Expression* label,
                        const ScopedPtrList<Statement>& statements)
     : label_(label), statements_(statements.ToConstVector(), zone) {}

src/ast/ast.h

@@ -1618,11 +1618,44 @@ class Property final : public Expression {
   Expression* key_;
 };
 
-class Call final : public Expression {
+class CallBase : public Expression {
  public:
   Expression* expression() const { return expression_; }
   const ZonePtrList<Expression>* arguments() const { return &arguments_; }
 
+  enum SpreadPosition { kNoSpread, kHasFinalSpread, kHasNonFinalSpread };
+  SpreadPosition spread_position() const {
+    return SpreadPositionField::decode(bit_field_);
+  }
+
+ protected:
+  CallBase(Zone* zone, NodeType type, Expression* expression,
+           const ScopedPtrList<Expression>& arguments, int pos, bool has_spread)
+      : Expression(pos, type),
+        expression_(expression),
+        arguments_(arguments.ToConstVector(), zone) {
+    DCHECK(type == kCall || type == kCallNew);
+    if (has_spread) {
+      ComputeSpreadPosition();
+    } else {
+      bit_field_ |= SpreadPositionField::encode(kNoSpread);
+    }
+  }
+
+  // Only valid to be called if there is a spread in arguments_.
+  void ComputeSpreadPosition();
+
+  using SpreadPositionField = Expression::NextBitField<SpreadPosition, 2>;
+
+  template <class T, int size>
+  using NextBitField = SpreadPositionField::Next<T, size>;
+
+  Expression* expression_;
+  ZonePtrList<Expression> arguments_;
+};
+
+class Call final : public CallBase {
+ public:
   bool is_possibly_eval() const {
     return IsPossiblyEvalField::decode(bit_field_);
   }
@@ -1635,16 +1668,6 @@ class Call final : public Expression {
     return IsOptionalChainLinkField::decode(bit_field_);
   }
 
-  enum SpreadPosition { kNoSpread, kHasFinalSpread, kHasNonFinalSpread };
-  SpreadPosition spread_position() const {
-    return SpreadPositionField::decode(bit_field_);
-  }
-
-  // TODO(syg): Remove this and its users.
-  bool only_last_arg_is_spread() {
-    return !arguments_.is_empty() && arguments_.last()->IsSpread();
-  }
-
   enum CallType {
     GLOBAL_CALL,
     WITH_CALL,
@@ -1677,63 +1700,35 @@ class Call final : public Expression {
   Call(Zone* zone, Expression* expression,
        const ScopedPtrList<Expression>& arguments, int pos, bool has_spread,
        PossiblyEval possibly_eval, bool optional_chain)
-      : Expression(pos, kCall),
-        expression_(expression),
-        arguments_(arguments.ToConstVector(), zone) {
+      : CallBase(zone, kCall, expression, arguments, pos, has_spread) {
     bit_field_ |=
         IsPossiblyEvalField::encode(possibly_eval == IS_POSSIBLY_EVAL) |
         IsTaggedTemplateField::encode(false) |
-        IsOptionalChainLinkField::encode(optional_chain) |
-        SpreadPositionField::encode(kNoSpread);
-    if (has_spread) ComputeSpreadPosition();
+        IsOptionalChainLinkField::encode(optional_chain);
   }
 
   Call(Zone* zone, Expression* expression,
        const ScopedPtrList<Expression>& arguments, int pos,
        TaggedTemplateTag tag)
-      : Expression(pos, kCall),
-        expression_(expression),
-        arguments_(arguments.ToConstVector(), zone) {
+      : CallBase(zone, kCall, expression, arguments, pos, false) {
     bit_field_ |= IsPossiblyEvalField::encode(false) |
                   IsTaggedTemplateField::encode(true) |
-                  IsOptionalChainLinkField::encode(false) |
-                  SpreadPositionField::encode(kNoSpread);
+                  IsOptionalChainLinkField::encode(false);
   }
 
-  // Only valid to be called if there is a spread in arguments_.
-  void ComputeSpreadPosition();
-
-  using IsPossiblyEvalField = Expression::NextBitField<bool, 1>;
+  using IsPossiblyEvalField = CallBase::NextBitField<bool, 1>;
   using IsTaggedTemplateField = IsPossiblyEvalField::Next<bool, 1>;
   using IsOptionalChainLinkField = IsTaggedTemplateField::Next<bool, 1>;
-  using SpreadPositionField = IsOptionalChainLinkField::Next<SpreadPosition, 2>;
-
-  Expression* expression_;
-  ZonePtrList<Expression> arguments_;
 };
 
-
-class CallNew final : public Expression {
- public:
-  Expression* expression() const { return expression_; }
-  const ZonePtrList<Expression>* arguments() const { return &arguments_; }
-
-  bool only_last_arg_is_spread() {
-    return !arguments_.is_empty() && arguments_.last()->IsSpread();
-  }
-
+class CallNew final : public CallBase {
  private:
   friend class AstNodeFactory;
   friend Zone;
 
   CallNew(Zone* zone, Expression* expression,
-          const ScopedPtrList<Expression>& arguments, int pos)
-      : Expression(pos, kCallNew),
-        expression_(expression),
-        arguments_(arguments.ToConstVector(), zone) {}
-
-  Expression* expression_;
-  ZonePtrList<Expression> arguments_;
+          const ScopedPtrList<Expression>& arguments, int pos, bool has_spread)
+      : CallBase(zone, kCallNew, expression, arguments, pos, has_spread) {}
 };
 
 // The CallRuntime class does not represent any official JavaScript
@@ -3092,8 +3087,9 @@ class AstNodeFactory final {
   }
 
   CallNew* NewCallNew(Expression* expression,
-                      const ScopedPtrList<Expression>& arguments, int pos) {
-    return zone_->New<CallNew>(zone_, expression, arguments, pos);
+                      const ScopedPtrList<Expression>& arguments, int pos,
+                      bool has_spread) {
+    return zone_->New<CallNew>(zone_, expression, arguments, pos, has_spread);
   }
 
   CallRuntime* NewCallRuntime(Runtime::FunctionId id,

src/interpreter/bytecode-generator.cc

@@ -5347,8 +5347,37 @@ void BytecodeGenerator::VisitCallSuper(Call* expr) {
 }
 
 void BytecodeGenerator::VisitCallNew(CallNew* expr) {
-  Register constructor = VisitForRegisterValue(expr->expression());
   RegisterList args = register_allocator()->NewGrowableRegisterList();
+
+  // Load the constructor. It's in the first register in args for ease of
+  // calling %reflect_construct if we have a non-final spread. For all other
+  // cases it is popped before emitting the construct below.
+  VisitAndPushIntoRegisterList(expr->expression(), &args);
+
+  // We compile the new differently depending on the presence of spreads and
+  // their positions.
+  //
+  // If there is only one spread and it is the final argument, there is a
+  // special ConstructWithSpread bytecode.
+  //
+  // If there is a non-final spread, we rewrite calls like
+  //     new ctor(1, ...x, 2)
+  // to
+  //     %reflect_construct(ctor, [1, ...x, 2])
+  const CallNew::SpreadPosition spread_position = expr->spread_position();
+
+  if (spread_position == CallNew::kHasNonFinalSpread) {
+    BuildCreateArrayLiteral(expr->arguments(), nullptr);
+    builder()->SetExpressionPosition(expr);
+    builder()
+        ->StoreAccumulatorInRegister(
+            register_allocator()->GrowRegisterList(&args))
+        .CallJSRuntime(Context::REFLECT_CONSTRUCT_INDEX, args);
+    return;
+  }
+
+  Register constructor = args.first_register();
+  args = args.PopLeft();
   VisitArguments(expr->arguments(), &args);
 
   // The accumulator holds new target which is the same as the
@@ -5357,9 +5386,10 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) {
   builder()->LoadAccumulatorWithRegister(constructor);
 
   int feedback_slot_index = feedback_index(feedback_spec()->AddCallICSlot());
-  if (expr->only_last_arg_is_spread()) {
+  if (spread_position == CallNew::kHasFinalSpread) {
     builder()->ConstructWithSpread(constructor, args, feedback_slot_index);
   } else {
+    DCHECK_EQ(spread_position, CallNew::kNoSpread);
     builder()->Construct(constructor, args, feedback_slot_index);
   }
 }

src/parsing/parser-base.h

@@ -1186,7 +1186,6 @@ class ParserBase {
   BlockT ParseClassStaticBlock(ClassInfo* class_info);
   ObjectLiteralPropertyT ParseObjectPropertyDefinition(
       ParsePropertyInfo* prop_info, bool* has_seen_proto);
-  // TODO(syg): Remove has_spread once SpreadCallNew is removed.
   void ParseArguments(
       ExpressionListT* args, bool* has_spread,
       ParsingArrowHeadFlag maybe_arrow = kCertainlyNotArrowHead);
@@ -3564,11 +3563,7 @@ ParserBase<Impl>::ParseMemberWithPresentNewPrefixesExpression() {
       bool has_spread;
       ParseArguments(&args, &has_spread);
 
-      if (has_spread) {
-        result = impl()->SpreadCallNew(result, args, new_pos);
-      } else {
-        result = factory()->NewCallNew(result, args, new_pos);
-      }
+      result = factory()->NewCallNew(result, args, new_pos, has_spread);
     }
     // The expression can still continue with . or [ after the arguments.
     return ParseMemberExpressionContinuation(result);
@@ -3582,7 +3577,7 @@ ParserBase<Impl>::ParseMemberWithPresentNewPrefixesExpression() {
 
   // NewExpression without arguments.
   ExpressionListT args(pointer_buffer());
-  return factory()->NewCallNew(result, args, new_pos);
+  return factory()->NewCallNew(result, args, new_pos, false);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -3345,19 +3345,6 @@ Expression* Parser::CloseTemplateLiteral(TemplateLiteralState* state, int start,
   }
 }
 
-namespace {
-
-bool OnlyLastArgIsSpread(const ScopedPtrList<Expression>& args) {
-  for (int i = 0; i < args.length() - 1; i++) {
-    if (args.at(i)->IsSpread()) {
-      return false;
-    }
-  }
-  return args.at(args.length() - 1)->IsSpread();
-}
-
-}  // namespace
-
 ArrayLiteral* Parser::ArrayLiteralFromListWithSpread(
     const ScopedPtrList<Expression>& list) {
   // If there's only a single spread argument, a fast path using CallWithSpread
@@ -3374,21 +3361,6 @@ ArrayLiteral* Parser::ArrayLiteralFromListWithSpread(
   return factory()->NewArrayLiteral(list, first_spread, kNoSourcePosition);
 }
 
-Expression* Parser::SpreadCallNew(Expression* function,
-                                  const ScopedPtrList<Expression>& args_list,
-                                  int pos) {
-  // TODO(syg): Handle all spread cases in BytecodeGenerator.
-  if (OnlyLastArgIsSpread(args_list)) {
-    // Handle in BytecodeGenerator.
-    return factory()->NewCallNew(function, args_list, pos);
-  }
-  ScopedPtrList<Expression> args(pointer_buffer());
-  args.Add(function);
-  args.Add(ArrayLiteralFromListWithSpread(args_list));
-
-  return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
-}
-
 void Parser::SetLanguageMode(Scope* scope, LanguageMode mode) {
   v8::Isolate::UseCounterFeature feature;
   if (is_sloppy(mode))

src/parsing/parser.h

@@ -493,8 +493,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   ArrayLiteral* ArrayLiteralFromListWithSpread(
       const ScopedPtrList<Expression>& list);
-  Expression* SpreadCallNew(Expression* function,
-                            const ScopedPtrList<Expression>& args, int pos);
   Expression* RewriteSuperCall(Expression* call_expression);
 
   void SetLanguageMode(Scope* scope, LanguageMode mode);

src/parsing/preparser.h

@@ -662,7 +662,7 @@ class PreParserFactory {
   }
   PreParserExpression NewCallNew(const PreParserExpression& expression,
                                  const PreParserExpressionList& arguments,
-                                 int pos) {
+                                 int pos, bool has_spread) {
     return PreParserExpression::Default();
   }
   PreParserStatement NewReturnStatement(
@@ -1050,10 +1050,6 @@ class PreParser : public ParserBase<PreParser> {
   }
   V8_INLINE void SetAsmModule() {}
 
-  V8_INLINE PreParserExpression
-  SpreadCallNew(const PreParserExpression& function,
-                const PreParserExpressionList& args, int pos);
-
   V8_INLINE void PrepareGeneratorVariables() {}
   V8_INLINE void RewriteAsyncFunctionBody(
       const PreParserScopedStatementList* body, PreParserStatement block,
@@ -1678,12 +1674,6 @@ class PreParser : public ParserBase<PreParser> {
   std::vector<void*> preparse_data_builder_buffer_;
 };
 
-PreParserExpression PreParser::SpreadCallNew(
-    const PreParserExpression& function, const PreParserExpressionList& args,
-    int pos) {
-  return factory()->NewCallNew(function, args, pos);
-}
-
 }  // namespace internal
 }  // namespace v8
 

test/cctest/interpreter/bytecode_expectations/NewAndSpread.golden

@@ -90,7 +90,7 @@ snippet: "
 "
 frame size: 7
 parameter count: 1
-bytecode array length: 110
+bytecode array length: 117
 bytecodes: [
   /*   30 E> */ B(CreateBlockContext), U8(0),
                 B(PushContext), R(1),
@@ -105,35 +105,40 @@ bytecodes: [
                 B(Star3),
                 B(PopContext), R(1),
                 B(Mov), R(4), R(0),
-  /*   89 S> */ B(CreateArrayLiteral), U8(3), U8(0), U8(37),
+  /*   89 S> */ B(CreateEmptyArrayLiteral), U8(0),
                 B(Star3),
-                B(LdaSmi), I8(1),
+                B(LdaZero),
+                B(Star2),
+                B(LdaZero),
+                B(StaInArrayLiteral), R(3), R(2), U8(1),
+                B(Ldar), R(2),
+                B(Inc), U8(3),
                 B(Star2),
-  /*  101 S> */ B(CreateArrayLiteral), U8(4), U8(1), U8(37),
+  /*  101 S> */ B(CreateArrayLiteral), U8(3), U8(4), U8(37),
                 B(Star6),
-  /*  101 E> */ B(GetIterator), R(6), U8(2), U8(4),
+  /*  101 E> */ B(GetIterator), R(6), U8(5), U8(7),
                 B(Mov), R(4), R(1),
                 B(JumpIfJSReceiver), U8(7),
                 B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),
                 B(Star5),
-                B(LdaNamedProperty), R(5), U8(5), U8(6),
+                B(LdaNamedProperty), R(5), U8(4), U8(9),
                 B(Star4),
                 B(CallProperty0), R(4), R(5), U8(15),
                 B(Star6),
                 B(JumpIfJSReceiver), U8(7),
                 B(CallRuntime), U16(Runtime::kThrowIteratorResultNotAnObject), R(6), U8(1),
-                B(LdaNamedProperty), R(6), U8(6), U8(17),
+                B(LdaNamedProperty), R(6), U8(5), U8(17),
                 B(JumpIfToBooleanTrue), U8(18),
-                B(LdaNamedProperty), R(6), U8(7), U8(8),
-                B(StaInArrayLiteral), R(3), R(2), U8(13),
+                B(LdaNamedProperty), R(6), U8(6), U8(11),
+                B(StaInArrayLiteral), R(3), R(2), U8(1),
                 B(Ldar), R(2),
-                B(Inc), U8(12),
+                B(Inc), U8(3),
                 B(Star2),
                 B(JumpLoop), U8(31), I8(0),
                 B(LdaSmi), I8(4),
-                B(StaInArrayLiteral), R(3), R(2), U8(13),
+                B(StaInArrayLiteral), R(3), R(2), U8(1),
                 B(Mov), R(3), R(2),
-                B(CallJSRuntime), U8(%reflect_construct), R(1), U8(2),
+  /*   89 E> */ B(CallJSRuntime), U8(%reflect_construct), R(1), U8(2),
                 B(LdaUndefined),
   /*  116 S> */ B(Return),
 ]
@@ -142,7 +147,6 @@ constant pool: [
   FIXED_ARRAY_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
   ARRAY_BOILERPLATE_DESCRIPTION_TYPE,
-  ARRAY_BOILERPLATE_DESCRIPTION_TYPE,
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["next"],
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["done"],
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["value"],