[nci] Add feedback input to JSCallWithArrayLike and JSCallWithSpread

Like in previous similar CLs, this also adds node wrapper classes for
both.

Bug: v8:8888
Change-Id: I9c83e98e3b665b72b944dec83b8854b9ef2c14a0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2277805
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68683}

src/compiler/bytecode-graph-builder.cc

@@ -155,11 +155,6 @@ class BytecodeGraphBuilder {
   Node** EnsureInputBufferSize(int size);
 
   Node* const* GetCallArgumentsFromRegisters(Node* callee, Node* receiver,
-                                             interpreter::Register first_arg,
-                                             int arg_count);
-  // Same as above, but for call nodes that take a feedback vector input.
-  // TODO(jgruber): Merge with the above.
-  Node* const* GetCallArgumentsFromRegisters1(Node* callee, Node* receiver,
                                               interpreter::Register first_arg,
                                               int arg_count);
   Node* const* ProcessCallVarArgs(ConvertReceiverMode receiver_mode,
@@ -2279,29 +2274,15 @@ void BytecodeGraphBuilder::VisitGetTemplateObject() {
 Node* const* BytecodeGraphBuilder::GetCallArgumentsFromRegisters(
     Node* callee, Node* receiver, interpreter::Register first_arg,
     int arg_count) {
-  int arity = kTargetAndReceiver + arg_count;
-  Node** all = local_zone()->NewArray<Node*>(static_cast<size_t>(arity));
-
-  all[0] = callee;
-  all[1] = receiver;
-
-  // The function arguments are in consecutive registers.
-  int arg_base = first_arg.index();
-  for (int i = 0; i < arg_count; ++i) {
-    all[kTargetAndReceiver + i] =
-        environment()->LookupRegister(interpreter::Register(arg_base + i));
-  }
-
-  return all;
-}
-
-Node* const* BytecodeGraphBuilder::GetCallArgumentsFromRegisters1(
-    Node* callee, Node* receiver, interpreter::Register first_arg,
-    int arg_count) {
   const int arity = JSCallNode::ArityForArgc(arg_count);
   Node** all = local_zone()->NewArray<Node*>(static_cast<size_t>(arity));
   int cursor = 0;
 
+  STATIC_ASSERT(JSCallNode::TargetIndex() == 0);
+  STATIC_ASSERT(JSCallNode::ReceiverIndex() == 1);
+  STATIC_ASSERT(JSCallNode::FirstArgumentIndex() == 2);
+  STATIC_ASSERT(JSCallNode::kFeedbackVectorIsLastInput);
+
   all[cursor++] = callee;
   all[cursor++] = receiver;
 
@@ -2366,8 +2347,8 @@ Node* const* BytecodeGraphBuilder::ProcessCallVarArgs(
     first_arg = interpreter::Register(first_reg.index() + 1);
   }
 
-  Node* const* call_args = GetCallArgumentsFromRegisters1(callee, receiver_node,
-                                                          first_arg, arg_count);
+  Node* const* call_args = GetCallArgumentsFromRegisters(callee, receiver_node,
+                                                         first_arg, arg_count);
   return call_args;
 }
 
@@ -2515,7 +2496,8 @@ void BytecodeGraphBuilder::VisitCallWithSpread() {
   CallFrequency frequency = ComputeCallFrequency(slot_id);
   SpeculationMode speculation_mode = GetSpeculationMode(slot_id);
   const Operator* op = javascript()->CallWithSpread(
-      static_cast<int>(reg_count + 1), frequency, feedback, speculation_mode);
+      JSCallWithSpreadNode::ArityForArgc(arg_count), frequency, feedback,
+      speculation_mode);
 
   JSTypeHintLowering::LoweringResult lowering = TryBuildSimplifiedCall(
       op, args, static_cast<int>(arg_count), feedback.slot);
@@ -2526,7 +2508,7 @@ void BytecodeGraphBuilder::VisitCallWithSpread() {
     node = lowering.value();
   } else {
     DCHECK(!lowering.Changed());
-    node = MakeNode(op, kTargetAndReceiver + arg_count, args);
+    node = MakeNode(op, JSCallWithSpreadNode::ArityForArgc(arg_count), args);
   }
   environment()->BindAccumulator(node, Environment::kAttachFrameState);
 }

src/compiler/js-call-reducer.h

@@ -114,7 +114,7 @@ class V8_EXPORT_PRIVATE JSCallReducer final : public AdvancedReducer {
                                         IterationKind kind);
 
   Reduction ReduceCallOrConstructWithArrayLikeOrSpread(
-      Node* node, int arity, CallFrequency const& frequency,
+      Node* node, int arraylike_or_spread_index, CallFrequency const& frequency,
       FeedbackSource const& feedback, SpeculationMode speculation_mode,
       CallFeedbackRelation feedback_relation);
   Reduction ReduceJSConstruct(Node* node);

src/compiler/js-generic-lowering.cc

@@ -918,6 +918,7 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
   }
 }
 
+// TODO(jgruber,v8:8888): Should this collect feedback?
 void JSGenericLowering::LowerJSCallForwardVarargs(Node* node) {
   CallForwardVarargsParameters p = CallForwardVarargsParametersOf(node->op());
   int const arg_count = static_cast<int>(p.arity() - 2);
@@ -970,80 +971,112 @@ void JSGenericLowering::LowerJSCall(Node* node) {
 }
 
 void JSGenericLowering::LowerJSCallWithArrayLike(Node* node) {
-  CallParameters const& p = CallParametersOf(node->op());
+  JSCallWithArrayLikeNode n(node);
+  CallParameters const& p = n.Parameters();
   const int arg_count = p.arity_without_implicit_args();
+  DCHECK_EQ(arg_count, 1);  // The arraylike object.
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
 
-  DCHECK_EQ(arg_count, 0);
+  static constexpr int kArrayLikeObject = 1;
   static constexpr int kReceiver = 1;
 
   if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
+    const int stack_argument_count = arg_count - kArrayLikeObject + kReceiver;
     Callable callable = Builtins::CallableFor(
         isolate(), Builtins::kCallWithArrayLike_WithFeedback);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
-        zone(), callable.descriptor(), arg_count + kReceiver, flags);
+        zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    Node* receiver = node->InputAt(1);
-    Node* arguments_list = node->InputAt(2);
-    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
+    Node* receiver = n.receiver();
+    Node* arguments_list = n.Argument(0);
+    Node* feedback_vector = n.feedback_vector();
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
-    node->InsertInput(zone(), 0, stub_code);
-    node->ReplaceInput(2, arguments_list);
+
+    // Shuffling inputs.
+    // Before: {target, receiver, arguments_list, vector}.
+
+    node->ReplaceInput(1, arguments_list);
+    node->ReplaceInput(2, feedback_vector);
     node->ReplaceInput(3, receiver);
+
+    // Now: {target, arguments_list, vector, receiver}.
+
+    node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 3, slot);
-    node->InsertInput(zone(), 4, feedback_vector);
+
+    // After: {code, target, arguments_list, slot, vector, receiver}.
+
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   } else {
+    const int stack_argument_count = arg_count - kArrayLikeObject + kReceiver;
     Callable callable = CodeFactory::CallWithArrayLike(isolate());
     auto call_descriptor = Linkage::GetStubCallDescriptor(
-        zone(), callable.descriptor(), arg_count + kReceiver, flags);
+        zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    Node* receiver = node->InputAt(1);
-    Node* arguments_list = node->InputAt(2);
+    Node* receiver = n.receiver();
+    Node* arguments_list = n.Argument(0);
+
+    // Shuffling inputs.
+    // Before: {target, receiver, arguments_list, vector}.
+
+    node->RemoveInput(n.FeedbackVectorIndex());
     node->InsertInput(zone(), 0, stub_code);
     node->ReplaceInput(2, arguments_list);
     node->ReplaceInput(3, receiver);
+
+    // After: {code, target, arguments_list, receiver}.
+
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   }
 }
 
 void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
-  CallParameters const& p = CallParametersOf(node->op());
+  JSCallWithSpreadNode n(node);
+  CallParameters const& p = n.Parameters();
   int const arg_count = p.arity_without_implicit_args();
-  int const spread_index = arg_count + 1;
+  DCHECK_GE(arg_count, 1);  // At least the spread.
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
 
+  static constexpr int kReceiver = 1;
   static constexpr int kTheSpread = 1;
   static constexpr int kMaybeFeedbackVector = 1;
 
   if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
-    const int stack_argument_count = arg_count + kMaybeFeedbackVector;
+    const int stack_argument_count =
+        arg_count - kTheSpread + kReceiver + kMaybeFeedbackVector;
     Callable callable = Builtins::CallableFor(
         isolate(), Builtins::kCallWithSpread_WithFeedback);
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
     Node* stub_code = jsgraph()->HeapConstant(callable.code());
-    Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
     Node* slot = jsgraph()->Int32Constant(p.feedback().index());
 
     // We pass the spread in a register, not on the stack.
     Node* stub_arity = jsgraph()->Int32Constant(arg_count - kTheSpread);
-    Node* spread = node->InputAt(spread_index);
-    node->RemoveInput(spread_index);
 
     // 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);
+
+    // Shuffling inputs.
+    // Before: {target, receiver, ...args, spread, vector}.
+
+    Node* spread = node->RemoveInput(n.LastArgumentIndex());
+
+    // Now: {target, receiver, ...args, vector}.
+
     node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 2, stub_arity);
     node->InsertInput(zone(), 3, spread);
     node->InsertInput(zone(), 4, slot);
+
+    // After: {code, target, arity, spread, slot, receiver, ...args, vector}.
+
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   } else {
-    const int stack_argument_count = arg_count;
+    const int stack_argument_count = arg_count - kTheSpread + kReceiver;
     Callable callable = CodeFactory::CallWithSpread(isolate());
     auto call_descriptor = Linkage::GetStubCallDescriptor(
         zone(), callable.descriptor(), stack_argument_count, flags);
@@ -1051,12 +1084,19 @@ void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
 
     // We pass the spread in a register, not on the stack.
     Node* stub_arity = jsgraph()->Int32Constant(arg_count - kTheSpread);
-    Node* spread = node->InputAt(spread_index);
-    node->RemoveInput(spread_index);
+
+    // Shuffling inputs.
+    // Before: {target, receiver, ...args, spread, vector}.
+
+    node->RemoveInput(n.FeedbackVectorIndex());
+    Node* spread = node->RemoveInput(n.LastArgumentIndex());
 
     node->InsertInput(zone(), 0, stub_code);
     node->InsertInput(zone(), 2, stub_arity);
     node->InsertInput(zone(), 3, spread);
+
+    // After: {code, target, arity, spread, receiver, ...args}.
+
     NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   }
 }

src/compiler/js-intrinsic-lowering.cc

@@ -333,6 +333,7 @@ Reduction JSIntrinsicLowering::ReduceCall(Node* node) {
   int const arity =
       static_cast<int>(CallRuntimeParametersOf(node->op()).arity());
   static constexpr int kTargetAndReceiver = 2;
+  STATIC_ASSERT(JSCallNode::kFeedbackVectorIsLastInput);
   Node* feedback = jsgraph()->UndefinedConstant();
   node->InsertInput(graph()->zone(), arity, feedback);
   NodeProperties::ChangeOp(

src/compiler/js-operator.cc

@@ -28,10 +28,16 @@ constexpr Operator::Properties BinopProperties(Operator::Opcode opcode) {
 
 }  // namespace
 
-TNode<Object> JSCallNode::ArgumentOrUndefined(int i, JSGraph* jsgraph) const {
+template <int kOpcode>
+TNode<Object> JSCallNodeBase<kOpcode>::ArgumentOrUndefined(
+    int i, JSGraph* jsgraph) const {
   return ArgumentOr(i, jsgraph->UndefinedConstant());
 }
 
+template class JSCallNodeBase<IrOpcode::kJSCall>;
+template class JSCallNodeBase<IrOpcode::kJSCallWithSpread>;
+template class JSCallNodeBase<IrOpcode::kJSCallWithArrayLike>;
+
 std::ostream& operator<<(std::ostream& os, CallFrequency const& f) {
   if (f.IsUnknown()) return os << "unknown";
   return os << f.value();
@@ -773,7 +779,7 @@ const Operator* JSOperatorBuilder::Call(
     FeedbackSource const& feedback, ConvertReceiverMode convert_mode,
     SpeculationMode speculation_mode, CallFeedbackRelation feedback_relation) {
   CallParameters parameters(arity, frequency, feedback, convert_mode,
-                            speculation_mode, feedback_relation, true);
+                            speculation_mode, feedback_relation);
   return new (zone()) Operator1<CallParameters>(   // --
       IrOpcode::kJSCall, Operator::kNoProperties,  // opcode
       "JSCall",                                    // name
@@ -784,12 +790,14 @@ const Operator* JSOperatorBuilder::Call(
 const Operator* JSOperatorBuilder::CallWithArrayLike(
     const CallFrequency& frequency, const FeedbackSource& feedback,
     SpeculationMode speculation_mode, CallFeedbackRelation feedback_relation) {
-  CallParameters parameters(2, frequency, feedback, ConvertReceiverMode::kAny,
-                            speculation_mode, feedback_relation, false);
+  static constexpr int kTheArrayLikeObject = 1;
+  CallParameters parameters(
+      JSCallWithArrayLikeNode::ArityForArgc(kTheArrayLikeObject), frequency,
+      feedback, ConvertReceiverMode::kAny, speculation_mode, feedback_relation);
   return new (zone()) Operator1<CallParameters>(                // --
       IrOpcode::kJSCallWithArrayLike, Operator::kNoProperties,  // opcode
       "JSCallWithArrayLike",                                    // name
-      3, 1, 1, 1, 1, 2,                                         // counts
+      parameters.arity(), 1, 1, 1, 1, 2,                        // counts
       parameters);                                              // parameter
 }
 
@@ -801,7 +809,7 @@ const Operator* JSOperatorBuilder::CallWithSpread(
                  feedback.IsValid());
   CallParameters parameters(arity, frequency, feedback,
                             ConvertReceiverMode::kAny, speculation_mode,
-                            feedback_relation, false);
+                            feedback_relation);
   return new (zone()) Operator1<CallParameters>(             // --
       IrOpcode::kJSCallWithSpread, Operator::kNoProperties,  // opcode
       "JSCallWithSpread",                                    // name

src/compiler/js-operator.h

@@ -218,35 +218,32 @@ std::ostream& operator<<(std::ostream&, CallForwardVarargsParameters const&);
 CallForwardVarargsParameters const& CallForwardVarargsParametersOf(
     Operator const*) V8_WARN_UNUSED_RESULT;
 
-// Part of CallParameters::arity.
-// TODO(jgruber): Fold this into JSCallNode.
-static constexpr int kTargetAndReceiver = 2;
-static constexpr int kTargetAndReceiverAndVector = 3;
-
 // Defines the arity (parameters plus the target and receiver) and the call
 // flags for a JavaScript function call. This is used as a parameter by JSCall,
 // JSCallWithArrayLike and JSCallWithSpread operators.
 class CallParameters final {
  public:
+  // A separate declaration to get around circular declaration dependencies.
+  // Checked to equal JSCallNode::kExtraInputCount below.
+  static constexpr int kExtraCallInputCount = 3;
+
   CallParameters(size_t arity, CallFrequency const& frequency,
                  FeedbackSource const& feedback,
                  ConvertReceiverMode convert_mode,
                  SpeculationMode speculation_mode,
-                 CallFeedbackRelation feedback_relation, bool has_vector)
+                 CallFeedbackRelation feedback_relation)
       : bit_field_(ArityField::encode(arity) |
                    CallFeedbackRelationField::encode(feedback_relation) |
                    SpeculationModeField::encode(speculation_mode) |
                    ConvertReceiverModeField::encode(convert_mode)),
         frequency_(frequency),
-        feedback_(feedback),
-        has_vector_(has_vector) {
+        feedback_(feedback) {
     // CallFeedbackRelation is ignored if the feedback slot is invalid.
     DCHECK_IMPLIES(speculation_mode == SpeculationMode::kAllowSpeculation,
                    feedback.IsValid());
     DCHECK_IMPLIES(!feedback.IsValid(),
                    feedback_relation == CallFeedbackRelation::kUnrelated);
-    DCHECK_GE(arity,
-              has_vector_ ? kTargetAndReceiverAndVector : kTargetAndReceiver);
+    DCHECK_GE(arity, kExtraCallInputCount);
     DCHECK(is_int32(arity));
   }
 
@@ -254,8 +251,7 @@ class CallParameters final {
   // without extra args in CallParameters.
   size_t arity() const { return ArityField::decode(bit_field_); }
   int arity_without_implicit_args() const {
-    return static_cast<int>(arity() - (has_vector_ ? kTargetAndReceiverAndVector
-                                                   : kTargetAndReceiver));
+    return static_cast<int>(arity() - kExtraCallInputCount);
   }
 
   CallFrequency const& frequency() const { return frequency_; }
@@ -294,8 +290,6 @@ class CallParameters final {
   uint32_t const bit_field_;
   CallFrequency const frequency_;
   FeedbackSource const feedback_;
-  // TODO(jgruber): Remove once Call variants all have a feedback vector input.
-  bool has_vector_;
 };
 
 size_t hash_value(CallParameters const&);
@@ -1222,10 +1216,11 @@ class JSStorePropertyNode final : public JSNodeWrapperBase {
 #undef INPUTS
 };
 
-class JSCallNode final : public JSNodeWrapperBase {
+template <int kOpcode>
+class JSCallNodeBase final : public JSNodeWrapperBase {
  public:
-  explicit constexpr JSCallNode(Node* node) : JSNodeWrapperBase(node) {
-    CONSTEXPR_DCHECK(node->opcode() == IrOpcode::kJSCall);
+  explicit constexpr JSCallNodeBase(Node* node) : JSNodeWrapperBase(node) {
+    CONSTEXPR_DCHECK(node->opcode() == kOpcode);
   }
 
   const CallParameters& Parameters() const {
@@ -1238,12 +1233,19 @@ class JSCallNode final : public JSNodeWrapperBase {
   INPUTS(DEFINE_INPUT_ACCESSORS)
 #undef INPUTS
 
-  // Besides actual arguments, JSCall nodes also take:
+  // 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.
   static constexpr int kTargetInputCount = 1;
   static constexpr int kReceiverInputCount = 1;
   static constexpr int kFeedbackVectorInputCount = 1;
   static constexpr int kExtraInputCount =
       kTargetInputCount + kReceiverInputCount + kFeedbackVectorInputCount;
+  STATIC_ASSERT(kExtraInputCount == CallParameters::kExtraCallInputCount);
+
+  // Just for static asserts for spots that rely on node layout.
+  static constexpr bool kFeedbackVectorIsLastInput = true;
 
   // This is the arity fed into CallArguments.
   static constexpr int ArityForArgc(int parameters) {
@@ -1257,19 +1259,31 @@ class JSCallNode final : public JSNodeWrapperBase {
     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;
   int ArgumentCount() const {
-    DCHECK_GE(node()->op()->ValueInputCount(), kExtraInputCount);
-    return node()->op()->ValueInputCount() - kExtraInputCount;
+    // 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 FeedbackVectorIndexForArgc(int argc) {
+    STATIC_ASSERT(kFeedbackVectorIsLastInput);
+    return ArgumentIndex(argc - 1) + 1;
+  }
   int FeedbackVectorIndex() const {
-    DCHECK_GE(node()->op()->ValueInputCount(), 1);
-    return node()->op()->ValueInputCount() - 1;
+    return FeedbackVectorIndexForArgc(ArgumentCount());
   }
   TNode<HeapObject> feedback_vector() const {
     return TNode<HeapObject>::UncheckedCast(
@@ -1277,6 +1291,10 @@ class JSCallNode final : public JSNodeWrapperBase {
   }
 };
 
+using JSCallNode = JSCallNodeBase<IrOpcode::kJSCall>;
+using JSCallWithSpreadNode = JSCallNodeBase<IrOpcode::kJSCallWithSpread>;
+using JSCallWithArrayLikeNode = JSCallNodeBase<IrOpcode::kJSCallWithArrayLike>;
+
 #undef DEFINE_INPUT_ACCESSORS
 
 }  // namespace compiler

src/compiler/node.cc

@@ -217,17 +217,18 @@ void Node::InsertInputs(Zone* zone, int index, int count) {
   Verify();
 }
 
-void Node::RemoveInput(int index) {
+Node* Node::RemoveInput(int index) {
   DCHECK_LE(0, index);
   DCHECK_LT(index, InputCount());
+  Node* result = InputAt(index);
   for (; index < InputCount() - 1; ++index) {
     ReplaceInput(index, InputAt(index + 1));
   }
   TrimInputCount(InputCount() - 1);
   Verify();
+  return result;
 }
 
-
 void Node::ClearInputs(int start, int count) {
   Node** input_ptr = GetInputPtr(start);
   Use* use_ptr = GetUsePtr(start);

src/compiler/node.h

@@ -89,7 +89,8 @@ class V8_EXPORT_PRIVATE Node final {
   void AppendInput(Zone* zone, Node* new_to);
   void InsertInput(Zone* zone, int index, Node* new_to);
   void InsertInputs(Zone* zone, int index, int count);
-  void RemoveInput(int index);
+  // Returns the removed input.
+  Node* RemoveInput(int index);
   void NullAllInputs();
   void TrimInputCount(int new_input_count);
   // Can trim, extend by appending new inputs, or do nothing.