[compiler] Don't calculate StateValueAccess::size in InstructionSelector

Since the size of the parameters and locals inputs is already stored on
the FrameStateFunctionInfo, this skips the calls to size() and just
reuses the previous values. The stack parameter can only have a size of
0 or 1 depending on whether it's a InterpretedFunction frame or not.

It also extends the verifier to check that the values to match those
returned by StateValueAccess::size and changes a unit test that added
a TypedStateValues of size 2 to the stack input.

Bug: v8:10051
Change-Id: I3693c04b4677812b9f19491c198d0551df20f817
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2047045Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66268}

src/compiler/backend/instruction-selector.cc

@@ -3055,17 +3055,10 @@ namespace {
 FrameStateDescriptor* GetFrameStateDescriptorInternal(Zone* zone, Node* state) {
   DCHECK_EQ(IrOpcode::kFrameState, state->opcode());
   DCHECK_EQ(kFrameStateInputCount, state->InputCount());
-  FrameStateInfo state_info = FrameStateInfoOf(state->op());
-
-  int parameters = static_cast<int>(
-      StateValuesAccess(state->InputAt(kFrameStateParametersInput)).size());
-  int locals = static_cast<int>(
-      StateValuesAccess(state->InputAt(kFrameStateLocalsInput)).size());
-  int stack = static_cast<int>(
-      StateValuesAccess(state->InputAt(kFrameStateStackInput)).size());
-
-  DCHECK_EQ(parameters, state_info.parameter_count());
-  DCHECK_EQ(locals, state_info.local_count());
+  const FrameStateInfo& state_info = FrameStateInfoOf(state->op());
+  int parameters = state_info.parameter_count();
+  int locals = state_info.local_count();
+  int stack = state_info.type() == FrameStateType::kInterpretedFunction ? 1 : 0;
 
   FrameStateDescriptor* outer_state = nullptr;
   Node* outer_node = state->InputAt(kFrameStateOuterStateInput);

src/compiler/verifier.cc

@@ -21,6 +21,7 @@
 #include "src/compiler/operator.h"
 #include "src/compiler/schedule.h"
 #include "src/compiler/simplified-operator.h"
+#include "src/compiler/state-values-utils.h"
 #include "src/compiler/type-cache.h"
 #include "src/utils/bit-vector.h"
 #include "src/utils/ostreams.h"
@@ -550,11 +551,34 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
               NodeProperties::GetValueInput(node, i)->opcode() ==
                   IrOpcode::kTypedStateValues);
       }
-      // The accumulator (InputAt(2)) cannot be kStateValues, but it can be
-      // kTypedStateValues (to signal the type). Once AST graph builder
-      // is removed, we should check this here. Until then, AST graph
-      // builder can generate expression stack as InputAt(2), which can
-      // still be kStateValues.
+
+      // Checks that the state input is empty for all but kInterpretedFunction
+      // frames, where it should have size one.
+      {
+        const FrameStateInfo& state_info = FrameStateInfoOf(node->op());
+        const FrameStateFunctionInfo* func_info = state_info.function_info();
+        CHECK_EQ(func_info->parameter_count(),
+                 StateValuesAccess(node->InputAt(kFrameStateParametersInput))
+                     .size());
+        CHECK_EQ(
+            func_info->local_count(),
+            StateValuesAccess(node->InputAt(kFrameStateLocalsInput)).size());
+
+        Node* accumulator = node->InputAt(kFrameStateStackInput);
+        if (func_info->type() == FrameStateType::kInterpretedFunction) {
+          // The accumulator (InputAt(2)) cannot be kStateValues.
+          // It can be kTypedStateValues (to signal the type) and it can have
+          // other Node types including that of the optimized_out HeapConstant.
+          CHECK_NE(accumulator->opcode(), IrOpcode::kStateValues);
+          if (accumulator->opcode() == IrOpcode::kTypedStateValues) {
+            CHECK_EQ(1, StateValuesAccess(accumulator).size());
+          }
+        } else {
+          CHECK(accumulator->opcode() == IrOpcode::kTypedStateValues ||
+                accumulator->opcode() == IrOpcode::kStateValues);
+          CHECK_EQ(0, StateValuesAccess(accumulator).size());
+        }
+      }
       break;
     }
     case IrOpcode::kObjectId:

test/unittests/compiler/backend/instruction-selector-unittest.cc

@@ -338,7 +338,8 @@ TARGET_TEST_F(InstructionSelectorTest, CallJSFunctionWithDeopt) {
   Node* context = m.Parameter(2);
 
   ZoneVector<MachineType> int32_type(1, MachineType::Int32(), zone());
-  ZoneVector<MachineType> empty_types(zone());
+  ZoneVector<MachineType> tagged_type(1, MachineType::AnyTagged(), zone());
+  ZoneVector<MachineType> empty_type(zone());
 
   auto call_descriptor = Linkage::GetJSCallDescriptor(
       zone(), false, 1,
@@ -349,9 +350,10 @@ TARGET_TEST_F(InstructionSelectorTest, CallJSFunctionWithDeopt) {
       m.common()->TypedStateValues(&int32_type, SparseInputMask::Dense()),
       m.Int32Constant(1));
   Node* locals = m.AddNode(
-      m.common()->TypedStateValues(&empty_types, SparseInputMask::Dense()));
+      m.common()->TypedStateValues(&empty_type, SparseInputMask::Dense()));
   Node* stack = m.AddNode(
-      m.common()->TypedStateValues(&empty_types, SparseInputMask::Dense()));
+      m.common()->TypedStateValues(&tagged_type, SparseInputMask::Dense()),
+      m.UndefinedConstant());
   Node* context_sentinel = m.Int32Constant(0);
   Node* state_node = m.AddNode(
       m.common()->FrameState(bailout_id, OutputFrameStateCombine::PokeAt(0),
@@ -487,7 +489,6 @@ TARGET_TEST_F(InstructionSelectorTest, CallStubWithDeoptRecursiveFrameState) {
   Node* context2 = m.Int32Constant(46);
 
   ZoneVector<MachineType> int32_type(1, MachineType::Int32(), zone());
-  ZoneVector<MachineType> int32x2_type(2, MachineType::Int32(), zone());
   ZoneVector<MachineType> float64_type(1, MachineType::Float64(), zone());
 
   Callable callable = Builtins::CallableFor(isolate(), Builtins::kToObject);
@@ -518,8 +519,8 @@ TARGET_TEST_F(InstructionSelectorTest, CallStubWithDeoptRecursiveFrameState) {
       m.common()->TypedStateValues(&float64_type, SparseInputMask::Dense()),
       m.Float64Constant(0.25));
   Node* stack2 = m.AddNode(
-      m.common()->TypedStateValues(&int32x2_type, SparseInputMask::Dense()),
-      m.Int32Constant(44), m.Int32Constant(45));
+      m.common()->TypedStateValues(&int32_type, SparseInputMask::Dense()),
+      m.Int32Constant(44));
   Node* state_node =
       m.AddNode(m.common()->FrameState(bailout_id_before,
                                        OutputFrameStateCombine::PokeAt(0),
@@ -550,7 +551,7 @@ TARGET_TEST_F(InstructionSelectorTest, CallStubWithDeoptRecursiveFrameState) {
       1 +  // Code object.
       1 +  // Poison index.
       1 +  // Frame state deopt id
-      6 +  // One input for each value in frame state + context.
+      5 +  // One input for each value in frame state + context.
       5 +  // One input for each value in the parent frame state + context.
       1 +  // Function.
       1;   // Context.
@@ -576,17 +577,16 @@ TARGET_TEST_F(InstructionSelectorTest, CallStubWithDeoptRecursiveFrameState) {
   // Values from the nested frame.
   EXPECT_EQ(1u, desc_before->parameters_count());
   EXPECT_EQ(1u, desc_before->locals_count());
-  EXPECT_EQ(2u, desc_before->stack_count());
+  EXPECT_EQ(1u, desc_before->stack_count());
   EXPECT_EQ(43, s.ToInt32(call_instr->InputAt(9)));
   EXPECT_EQ(46, s.ToInt32(call_instr->InputAt(10)));
   EXPECT_EQ(0.25, s.ToFloat64(call_instr->InputAt(11)));
   EXPECT_EQ(44, s.ToInt32(call_instr->InputAt(12)));
-  EXPECT_EQ(45, s.ToInt32(call_instr->InputAt(13)));
 
   // Function.
-  EXPECT_EQ(s.ToVreg(function_node), s.ToVreg(call_instr->InputAt(14)));
+  EXPECT_EQ(s.ToVreg(function_node), s.ToVreg(call_instr->InputAt(13)));
   // Context.
-  EXPECT_EQ(s.ToVreg(context2), s.ToVreg(call_instr->InputAt(15)));
+  EXPECT_EQ(s.ToVreg(context2), s.ToVreg(call_instr->InputAt(14)));
   // Continuation.
 
   EXPECT_EQ(kArchRet, s[index++]->arch_opcode());