[nci] Modify Construct node layouts

Prior to this CL, the construct node layout was:

 {target, args..., new_target}

The new layout is:

 {target, new_target, args..., feedback_vector}

Having new_target at index 1 brings it closer to call node layout,
which is now identical except that it has receiver at index 1. The new
feedback vector input will be needed for NCI code.

Affected node kinds are:

- JSConstruct
- JSConstructWithArrayLike
- JSConstructWithSpread
- JSConstructForwardVarargs (just the new_target position change)

Bug: v8:8888
Change-Id: I4c68a0901d01e8862fd276c8a858799d5f4ff024
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2278475
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68692}

src/compiler/bytecode-graph-builder.cc

@@ -2587,10 +2587,12 @@ Node* const* BytecodeGraphBuilder::GetConstructArgumentsFromRegister(
   int cursor = 0;
 
   STATIC_ASSERT(JSConstructNode::TargetIndex() == 0);
-  STATIC_ASSERT(JSConstructNode::FirstArgumentIndex() == 1);
-  STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
+  STATIC_ASSERT(JSConstructNode::NewTargetIndex() == 1);
+  STATIC_ASSERT(JSConstructNode::FirstArgumentIndex() == 2);
+  STATIC_ASSERT(JSConstructNode::kFeedbackVectorIsLastInput);
 
   all[cursor++] = target;
+  all[cursor++] = new_target;
 
   // The function arguments are in consecutive registers.
   int arg_base = first_arg.index();
@@ -2599,7 +2601,7 @@ Node* const* BytecodeGraphBuilder::GetConstructArgumentsFromRegister(
         environment()->LookupRegister(interpreter::Register(arg_base + i));
   }
 
-  all[cursor++] = new_target;
+  all[cursor++] = feedback_vector_node();
 
   DCHECK_EQ(cursor, arity);
   return all;

src/compiler/js-generic-lowering.cc

@@ -737,6 +737,7 @@ bool CollectCallAndConstructFeedback(JSHeapBroker* broker) {
 
 }  // namespace
 
+// TODO(jgruber,v8:8888): Should this collect feedback?
 void JSGenericLowering::LowerJSConstructForwardVarargs(Node* node) {
   ConstructForwardVarargsParameters p =
       ConstructForwardVarargsParametersOf(node->op());
@@ -748,11 +749,8 @@ void JSGenericLowering::LowerJSConstructForwardVarargs(Node* node) {
   Node* stub_code = jsgraph()->HeapConstant(callable.code());
   Node* stub_arity = jsgraph()->Int32Constant(arg_count);
   Node* start_index = jsgraph()->Uint32Constant(p.start_index());
-  Node* new_target = node->InputAt(arg_count + 1);
   Node* receiver = jsgraph()->UndefinedConstant();
-  node->RemoveInput(arg_count + 1);  // Drop new target.
   node->InsertInput(zone(), 0, stub_code);
-  node->InsertInput(zone(), 2, new_target);
   node->InsertInput(zone(), 3, stub_arity);
   node->InsertInput(zone(), 4, start_index);
   node->InsertInput(zone(), 5, receiver);
@@ -776,22 +774,16 @@ void JSGenericLowering::LowerJSConstruct(Node* node) {
         Builtins::CallableFor(isolate(), Builtins::kConstruct_WithFeedback);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    Node* new_target = n.new_target();
     Node* stub_arity = jsgraph()->Int32Constant(arg_count);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
     Node* receiver = jsgraph()->UndefinedConstant();
-    node->RemoveInput(n.NewTargetIndex());
     // Register argument inputs are followed by stack argument inputs (such as
     // feedback_vector). Both are listed in ascending order. Note that
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
-    node->InsertInput(zone(), arg_count + 1, feedback_vector);
     node->InsertInput(zone(), 0, stub_code);
-    node->InsertInput(zone(), 2, new_target);
     node->InsertInput(zone(), 3, stub_arity);
     node->InsertInput(zone(), 4, slot);
     node->InsertInput(zone(), 5, receiver);
@@ -807,11 +799,9 @@ void JSGenericLowering::LowerJSConstruct(Node* node) {
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
     Node* stub_arity = jsgraph()->Int32Constant(arg_count);
-    Node* new_target = n.new_target();
     Node* receiver = jsgraph()->UndefinedConstant();
-    node->RemoveInput(n.NewTargetIndex());
+    node->RemoveInput(n.FeedbackVectorIndex());
     node->InsertInput(zone(), 0, stub_code);
-    node->InsertInput(zone(), 2, new_target);
     node->InsertInput(zone(), 3, stub_arity);
     node->InsertInput(zone(), 4, receiver);
 
@@ -842,23 +832,16 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
     Node* receiver = jsgraph()->UndefinedConstant();
-    Node* arguments_list = n.Argument(0);
-    Node* new_target = n.new_target();
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
 
-    node->InsertInput(zone(), 0, stub_code);
-    node->ReplaceInput(2, new_target);
-    node->ReplaceInput(3, arguments_list);
     // Register argument inputs are followed by stack argument inputs (such as
     // feedback_vector). Both are listed in ascending order. Note that
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
+    node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 4, slot);
     node->InsertInput(zone(), 5, receiver);
-    node->InsertInput(zone(), 6, feedback_vector);
 
     // After: {code, target, new_target, arguments_list, slot, receiver,
     // vector}.
@@ -872,11 +855,8 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
     Node* receiver = jsgraph()->UndefinedConstant();
-    Node* arguments_list = n.Argument(0);
-    Node* new_target = n.new_target();
+    node->RemoveInput(n.FeedbackVectorIndex());
     node->InsertInput(zone(), 0, stub_code);
-    node->ReplaceInput(2, new_target);
-    node->ReplaceInput(3, arguments_list);
     node->InsertInput(zone(), 4, receiver);
 
     // After: {code, target, new_target, arguments_list, receiver}.
@@ -905,14 +885,11 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
 
     // The single available register is needed for `slot`, thus `spread` remains
     // on the stack here.
     Node* stub_arity = jsgraph()->Int32Constant(arg_count - kTheSpread);
-    Node* new_target = node->RemoveInput(n.NewTargetIndex());
     Node* receiver = jsgraph()->UndefinedConstant();
 
     // Register argument inputs are followed by stack argument inputs (such as
@@ -920,9 +897,7 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
     // the receiver is implicitly placed on the stack and is thus inserted
     // between explicitly-specified register and stack arguments.
     // TODO(jgruber): Implement a simpler way to specify these mutations.
-    node->InsertInput(zone(), arg_count + 1, feedback_vector);
     node->InsertInput(zone(), 0, stub_code);
-    node->InsertInput(zone(), 2, new_target);
     node->InsertInput(zone(), 3, stub_arity);
     node->InsertInput(zone(), 4, slot);
     node->InsertInput(zone(), 5, receiver);
@@ -941,13 +916,11 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
     // We pass the spread in a register, not on the stack.
     Node* stub_arity = jsgraph()->Int32Constant(arg_count - kTheSpread);
     Node* receiver = jsgraph()->UndefinedConstant();
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    DCHECK(n.NewTargetIndex() > n.LastArgumentIndex());
-    Node* new_target = node->RemoveInput(n.NewTargetIndex());
+    DCHECK(n.FeedbackVectorIndex() > n.LastArgumentIndex());
+    node->RemoveInput(n.FeedbackVectorIndex());
     Node* spread = node->RemoveInput(n.LastArgumentIndex());
 
     node->InsertInput(zone(), 0, stub_code);
-    node->InsertInput(zone(), 2, new_target);
     node->InsertInput(zone(), 3, stub_arity);
     node->InsertInput(zone(), 4, spread);
     node->InsertInput(zone(), 5, receiver);

src/compiler/js-inlining-heuristic.cc

@@ -629,6 +629,8 @@ void JSInliningHeuristic::CreateOrReuseDispatch(Node* node, Node* callee,
     return;
   }
 
+  STATIC_ASSERT(JSCallOrConstructNode::kHaveIdenticalLayouts);
+
   Node* fallthrough_control = NodeProperties::GetControlInput(node);
   int const num_calls = candidate.num_functions;
 
@@ -655,15 +657,13 @@ void JSInliningHeuristic::CreateOrReuseDispatch(Node* node, Node* callee,
     // to the same node as the {node}'s target input, so that we can later
     // properly inline the JSCreate operations.
     if (node->opcode() == IrOpcode::kJSConstruct) {
-      UNREACHABLE();  // https://crbug.com/v8/10675.
-    }
-    if (node->opcode() == IrOpcode::kJSConstruct && inputs[0] == inputs[1]) {
-      // TODO(jgruber): Is this correct? JSConstruct nodes have the new_target
-      // at the last index, not at index 1.
-      STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-      inputs[1] = target;
+      // TODO(jgruber, v8:10675): This branch seems unreachable.
+      JSConstructNode n(node);
+      if (inputs[n.TargetIndex()] == inputs[n.NewTargetIndex()]) {
+        inputs[n.NewTargetIndex()] = target;
+      }
     }
-    inputs[0] = target;
+    inputs[JSCallOrConstructNode::TargetIndex()] = target;
     inputs[input_count - 1] = if_successes[i];
     calls[i] = if_successes[i] =
         graph()->NewNode(node->op(), input_count, inputs);

src/compiler/js-inlining.cc

@@ -51,8 +51,7 @@ class JSCallAccessor {
   }
 
   Node* target() const {
-    STATIC_ASSERT(JSCallNode::TargetIndex() == JSConstructNode::TargetIndex());
-    return call_->InputAt(JSCallNode::TargetIndex());
+    return call_->InputAt(JSCallOrConstructNode::TargetIndex());
   }
 
   Node* receiver() const {
@@ -65,7 +64,7 @@ class JSCallAccessor {
     return NodeProperties::GetFrameStateInput(call_);
   }
 
-  int formal_arguments() const {
+  int argument_count() const {
     return (call_->opcode() == IrOpcode::kJSCall)
                ? JSCallNode{call_}.ArgumentCount()
                : JSConstructNode{call_}.ArgumentCount();
@@ -85,6 +84,8 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
                                 Node* frame_state, Node* start, Node* end,
                                 Node* exception_target,
                                 const NodeVector& uncaught_subcalls) {
+  JSCallAccessor c(call);
+
   // The scheduler is smart enough to place our code; we just ensure {control}
   // becomes the control input of the start of the inlinee, and {effect} becomes
   // the effect input of the start of the inlinee.
@@ -98,16 +99,11 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
   int const inlinee_context_index =
       static_cast<int>(start->op()->ValueOutputCount()) - 1;
 
-  // {inliner_inputs} counts JSFunction, receiver, arguments, but not
-  // new target value, argument count, context, effect or control.
-  // TODO(jgruber): Refactor this once JSConstructNode is implemented.
-  STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-  STATIC_ASSERT(JSConstructNode::kExtraInputCount == 2);
-  const bool inliner_is_call = call->opcode() == IrOpcode::kJSCall;
-  const int inliner_inputs = inliner_is_call
-                                 ? call->op()->ValueInputCount() -
-                                       JSCallNode::kFeedbackVectorInputCount
-                                 : call->op()->ValueInputCount();
+  // {inliner_inputs} counts the target, receiver/new_target, and arguments; but
+  // not feedback vector, context, effect or control.
+  const int inliner_inputs = c.argument_count() +
+                             JSCallOrConstructNode::kExtraInputCount -
+                             JSCallOrConstructNode::kFeedbackVectorInputCount;
   // Iterate over all uses of the start node.
   for (Edge edge : start->use_edges()) {
     Node* use = edge.from();
@@ -124,10 +120,7 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
           Replace(use, new_target);
         } else if (index == inlinee_arity_index) {
           // The projection is requesting the number of arguments.
-          STATIC_ASSERT(JSCallNode::kExtraInputCount ==
-                        JSCallNode::kFeedbackVectorInputCount + 2);
-          STATIC_ASSERT(JSConstructNode::kExtraInputCount == 2);
-          Replace(use, jsgraph()->Constant(inliner_inputs - 2));
+          Replace(use, jsgraph()->Constant(c.argument_count()));
         } else if (index == inlinee_context_index) {
           // The projection is requesting the inlinee function context.
           Replace(use, context);
@@ -245,7 +238,7 @@ Node* JSInliner::CreateArtificialFrameState(Node* node, Node* outer_frame_state,
                                             SharedFunctionInfoRef shared,
                                             Node* context) {
   const int parameter_count_with_receiver =
-      parameter_count + JSCallNode::kReceiverInputCount;
+      parameter_count + JSCallOrConstructNode::kReceiverOrNewTargetInputCount;
   const FrameStateFunctionInfo* state_info =
       common()->CreateFrameStateFunctionInfo(
           frame_state_type, parameter_count_with_receiver, 0, shared.object());
@@ -255,17 +248,11 @@ Node* JSInliner::CreateArtificialFrameState(Node* node, Node* outer_frame_state,
   const Operator* op0 = common()->StateValues(0, SparseInputMask::Dense());
   Node* node0 = graph()->NewNode(op0);
 
-  // Note how we refer to 'receiver' even though `node` may be a
-  // JSConstruct node. That's because the layout of construct nodes
-  // has been modified to look like call nodes in `ReduceJSCall`; the
-  // `new_target` has been put into the receiver slot.
-  // TODO(jgruber): Either unify the layout of these two nodes, or
-  // refactor this to be more expected in some way.
-  STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
   NodeVector params(local_zone_);
-  params.push_back(node->InputAt(JSCallNode::ReceiverIndex()));
+  params.push_back(
+      node->InputAt(JSCallOrConstructNode::ReceiverOrNewTargetIndex()));
   for (int i = 0; i < parameter_count; i++) {
-    params.push_back(node->InputAt(JSCallNode::ArgumentIndex(i)));
+    params.push_back(node->InputAt(JSCallOrConstructNode::ArgumentIndex(i)));
   }
   const Operator* op_param = common()->StateValues(
       static_cast<int>(params.size()), SparseInputMask::Dense());
@@ -273,7 +260,7 @@ Node* JSInliner::CreateArtificialFrameState(Node* node, Node* outer_frame_state,
       op_param, static_cast<int>(params.size()), &params.front());
   if (context == nullptr) context = jsgraph()->UndefinedConstant();
   return graph()->NewNode(op, params_node, node0, node0, context,
-                          node->InputAt(JSCallNode::TargetIndex()),
+                          node->InputAt(JSCallOrConstructNode::TargetIndex()),
                           outer_frame_state);
 }
 
@@ -293,13 +280,12 @@ bool NeedsImplicitReceiver(SharedFunctionInfoRef shared_info) {
 base::Optional<SharedFunctionInfoRef> JSInliner::DetermineCallTarget(
     Node* node) {
   DCHECK(IrOpcode::IsInlineeOpcode(node->opcode()));
-  HeapObjectMatcher match(node->InputAt(JSCallNode::TargetIndex()));
+  HeapObjectMatcher match(node->InputAt(JSCallOrConstructNode::TargetIndex()));
 
   // This reducer can handle both normal function calls as well a constructor
   // calls whenever the target is a constant function object, as follows:
-  //  - JSCall(target:constant, receiver, args...)
-  //  - JSConstruct(target:constant, args..., new.target)
-  STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
+  //  - JSCall(target:constant, receiver, args..., vector)
+  //  - JSConstruct(target:constant, new.target, args..., vector)
   if (match.HasValue() && match.Ref(broker()).IsJSFunction()) {
     JSFunctionRef function = match.Ref(broker()).AsJSFunction();
 
@@ -325,9 +311,9 @@ base::Optional<SharedFunctionInfoRef> JSInliner::DetermineCallTarget(
 
   // This reducer can also handle calls where the target is statically known to
   // be the result of a closure instantiation operation, as follows:
-  //  - JSCall(JSCreateClosure[shared](context), receiver, args...)
-  //  - JSConstruct(JSCreateClosure[shared](context), args..., new.target)
-  STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
+  //  - JSCall(JSCreateClosure[shared](context), receiver, args..., vector)
+  //  - JSConstruct(JSCreateClosure[shared](context),
+  //                new.target, args..., vector)
   if (match.IsJSCreateClosure()) {
     CreateClosureParameters const& p = CreateClosureParametersOf(match.op());
     FeedbackCellRef cell(broker(), p.feedback_cell());
@@ -348,7 +334,7 @@ base::Optional<SharedFunctionInfoRef> JSInliner::DetermineCallTarget(
 FeedbackVectorRef JSInliner::DetermineCallContext(Node* node,
                                                   Node** context_out) {
   DCHECK(IrOpcode::IsInlineeOpcode(node->opcode()));
-  HeapObjectMatcher match(node->InputAt(JSCallNode::TargetIndex()));
+  HeapObjectMatcher match(node->InputAt(JSCallOrConstructNode::TargetIndex()));
 
   if (match.HasValue() && match.Ref(broker()).IsJSFunction()) {
     JSFunctionRef function = match.Ref(broker()).AsJSFunction();
@@ -515,13 +501,10 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
 
   // Inline {JSConstruct} requires some additional magic.
   if (node->opcode() == IrOpcode::kJSConstruct) {
+    STATIC_ASSERT(JSCallOrConstructNode::kHaveIdenticalLayouts);
     JSConstructNode n(node);
-    // Swizzle the inputs of the {JSConstruct} node to look like inputs to a
-    // normal {JSCall} node so that the rest of the inlining machinery
-    // behaves as if we were dealing with a regular function invocation.
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    new_target = node->RemoveInput(n.NewTargetIndex());
-    node->InsertInput(graph()->zone(), JSCallNode::ReceiverIndex(), new_target);
+
+    new_target = n.new_target();
 
     // Insert nodes around the call that model the behavior required for a
     // constructor dispatch (allocate implicit receiver and check return value).
@@ -620,9 +603,9 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
   // to the call.
   int parameter_count = shared_info->internal_formal_parameter_count();
   DCHECK_EQ(parameter_count, start->op()->ValueOutputCount() - 5);
-  if (call.formal_arguments() != parameter_count) {
+  if (call.argument_count() != parameter_count) {
     frame_state = CreateArtificialFrameState(
-        node, frame_state, call.formal_arguments(), BailoutId::None(),
+        node, frame_state, call.argument_count(), BailoutId::None(),
         FrameStateType::kArgumentsAdaptor, *shared_info);
   }
 

src/compiler/js-operator.cc

@@ -864,8 +864,10 @@ const Operator* JSOperatorBuilder::Construct(uint32_t arity,
 
 const Operator* JSOperatorBuilder::ConstructWithArrayLike(
     CallFrequency const& frequency, FeedbackSource const& feedback) {
-  static constexpr uint32_t arity = 3;
-  ConstructParameters parameters(arity, frequency, feedback);
+  static constexpr int kTheArrayLikeObject = 1;
+  ConstructParameters parameters(
+      JSConstructWithArrayLikeNode::ArityForArgc(kTheArrayLikeObject),
+      frequency, feedback);
   return new (zone()) Operator1<ConstructParameters>(  // --
       IrOpcode::kJSConstructWithArrayLike,             // opcode
       Operator::kNoProperties,                         // properties

src/compiler/js-operator.h

@@ -147,7 +147,7 @@ class ConstructParameters final {
  public:
   // A separate declaration to get around circular declaration dependencies.
   // Checked to equal JSConstructNode::kExtraInputCount below.
-  static constexpr int kExtraConstructInputCount = 2;
+  static constexpr int kExtraConstructInputCount = 3;
 
   ConstructParameters(uint32_t arity, CallFrequency const& frequency,
                       FeedbackSource const& feedback)
@@ -1222,44 +1222,56 @@ namespace js_node_wrapper_utils {
 TNode<Oddball> UndefinedConstant(JSGraph* jsgraph);
 }  // namespace js_node_wrapper_utils
 
-template <int kOpcode>
-class JSCallNodeBase final : public JSNodeWrapperBase {
+class JSCallOrConstructNode : public JSNodeWrapperBase {
  public:
-  explicit constexpr JSCallNodeBase(Node* node) : JSNodeWrapperBase(node) {
-    CONSTEXPR_DCHECK(node->opcode() == kOpcode);
-  }
-
-  const CallParameters& Parameters() const {
-    return CallParametersOf(node()->op());
+  explicit constexpr JSCallOrConstructNode(Node* node)
+      : JSNodeWrapperBase(node) {
+    CONSTEXPR_DCHECK(node->opcode() == IrOpcode::kJSCall ||
+                     node->opcode() == IrOpcode::kJSCallWithArrayLike ||
+                     node->opcode() == IrOpcode::kJSCallWithSpread ||
+                     node->opcode() == IrOpcode::kJSConstruct ||
+                     node->opcode() == IrOpcode::kJSConstructWithArrayLike ||
+                     node->opcode() == IrOpcode::kJSConstructWithSpread);
   }
 
 #define INPUTS(V)              \
   V(Target, target, 0, Object) \
-  V(Receiver, receiver, 1, Object)
+  V(ReceiverOrNewTarget, receiver_or_new_target, 1, Object)
   INPUTS(DEFINE_INPUT_ACCESSORS)
 #undef INPUTS
 
   // Besides actual arguments, JSCall nodes (and variants) also take the
   // following. Note that we rely on the fact that all variants (JSCall,
-  // JSCallWithArrayLike, JSCallWithSpread) have the same underlying node
-  // layout.
+  // JSCallWithArrayLike, JSCallWithSpread, JSConstruct,
+  // JSConstructWithArrayLike, JSConstructWithSpread) have the same underlying
+  // node layout.
   static constexpr int kTargetInputCount = 1;
-  static constexpr int kReceiverInputCount = 1;
+  static constexpr int kReceiverOrNewTargetInputCount = 1;
   static constexpr int kFeedbackVectorInputCount = 1;
-  static constexpr int kExtraInputCount =
-      kTargetInputCount + kReceiverInputCount + kFeedbackVectorInputCount;
+  static constexpr int kExtraInputCount = kTargetInputCount +
+                                          kReceiverOrNewTargetInputCount +
+                                          kFeedbackVectorInputCount;
   STATIC_ASSERT(kExtraInputCount == CallParameters::kExtraCallInputCount);
+  STATIC_ASSERT(kExtraInputCount ==
+                ConstructParameters::kExtraConstructInputCount);
 
   // Just for static asserts for spots that rely on node layout.
   static constexpr bool kFeedbackVectorIsLastInput = true;
 
-  // This is the arity fed into CallArguments.
+  // Some spots rely on the fact that call and construct variants have the same
+  // layout.
+  static constexpr bool kHaveIdenticalLayouts = true;
+
+  // This is the arity fed into Call/ConstructArguments.
   static constexpr int ArityForArgc(int parameters) {
     return parameters + kExtraInputCount;
   }
 
-  static constexpr int FirstArgumentIndex() { return ReceiverIndex() + 1; }
+  static constexpr int FirstArgumentIndex() {
+    return ReceiverOrNewTargetIndex() + 1;
+  }
   static constexpr int ArgumentIndex(int i) { return FirstArgumentIndex() + i; }
+
   TNode<Object> Argument(int i) const {
     DCHECK_LT(i, ArgumentCount());
     return TNode<Object>::UncheckedCast(
@@ -1280,13 +1292,9 @@ class JSCallNodeBase final : public JSNodeWrapperBase {
   TNode<Object> ArgumentOrUndefined(int i, JSGraph* jsgraph) const {
     return ArgumentOr(i, js_node_wrapper_utils::UndefinedConstant(jsgraph));
   }
-  int ArgumentCount() const {
-    // Note: The count reported by this function depends only on the parameter,
-    // thus adding/removing inputs will not affect it.
-    return Parameters().arity_without_implicit_args();
-  }
+  virtual int ArgumentCount() const = 0;
 
-  static int FeedbackVectorIndexForArgc(int argc) {
+  static constexpr int FeedbackVectorIndexForArgc(int argc) {
     STATIC_ASSERT(kFeedbackVectorIsLastInput);
     return ArgumentIndex(argc - 1) + 1;
   }
@@ -1299,14 +1307,43 @@ class JSCallNodeBase final : public JSNodeWrapperBase {
   }
 };
 
+template <int kOpcode>
+class JSCallNodeBase final : public JSCallOrConstructNode {
+ public:
+  explicit constexpr JSCallNodeBase(Node* node) : JSCallOrConstructNode(node) {
+    CONSTEXPR_DCHECK(node->opcode() == kOpcode);
+  }
+
+  const CallParameters& Parameters() const {
+    return CallParametersOf(node()->op());
+  }
+
+#define INPUTS(V)              \
+  V(Target, target, 0, Object) \
+  V(Receiver, receiver, 1, Object)
+  INPUTS(DEFINE_INPUT_ACCESSORS)
+#undef INPUTS
+
+  static constexpr int kReceiverInputCount = 1;
+  STATIC_ASSERT(kReceiverInputCount ==
+                JSCallOrConstructNode::kReceiverOrNewTargetInputCount);
+
+  int ArgumentCount() const override {
+    // Note: The count reported by this function depends only on the parameter,
+    // thus adding/removing inputs will not affect it.
+    return Parameters().arity_without_implicit_args();
+  }
+};
+
 using JSCallNode = JSCallNodeBase<IrOpcode::kJSCall>;
 using JSCallWithSpreadNode = JSCallNodeBase<IrOpcode::kJSCallWithSpread>;
 using JSCallWithArrayLikeNode = JSCallNodeBase<IrOpcode::kJSCallWithArrayLike>;
 
 template <int kOpcode>
-class JSConstructNodeBase final : public JSNodeWrapperBase {
+class JSConstructNodeBase final : public JSCallOrConstructNode {
  public:
-  explicit constexpr JSConstructNodeBase(Node* node) : JSNodeWrapperBase(node) {
+  explicit constexpr JSConstructNodeBase(Node* node)
+      : JSCallOrConstructNode(node) {
     CONSTEXPR_DCHECK(node->opcode() == kOpcode);
   }
 
@@ -1314,66 +1351,21 @@ class JSConstructNodeBase final : public JSNodeWrapperBase {
     return ConstructParametersOf(node()->op());
   }
 
-#define INPUTS(V) V(Target, target, 0, Object)
+#define INPUTS(V)              \
+  V(Target, target, 0, Object) \
+  V(NewTarget, new_target, 1, Object)
   INPUTS(DEFINE_INPUT_ACCESSORS)
 #undef INPUTS
 
-  // Besides actual arguments, JSConstruct nodes (and variants) also take the
-  // following. Note that we rely on the fact that all variants (JSConstruct,
-  // JSConstructWithArrayLike, JSConstructWithSpread) have the same underlying
-  // node layout.
-  static constexpr int kTargetInputCount = 1;
   static constexpr int kNewTargetInputCount = 1;
-  static constexpr int kExtraInputCount =
-      kTargetInputCount + kNewTargetInputCount;
-  STATIC_ASSERT(kExtraInputCount ==
-                ConstructParameters::kExtraConstructInputCount);
-
-  // Just for static asserts for spots that rely on node layout.
-  static constexpr bool kNewTargetIsLastInput = true;
+  STATIC_ASSERT(kNewTargetInputCount ==
+                JSCallOrConstructNode::kReceiverOrNewTargetInputCount);
 
-  // This is the arity fed into ConstructArguments.
-  static constexpr int ArityForArgc(int parameters) {
-    return parameters + kExtraInputCount;
-  }
-
-  static constexpr int FirstArgumentIndex() { return TargetIndex() + 1; }
-  static constexpr int ArgumentIndex(int i) { return FirstArgumentIndex() + i; }
-  TNode<Object> Argument(int i) const {
-    DCHECK_LT(i, ArgumentCount());
-    return TNode<Object>::UncheckedCast(
-        NodeProperties::GetValueInput(node(), ArgumentIndex(i)));
-  }
-  int LastArgumentIndex() const {
-    DCHECK_GT(ArgumentCount(), 0);
-    return ArgumentIndex(ArgumentCount() - 1);
-  }
-  TNode<Object> LastArgument() const {
-    DCHECK_GT(ArgumentCount(), 0);
-    return Argument(ArgumentCount() - 1);
-  }
-  TNode<Object> ArgumentOr(int i, Node* default_value) const {
-    return i < ArgumentCount() ? Argument(i)
-                               : TNode<Object>::UncheckedCast(default_value);
-  }
-  TNode<Object> ArgumentOrUndefined(int i, JSGraph* jsgraph) const {
-    return ArgumentOr(i, js_node_wrapper_utils::UndefinedConstant(jsgraph));
-  }
   int ArgumentCount() const {
-    // Note: The count reported by this function remains static even if the node
-    // is currently being modified.
+    // Note: The count reported by this function depends only on the parameter,
+    // thus adding/removing inputs will not affect it.
     return Parameters().arity_without_implicit_args();
   }
-
-  static int NewTargetIndexForArgc(int argc) {
-    STATIC_ASSERT(kNewTargetIsLastInput);
-    return ArgumentIndex(argc - 1) + 1;
-  }
-  int NewTargetIndex() const { return NewTargetIndexForArgc(ArgumentCount()); }
-  TNode<Object> new_target() const {
-    return TNode<Object>::UncheckedCast(
-        NodeProperties::GetValueInput(node(), NewTargetIndex()));
-  }
 };
 
 using JSConstructNode = JSConstructNodeBase<IrOpcode::kJSConstruct>;

src/compiler/js-typed-lowering.cc

@@ -1526,20 +1526,23 @@ void ReduceBuiltin(JSGraph* jsgraph, Node* node, int builtin_index, int arity,
   // The logic contained here is mirrored in Builtins::Generate_Adaptor.
   // Keep these in sync.
 
-  STATIC_ASSERT(JSCallNode::TargetIndex() == JSConstructNode::TargetIndex());
-  Node* target = node->InputAt(JSCallNode::TargetIndex());
+  Node* target = node->InputAt(JSCallOrConstructNode::TargetIndex());
 
+  // Unify representations between construct and call nodes. For construct
+  // nodes, the receiver is undefined. For call nodes, the new_target is
+  // undefined.
   Node* new_target;
   Zone* zone = jsgraph->zone();
   if (node->opcode() == IrOpcode::kJSConstruct) {
-    // Unify representations between construct and call nodes.
-    // Remove new target and add receiver.
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    new_target = node->RemoveInput(JSConstructNode{node}.NewTargetIndex());
-    node->InsertInput(zone, JSCallNode::ReceiverIndex(),
-                      jsgraph->UndefinedConstant());
+    STATIC_ASSERT(JSCallNode::ReceiverIndex() ==
+                  JSConstructNode::NewTargetIndex());
+    new_target = JSConstructNode{node}.new_target();
+    node->ReplaceInput(JSConstructNode::NewTargetIndex(),
+                       jsgraph->UndefinedConstant());
+    node->RemoveInput(JSConstructNode{node}.FeedbackVectorIndex());
   } else {
     new_target = jsgraph->UndefinedConstant();
+    node->RemoveInput(JSCallNode{node}.FeedbackVectorIndex());
   }
 
   // CPP builtins are implemented in C++, and we can inline it.
@@ -1603,7 +1606,6 @@ Reduction JSTypedLowering::ReduceJSConstructForwardVarargs(Node* node) {
   int const start_index = static_cast<int>(p.start_index());
   Node* target = NodeProperties::GetValueInput(node, 0);
   Type target_type = NodeProperties::GetType(target);
-  Node* new_target = NodeProperties::GetValueInput(node, arity + 1);
 
   // Check if {target} is a JSFunction.
   if (target_type.IsHeapConstant() &&
@@ -1613,10 +1615,8 @@ Reduction JSTypedLowering::ReduceJSConstructForwardVarargs(Node* node) {
     if (!function.map().is_constructor()) return NoChange();
     // Patch {node} to an indirect call via ConstructFunctionForwardVarargs.
     Callable callable = CodeFactory::ConstructFunctionForwardVarargs(isolate());
-    node->RemoveInput(arity + 1);
     node->InsertInput(graph()->zone(), 0,
                       jsgraph()->HeapConstant(callable.code()));
-    node->InsertInput(graph()->zone(), 2, new_target);
     node->InsertInput(graph()->zone(), 3, jsgraph()->Constant(arity));
     node->InsertInput(graph()->zone(), 4, jsgraph()->Constant(start_index));
     node->InsertInput(graph()->zone(), 5, jsgraph()->UndefinedConstant());
@@ -1636,7 +1636,6 @@ Reduction JSTypedLowering::ReduceJSConstruct(Node* node) {
   int const arity = p.arity_without_implicit_args();
   Node* target = n.target();
   Type target_type = NodeProperties::GetType(target);
-  Node* new_target = n.new_target();
 
   // Check if {target} is a known JSFunction.
   if (target_type.IsHeapConstant() &&
@@ -1657,10 +1656,10 @@ Reduction JSTypedLowering::ReduceJSConstruct(Node* node) {
                  use_builtin_construct_stub
                      ? BUILTIN_CODE(isolate(), JSBuiltinsConstructStub)
                      : BUILTIN_CODE(isolate(), JSConstructStubGeneric));
-    STATIC_ASSERT(JSConstructNode::kNewTargetIsLastInput);
-    node->RemoveInput(n.NewTargetIndex());
+    STATIC_ASSERT(JSConstructNode::TargetIndex() == 0);
+    STATIC_ASSERT(JSConstructNode::NewTargetIndex() == 1);
+    node->RemoveInput(n.FeedbackVectorIndex());
     node->InsertInput(graph()->zone(), 0, jsgraph()->Constant(code));
-    node->InsertInput(graph()->zone(), 2, new_target);
     node->InsertInput(graph()->zone(), 3, jsgraph()->Constant(arity));
     node->InsertInput(graph()->zone(), 4, jsgraph()->UndefinedConstant());
     node->InsertInput(graph()->zone(), 5, jsgraph()->UndefinedConstant());
@@ -1780,10 +1779,9 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
     CallDescriptor::Flags flags = CallDescriptor::kNeedsFrameState;
     Node* new_target = jsgraph()->UndefinedConstant();
 
-    // The node will change operators, remove the feedback vector.
-    node->RemoveInput(n.FeedbackVectorIndex());
-
     if (NeedsArgumentAdaptorFrame(*shared, arity)) {
+      node->RemoveInput(n.FeedbackVectorIndex());
+
       // Check if it's safe to skip the arguments adaptor for {shared},
       // that is whether the target function anyways cannot observe the
       // actual arguments. Details can be found in this document at
@@ -1843,12 +1841,14 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
       auto call_descriptor = Linkage::GetStubCallDescriptor(
           graph()->zone(), descriptor, 1 + arity, flags);
       Node* stub_code = jsgraph()->HeapConstant(callable.code());
+      node->RemoveInput(n.FeedbackVectorIndex());
       node->InsertInput(graph()->zone(), 0, stub_code);  // Code object.
       node->InsertInput(graph()->zone(), 2, new_target);
       node->InsertInput(graph()->zone(), 3, jsgraph()->Constant(arity));
       NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
     } else {
       // Patch {node} to a direct call.
+      node->RemoveInput(n.FeedbackVectorIndex());
       node->InsertInput(graph()->zone(), arity + 2, new_target);
       node->InsertInput(graph()->zone(), arity + 3, jsgraph()->Constant(arity));
       NodeProperties::ChangeOp(node,

test/unittests/compiler/js-call-reducer-unittest.cc

@@ -134,10 +134,11 @@ TEST_F(JSCallReducerTest, PromiseConstructorNoArgs) {
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
   Node* frame_state = graph()->start();
+  Node* feedback = UndefinedConstant();
 
-  Node* construct =
-      graph()->NewNode(javascript()->Construct(2), promise, promise, context,
-                       frame_state, effect, control);
+  Node* construct = graph()->NewNode(
+      javascript()->Construct(JSConstructNode::ArityForArgc(0)), promise,
+      promise, feedback, context, frame_state, effect, control);
 
   Reduction r = Reduce(construct);
 
@@ -153,11 +154,12 @@ TEST_F(JSCallReducerTest, PromiseConstructorSubclass) {
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
   Node* frame_state = graph()->start();
+  Node* feedback = UndefinedConstant();
 
   Node* executor = UndefinedConstant();
-  Node* construct =
-      graph()->NewNode(javascript()->Construct(3), promise, executor,
-                       new_target, context, frame_state, effect, control);
+  Node* construct = graph()->NewNode(
+      javascript()->Construct(JSConstructNode::ArityForArgc(1)), promise,
+      new_target, executor, feedback, context, frame_state, effect, control);
 
   Reduction r = Reduce(construct);
 
@@ -171,11 +173,12 @@ TEST_F(JSCallReducerTest, PromiseConstructorBasic) {
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
   Node* frame_state = graph()->start();
+  Node* feedback = UndefinedConstant();
 
   Node* executor = UndefinedConstant();
-  Node* construct =
-      graph()->NewNode(javascript()->Construct(3), promise, executor, promise,
-                       context, frame_state, effect, control);
+  Node* construct = graph()->NewNode(
+      javascript()->Construct(JSConstructNode::ArityForArgc(1)), promise,
+      promise, executor, feedback, context, frame_state, effect, control);
 
   Reduction r = Reduce(construct);
   ASSERT_TRUE(r.Changed());
@@ -190,11 +193,12 @@ TEST_F(JSCallReducerTest, PromiseConstructorWithHook) {
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
   Node* frame_state = graph()->start();
+  Node* feedback = UndefinedConstant();
 
   Node* executor = UndefinedConstant();
-  Node* construct =
-      graph()->NewNode(javascript()->Construct(3), promise, executor, promise,
-                       context, frame_state, effect, control);
+  Node* construct = graph()->NewNode(
+      javascript()->Construct(JSConstructNode::ArityForArgc(1)), promise,
+      promise, executor, feedback, context, frame_state, effect, control);
 
   Protectors::InvalidatePromiseHook(isolate());