[compiler] Add more StartNode helpers

Start nodes for JS functions have the following Parameter node value
outputs:

 closure, ...args_including_receiver, new_target, argc, context

This CL adds helper functions for these. There's two interesting
gotcha's:

- Each Parameter node is associated with an index, starting at -1.
Value output indices obviously start at 0, so there's an off-by-one
between the value output of the Parameter node, and the Parameter
node's associated index.
- CSA/Torque graphs use different Start node layouts, yet these are
not reflected in compiler logic. There's potential for confusion here.
The two layouts should be unified or made explicit.

Finally, tests create Start nodes with arbitrary layouts. This blocks
removal of methods marked _MaybeNonStandardLayout.

In an ideal world, the parameter index would equal the start node
output index, and the layout of all Start nodes would be equal. Future
work..

Change-Id: I908909880817979062d459b7a80ed4fede40e2ec
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2649035
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Auto-Submit: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72352}

src/compiler/common-operator.h

@@ -700,6 +700,67 @@ class StartNode final : public CommonNodeWrapperBase {
     return node()->op()->ValueOutputCount() - kExtraOutputCount -
            kReceiverOutputCount;
   }
+
+  // Note these functions don't return the index of the Start output; instead
+  // they return the index assigned to the Parameter node.
+  // TODO(jgruber): Consider unifying the two.
+  int NewTargetParameterIndex() const {
+    return Linkage::GetJSCallNewTargetParamIndex(FormalParameterCount());
+  }
+  int ArgCountParameterIndex() const {
+    return Linkage::GetJSCallArgCountParamIndex(FormalParameterCount());
+  }
+  int ContextParameterIndex() const {
+    return Linkage::GetJSCallContextParamIndex(FormalParameterCount());
+  }
+
+  // TODO(jgruber): Remove this function and use
+  // Linkage::GetJSCallContextParamIndex instead. This currently doesn't work
+  // because tests don't create valid Start nodes - for example, they may add
+  // only two context outputs (and not the closure, new target, argc). Once
+  // tests are fixed, remove this function.
+  int ContextParameterIndex_MaybeNonStandardLayout() const {
+    // The context is always the last parameter to a JavaScript function, and
+    // {Parameter} indices start at -1, so value outputs of {Start} look like
+    // this: closure, receiver, param0, ..., paramN, context.
+    //
+    // TODO(jgruber): This function is called from spots that operate on
+    // CSA/Torque graphs; Start node layout appears to be different there.
+    // These should be unified to avoid confusion. Once done, enable this
+    // DCHECK: DCHECK_EQ(LastOutputIndex(), ContextOutputIndex());
+    return node()->op()->ValueOutputCount() - 2;
+  }
+  int LastParameterIndex_MaybeNonStandardLayout() const {
+    return ContextParameterIndex_MaybeNonStandardLayout();
+  }
+
+  // Unlike ContextParameterIndex_MaybeNonStandardLayout above, these return
+  // output indices (and not the index assigned to a Parameter).
+  int NewTargetOutputIndex() const {
+    // Indices assigned to parameters are off-by-one (Parameters indices start
+    // at -1).
+    // TODO(jgruber): Consider starting at 0.
+    DCHECK_EQ(Linkage::GetJSCallNewTargetParamIndex(FormalParameterCount()) + 1,
+              node()->op()->ValueOutputCount() - 3);
+    return node()->op()->ValueOutputCount() - 3;
+  }
+  int ArgCountOutputIndex() const {
+    // Indices assigned to parameters are off-by-one (Parameters indices start
+    // at -1).
+    // TODO(jgruber): Consider starting at 0.
+    DCHECK_EQ(Linkage::GetJSCallArgCountParamIndex(FormalParameterCount()) + 1,
+              node()->op()->ValueOutputCount() - 2);
+    return node()->op()->ValueOutputCount() - 2;
+  }
+  int ContextOutputIndex() const {
+    // Indices assigned to parameters are off-by-one (Parameters indices start
+    // at -1).
+    // TODO(jgruber): Consider starting at 0.
+    DCHECK_EQ(Linkage::GetJSCallContextParamIndex(FormalParameterCount()) + 1,
+              node()->op()->ValueOutputCount() - 1);
+    return node()->op()->ValueOutputCount() - 1;
+  }
+  int LastOutputIndex() const { return ContextOutputIndex(); }
 };
 
 class DynamicCheckMapsWithDeoptUnlessNode final : public CommonNodeWrapperBase {

src/compiler/js-context-specialization.cc

@@ -89,13 +89,9 @@ namespace {
 
 bool IsContextParameter(Node* node) {
   DCHECK_EQ(IrOpcode::kParameter, node->opcode());
-  Node* const start = NodeProperties::GetValueInput(node, 0);
-  DCHECK_EQ(IrOpcode::kStart, start->opcode());
-  int const index = ParameterIndexOf(node->op());
-  // The context is always the last parameter to a JavaScript function, and
-  // {Parameter} indices start at -1, so value outputs of {Start} look like
-  // this: closure, receiver, param0, ..., paramN, context.
-  return index == start->op()->ValueOutputCount() - 2;
+  return ParameterIndexOf(node->op()) ==
+         StartNode{NodeProperties::GetValueInput(node, 0)}
+             .ContextParameterIndex_MaybeNonStandardLayout();
 }
 
 // Given a context {node} and the {distance} from that context to the target

src/compiler/js-inlining.cc

@@ -81,7 +81,7 @@ class JSCallAccessor {
 };
 
 Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
-                                Node* frame_state, Node* start, Node* end,
+                                Node* frame_state, StartNode start, Node* end,
                                 Node* exception_target,
                                 const NodeVector& uncaught_subcalls) {
   JSCallAccessor c(call);
@@ -92,12 +92,9 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
   Node* control = NodeProperties::GetControlInput(call);
   Node* effect = NodeProperties::GetEffectInput(call);
 
-  int const inlinee_new_target_index =
-      static_cast<int>(start->op()->ValueOutputCount()) - 3;
-  int const inlinee_arity_index =
-      static_cast<int>(start->op()->ValueOutputCount()) - 2;
-  int const inlinee_context_index =
-      static_cast<int>(start->op()->ValueOutputCount()) - 1;
+  int const inlinee_new_target_index = start.NewTargetOutputIndex();
+  int const inlinee_arity_index = start.ArgCountOutputIndex();
+  int const inlinee_context_index = start.ContextOutputIndex();
 
   // {inliner_inputs} counts the target, receiver/new_target, and arguments; but
   // not feedback vector, context, effect or control.
@@ -456,7 +453,7 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
                                 source_positions_->GetSourcePosition(node));
 
   // Create the subgraph for the inlinee.
-  Node* start;
+  Node* start_node;
   Node* end;
   {
     // Run the BytecodeGraphBuilder to create the subgraph.
@@ -478,9 +475,10 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
     }
 
     // Extract the inlinee start/end nodes.
-    start = graph()->start();
+    start_node = graph()->start();
     end = graph()->end();
   }
+  StartNode start{start_node};
 
   // If we are inlining into a surrounding exception handler, we collect all
   // potentially throwing nodes within the inlinee that are not handled locally
@@ -603,11 +601,10 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
   }
 
   // Insert argument adaptor frame if required. The callees formal parameter
-  // count (i.e. value outputs of start node minus target, receiver, new target,
-  // arguments count and context) have to match the number of arguments passed
+  // count have to match the number of arguments passed
   // to the call.
   int parameter_count = shared_info->internal_formal_parameter_count();
-  DCHECK_EQ(parameter_count, start->op()->ValueOutputCount() - 5);
+  DCHECK_EQ(parameter_count, start.FormalParameterCountWithoutReceiver());
   if (call.argument_count() != parameter_count) {
     frame_state = CreateArtificialFrameState(
         node, frame_state, call.argument_count(), BytecodeOffset::None(),

src/compiler/js-inlining.h

@@ -67,7 +67,7 @@ class JSInliner final : public AdvancedReducer {
       SharedFunctionInfoRef shared, Node* context = nullptr);
 
   Reduction InlineCall(Node* call, Node* new_target, Node* context,
-                       Node* frame_state, Node* start, Node* end,
+                       Node* frame_state, StartNode start, Node* end,
                        Node* exception_target,
                        const NodeVector& uncaught_subcalls);
 };

src/compiler/typer.cc

@@ -680,10 +680,7 @@ Type Typer::Visitor::TypeIfException(Node* node) { return Type::NonInternal(); }
 // Common operators.
 
 Type Typer::Visitor::TypeParameter(Node* node) {
-  Node* const start = node->InputAt(0);
-  DCHECK_EQ(IrOpcode::kStart, start->opcode());
-  int const parameter_count = start->op()->ValueOutputCount() - 4;
-  DCHECK_LE(1, parameter_count);
+  StartNode start{node->InputAt(0)};
   int const index = ParameterIndexOf(node->op());
   if (index == Linkage::kJSCallClosureParamIndex) {
     return Type::Function();
@@ -694,15 +691,15 @@ Type Typer::Visitor::TypeParameter(Node* node) {
       // Parameter[this] can be the_hole for derived class constructors.
       return Type::Union(Type::Hole(), Type::NonInternal(), typer_->zone());
     }
-  } else if (index == Linkage::GetJSCallNewTargetParamIndex(parameter_count)) {
+  } else if (index == start.NewTargetParameterIndex()) {
     if (typer_->flags() & Typer::kNewTargetIsReceiver) {
       return Type::Receiver();
     } else {
       return Type::Union(Type::Receiver(), Type::Undefined(), typer_->zone());
     }
-  } else if (index == Linkage::GetJSCallArgCountParamIndex(parameter_count)) {
+  } else if (index == start.ArgCountParameterIndex()) {
     return Type::Range(0.0, FixedArray::kMaxLength, typer_->zone());
-  } else if (index == Linkage::GetJSCallContextParamIndex(parameter_count)) {
+  } else if (index == start.ContextParameterIndex()) {
     return Type::OtherInternal();
   }
   return Type::NonInternal();

src/compiler/verifier.cc

@@ -396,11 +396,10 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
       CHECK_EQ(1, input_count);
       // Parameter has an input that produces enough values.
       int const index = ParameterIndexOf(node->op());
-      Node* const start = NodeProperties::GetValueInput(node, 0);
-      CHECK_EQ(IrOpcode::kStart, start->opcode());
+      StartNode start{NodeProperties::GetValueInput(node, 0)};
       // Currently, parameter indices start at -1 instead of 0.
       CHECK_LE(-1, index);
-      CHECK_LT(index + 1, start->op()->ValueOutputCount());
+      CHECK_LE(index, start.LastParameterIndex_MaybeNonStandardLayout());
       CheckTypeIs(node, Type::Any());
       break;
     }