[compiler] Clarify ConstructParameters::arity()

... and CallParameters::arity().

The construct arity contains the actual argument count, plus 2 for the
target  (the first input) and new target (the last input). This CL adds
a named constant and a helper method for accessing arity without extra
args. In the future we may want to remove the extra args from arity()
altogether.

Call arity is similar but includes the target and receiver.

Bug: v8:10542,v8:8888
Change-Id: I850fa314f88c2bee9d4dcd87eac9295b2bf88281
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2208850
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67963}

src/compiler/bytecode-graph-builder.cc

@@ -2210,10 +2210,7 @@ void BytecodeGraphBuilder::VisitGetTemplateObject() {
 Node* const* BytecodeGraphBuilder::GetCallArgumentsFromRegisters(
     Node* callee, Node* receiver, interpreter::Register first_arg,
     int arg_count) {
-  // The arity of the Call node -- includes the callee, receiver and function
-  // arguments.
-  int arity = 2 + arg_count;
-
+  int arity = kTargetAndReceiver + arg_count;
   Node** all = local_zone()->NewArray<Node*>(static_cast<size_t>(arity));
 
   all[0] = callee;
@@ -2222,7 +2219,7 @@ Node* const* BytecodeGraphBuilder::GetCallArgumentsFromRegisters(
   // The function arguments are in consecutive registers.
   int arg_base = first_arg.index();
   for (int i = 0; i < arg_count; ++i) {
-    all[2 + i] =
+    all[kTargetAndReceiver + i] =
         environment()->LookupRegister(interpreter::Register(arg_base + i));
   }
 
@@ -2247,7 +2244,8 @@ Node* BytecodeGraphBuilder::ProcessCallArguments(const Operator* call_op,
 
   Node* const* call_args = GetCallArgumentsFromRegisters(callee, receiver_node,
                                                          first_arg, arg_count);
-  return ProcessCallArguments(call_op, call_args, 2 + arg_count);
+  return ProcessCallArguments(call_op, call_args,
+                              kTargetAndReceiver + arg_count);
 }
 
 void BytecodeGraphBuilder::BuildCall(ConvertReceiverMode receiver_mode,
@@ -2318,8 +2316,8 @@ void BytecodeGraphBuilder::BuildCallVarArgs(ConvertReceiverMode receiver_mode) {
                       : static_cast<int>(reg_count) - 1;
   Node* const* call_args =
       ProcessCallVarArgs(receiver_mode, callee, first_reg, arg_count);
-  BuildCall(receiver_mode, call_args, static_cast<size_t>(2 + arg_count),
-            slot_id);
+  BuildCall(receiver_mode, call_args,
+            static_cast<size_t>(kTargetAndReceiver + arg_count), slot_id);
 }
 
 void BytecodeGraphBuilder::VisitCallAnyReceiver() {
@@ -2341,9 +2339,7 @@ void BytecodeGraphBuilder::VisitCallNoFeedback() {
   // The receiver is the first register, followed by the arguments in the
   // consecutive registers.
   int arg_count = static_cast<int>(reg_count) - 1;
-  // The arity of the Call node -- includes the callee, receiver and function
-  // arguments.
-  int arity = 2 + arg_count;
+  int arity = kTargetAndReceiver + arg_count;
 
   // Setting call frequency to a value less than min_inlining frequency to
   // prevent inlining of one-shot call node.
@@ -2459,7 +2455,7 @@ void BytecodeGraphBuilder::VisitCallWithSpread() {
     node = lowering.value();
   } else {
     DCHECK(!lowering.Changed());
-    node = ProcessCallArguments(op, args, 2 + arg_count);
+    node = ProcessCallArguments(op, args, kTargetAndReceiver + arg_count);
   }
   environment()->BindAccumulator(node, Environment::kAttachFrameState);
 }
@@ -2472,10 +2468,11 @@ void BytecodeGraphBuilder::VisitCallJSRuntime() {
   size_t reg_count = bytecode_iterator().GetRegisterCountOperand(2);
   int arg_count = static_cast<int>(reg_count);
 
-  const Operator* call = javascript()->Call(2 + arg_count);
+  const Operator* call = javascript()->Call(kTargetAndReceiver + arg_count);
   Node* const* call_args = ProcessCallVarArgs(
       ConvertReceiverMode::kNullOrUndefined, callee, first_reg, arg_count);
-  Node* value = ProcessCallArguments(call, call_args, 2 + arg_count);
+  Node* value =
+      ProcessCallArguments(call, call_args, kTargetAndReceiver + arg_count);
   environment()->BindAccumulator(value, Environment::kAttachFrameState);
 }
 
@@ -2532,8 +2529,7 @@ void BytecodeGraphBuilder::VisitCallRuntimeForPair() {
 Node* const* BytecodeGraphBuilder::GetConstructArgumentsFromRegister(
     Node* target, Node* new_target, interpreter::Register first_arg,
     int arg_count) {
-  // arity is args + callee and new target.
-  int arity = arg_count + 2;
+  int arity = kTargetAndNewTarget + arg_count;
   Node** all = local_zone()->NewArray<Node*>(static_cast<size_t>(arity));
   all[0] = target;
   int first_arg_index = first_arg.index();
@@ -2563,9 +2559,10 @@ void BytecodeGraphBuilder::VisitConstruct() {
   Node* callee = environment()->LookupRegister(callee_reg);
 
   CallFrequency frequency = ComputeCallFrequency(slot_id);
-  const Operator* op = javascript()->Construct(
-      static_cast<uint32_t>(reg_count + 2), frequency, feedback);
-  int arg_count = static_cast<int>(reg_count);
+  const uint32_t arg_count = static_cast<uint32_t>(reg_count);
+  const uint32_t arg_count_with_extra_args = kTargetAndNewTarget + arg_count;
+  const Operator* op =
+      javascript()->Construct(arg_count_with_extra_args, frequency, feedback);
   Node* const* args = GetConstructArgumentsFromRegister(callee, new_target,
                                                         first_reg, arg_count);
   JSTypeHintLowering::LoweringResult lowering = TryBuildSimplifiedConstruct(
@@ -2577,7 +2574,7 @@ void BytecodeGraphBuilder::VisitConstruct() {
     node = lowering.value();
   } else {
     DCHECK(!lowering.Changed());
-    node = ProcessConstructArguments(op, args, 2 + arg_count);
+    node = ProcessConstructArguments(op, args, arg_count_with_extra_args);
   }
   environment()->BindAccumulator(node, Environment::kAttachFrameState);
 }
@@ -2594,9 +2591,10 @@ void BytecodeGraphBuilder::VisitConstructWithSpread() {
   Node* callee = environment()->LookupRegister(callee_reg);
 
   CallFrequency frequency = ComputeCallFrequency(slot_id);
+  const uint32_t arg_count = static_cast<uint32_t>(reg_count);
+  const uint32_t arg_count_with_extra_args = kTargetAndNewTarget + arg_count;
   const Operator* op = javascript()->ConstructWithSpread(
-      static_cast<uint32_t>(reg_count + 2), frequency, feedback);
-  int arg_count = static_cast<int>(reg_count);
+      arg_count_with_extra_args, frequency, feedback);
   Node* const* args = GetConstructArgumentsFromRegister(callee, new_target,
                                                         first_reg, arg_count);
   JSTypeHintLowering::LoweringResult lowering = TryBuildSimplifiedConstruct(
@@ -2608,7 +2606,7 @@ void BytecodeGraphBuilder::VisitConstructWithSpread() {
     node = lowering.value();
   } else {
     DCHECK(!lowering.Changed());
-    node = ProcessConstructArguments(op, args, 2 + arg_count);
+    node = ProcessConstructArguments(op, args, arg_count_with_extra_args);
   }
   environment()->BindAccumulator(node, Environment::kAttachFrameState);
 }

src/compiler/js-generic-lowering.cc

@@ -722,9 +722,7 @@ void JSGenericLowering::LowerJSConstructForwardVarargs(Node* node) {
 
 void JSGenericLowering::LowerJSConstruct(Node* node) {
   ConstructParameters const& p = ConstructParametersOf(node->op());
-  // TODO(jgruber): Document or refactor the magic `- 2` (target and
-  // new_target) here and elsewhere.
-  int const arg_count = static_cast<int>(p.arity() - 2);
+  int const arg_count = p.arity_without_implicit_args();
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
 
   // TODO(jgruber): Understand and document how stack_argument_count is
@@ -780,7 +778,7 @@ void JSGenericLowering::LowerJSConstruct(Node* node) {
 void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
   ConstructParameters const& p = ConstructParametersOf(node->op());
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
-  const int arg_count = static_cast<int>(p.arity() - 2);
+  const int arg_count = p.arity_without_implicit_args();
   DCHECK_EQ(arg_count, 1);
 
   static constexpr int kReceiver = 1;
@@ -834,7 +832,7 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
 
 void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
   ConstructParameters const& p = ConstructParametersOf(node->op());
-  int const arg_count = static_cast<int>(p.arity() - 2);
+  int const arg_count = p.arity_without_implicit_args();
   int const spread_index = arg_count;
   int const new_target_index = arg_count + 1;
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
@@ -916,7 +914,7 @@ void JSGenericLowering::LowerJSCallForwardVarargs(Node* node) {
 
 void JSGenericLowering::LowerJSCall(Node* node) {
   CallParameters const& p = CallParametersOf(node->op());
-  int const arg_count = static_cast<int>(p.arity() - 2);
+  int const arg_count = p.arity_without_implicit_args();
   ConvertReceiverMode const mode = p.convert_mode();
 
   if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
@@ -948,7 +946,7 @@ void JSGenericLowering::LowerJSCall(Node* node) {
 
 void JSGenericLowering::LowerJSCallWithArrayLike(Node* node) {
   CallParameters const& p = CallParametersOf(node->op());
-  const int arg_count = static_cast<int>(p.arity() - 2);
+  const int arg_count = p.arity_without_implicit_args();
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
 
   DCHECK_EQ(arg_count, 0);
@@ -986,7 +984,7 @@ void JSGenericLowering::LowerJSCallWithArrayLike(Node* node) {
 
 void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
   CallParameters const& p = CallParametersOf(node->op());
-  int const arg_count = static_cast<int>(p.arity() - 2);
+  int const arg_count = p.arity_without_implicit_args();
   int const spread_index = arg_count + 1;
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
 

src/compiler/js-native-context-specialization.cc

@@ -488,7 +488,8 @@ Reduction JSNativeContextSpecialization::ReduceJSInstanceOf(Node* node) {
     node->ReplaceInput(4, continuation_frame_state);
     node->ReplaceInput(5, effect);
     NodeProperties::ChangeOp(
-        node, javascript()->Call(3, CallFrequency(), FeedbackSource(),
+        node, javascript()->Call(1 + kTargetAndReceiver, CallFrequency(),
+                                 FeedbackSource(),
                                  ConvertReceiverMode::kNotNullOrUndefined));
 
     // Rewire the value uses of {node} to ToBoolean conversion of the result.
@@ -1428,10 +1429,10 @@ Reduction JSNativeContextSpecialization::ReduceJSGetIterator(Node* node) {
   SpeculationMode mode = feedback.IsInsufficient()
                              ? SpeculationMode::kDisallowSpeculation
                              : feedback.AsCall().speculation_mode();
-  const Operator* call_op =
-      javascript()->Call(2, CallFrequency(), p.callFeedback(),
-                         ConvertReceiverMode::kNotNullOrUndefined, mode,
-                         CallFeedbackRelation::kRelated);
+  const Operator* call_op = javascript()->Call(
+      0 + kTargetAndReceiver, CallFrequency(), p.callFeedback(),
+      ConvertReceiverMode::kNotNullOrUndefined, mode,
+      CallFeedbackRelation::kRelated);
   Node* call_property = graph()->NewNode(call_op, load_property, receiver,
                                          context, frame_state, effect, control);
 
@@ -2048,7 +2049,8 @@ Node* JSNativeContextSpecialization::InlinePropertyGetterCall(
   Node* value;
   if (constant.IsJSFunction()) {
     value = *effect = *control = graph()->NewNode(
-        jsgraph()->javascript()->Call(2, CallFrequency(), FeedbackSource(),
+        jsgraph()->javascript()->Call(0 + kTargetAndReceiver, CallFrequency(),
+                                      FeedbackSource(),
                                       ConvertReceiverMode::kNotNullOrUndefined),
         target, receiver, context, frame_state, *effect, *control);
   } else {
@@ -2085,7 +2087,8 @@ void JSNativeContextSpecialization::InlinePropertySetterCall(
   // Introduce the call to the setter function.
   if (constant.IsJSFunction()) {
     *effect = *control = graph()->NewNode(
-        jsgraph()->javascript()->Call(3, CallFrequency(), FeedbackSource(),
+        jsgraph()->javascript()->Call(1 + kTargetAndReceiver, CallFrequency(),
+                                      FeedbackSource(),
                                       ConvertReceiverMode::kNotNullOrUndefined),
         target, receiver, value, context, frame_state, *effect, *control);
   } else {

src/compiler/js-operator.h

@@ -95,16 +95,32 @@ std::ostream& operator<<(std::ostream&,
 ConstructForwardVarargsParameters const& ConstructForwardVarargsParametersOf(
     Operator const*) V8_WARN_UNUSED_RESULT;
 
-// Defines the arity and the feedback for a JavaScript constructor call. This is
-// used as a parameter by JSConstruct, JSConstructWithArrayLike, and
-// JSConstructWithSpread operators.
+// Part of ConstructParameters::arity.
+static constexpr int kTargetAndNewTarget = 2;
+
+// Defines the arity (parameters plus the target and new target) and the
+// feedback for a JavaScript constructor call. This is used as a parameter by
+// JSConstruct, JSConstructWithArrayLike, and JSConstructWithSpread operators.
 class ConstructParameters final {
  public:
   ConstructParameters(uint32_t arity, CallFrequency const& frequency,
                       FeedbackSource const& feedback)
-      : arity_(arity), frequency_(frequency), feedback_(feedback) {}
+      : arity_(arity), frequency_(frequency), feedback_(feedback) {
+    DCHECK_GE(arity, kTargetAndNewTarget);
+    DCHECK(is_int32(arity));
+  }
 
+  // TODO(jgruber): Consider removing `arity()` and just storing the arity
+  // without extra args in ConstructParameters. Every spot that creates
+  // ConstructParameters artifically adds the extra args. Every spot that uses
+  // ConstructParameters artificially subtracts the extra args.
+  // We keep them for now for consistency with other spots
+  // that expect `arity()` to include extra args.
   uint32_t arity() const { return arity_; }
+  int arity_without_implicit_args() const {
+    return static_cast<int>(arity_ - kTargetAndNewTarget);
+  }
+
   CallFrequency const& frequency() const { return frequency_; }
   FeedbackSource const& feedback() const { return feedback_; }
 
@@ -157,8 +173,12 @@ std::ostream& operator<<(std::ostream&, CallForwardVarargsParameters const&);
 CallForwardVarargsParameters const& CallForwardVarargsParametersOf(
     Operator const*) V8_WARN_UNUSED_RESULT;
 
-// Defines the arity and the call flags for a JavaScript function call. This is
-// used as a parameter by JSCall and JSCallWithSpread operators.
+// Part of CallParameters::arity.
+static constexpr int kTargetAndReceiver = 2;
+
+// 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:
   CallParameters(size_t arity, CallFrequency const& frequency,
@@ -177,9 +197,17 @@ class CallParameters final {
                    feedback.IsValid());
     DCHECK_IMPLIES(!feedback.IsValid(),
                    feedback_relation == CallFeedbackRelation::kUnrelated);
+    DCHECK_GE(arity, kTargetAndReceiver);
+    DCHECK(is_int32(arity));
   }
 
+  // TODO(jgruber): Consider removing `arity()` and just storing the arity
+  // 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() - kTargetAndReceiver);
+  }
+
   CallFrequency const& frequency() const { return frequency_; }
   ConvertReceiverMode convert_mode() const {
     return ConvertReceiverModeField::decode(bit_field_);

src/compiler/js-typed-lowering.cc

@@ -1577,8 +1577,7 @@ Reduction JSTypedLowering::ReduceJSConstructForwardVarargs(Node* node) {
 Reduction JSTypedLowering::ReduceJSConstruct(Node* node) {
   DCHECK_EQ(IrOpcode::kJSConstruct, node->opcode());
   ConstructParameters const& p = ConstructParametersOf(node->op());
-  DCHECK_LE(2u, p.arity());
-  int const arity = static_cast<int>(p.arity() - 2);
+  int const arity = p.arity_without_implicit_args();
   Node* target = NodeProperties::GetValueInput(node, 0);
   Type target_type = NodeProperties::GetType(target);
   Node* new_target = NodeProperties::GetValueInput(node, arity + 1);
@@ -1649,7 +1648,7 @@ Reduction JSTypedLowering::ReduceJSCallForwardVarargs(Node* node) {
 Reduction JSTypedLowering::ReduceJSCall(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
   CallParameters const& p = CallParametersOf(node->op());
-  int arity = static_cast<int>(p.arity() - 2);
+  int arity = p.arity_without_implicit_args();
   ConvertReceiverMode convert_mode = p.convert_mode();
   Node* target = NodeProperties::GetValueInput(node, 0);
   Type target_type = NodeProperties::GetType(target);