[maglev] Keep receiver in a stack slot for OptimizedFrame::Summarize

For frame inspection (i.e. not deoptimization), no RegisterValues are
available to TranslatedState and thus any register-allocated value is
unavailable.

Stack trace collection require `function` and `receiver` values to be
available and thus stack-allocated. Both are immutable and have fixed
stack slots so this is not a problem; we just lost track of the receiver
inside Maglev when function parameters were wrapped inside exception Phi
nodes.

We solve this for now by special-casing the `receiver` to reuse the
InitialValue node instead of creating a new Phi.

Bug: v8:7700
Change-Id: I4f4de9a643b98e2fcbc7ee7a53688cc97a8d6f1d
Fixed: chromium:1359428
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3893856
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Jakob Linke <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83164}

BUILD.gn

@@ -4731,6 +4731,7 @@ v8_source_set("v8_base_without_compiler") {
       "src/maglev/maglev-concurrent-dispatcher.cc",
       "src/maglev/maglev-graph-builder.cc",
       "src/maglev/maglev-graph-printer.cc",
+      "src/maglev/maglev-interpreter-frame-state.cc",
       "src/maglev/maglev-ir.cc",
       "src/maglev/maglev-regalloc.cc",
       "src/maglev/maglev.cc",

src/interpreter/bytecode-register.h

@@ -29,6 +29,9 @@ class V8_EXPORT_PRIVATE Register final {
   static Register FromParameterIndex(int index);
   int ToParameterIndex() const;
 
+  static Register receiver() { return FromParameterIndex(0); }
+  bool is_receiver() const { return ToParameterIndex() == 0; }
+
   // Returns an invalid register.
   static Register invalid_value() { return Register(); }
 

src/maglev/maglev-graph-builder.cc

@@ -72,36 +72,6 @@ MaglevGraphBuilder::MaglevGraphBuilder(LocalIsolate* local_isolate,
   }
 
   CalculatePredecessorCounts();
-
-  for (auto& offset_and_info : bytecode_analysis().GetLoopInfos()) {
-    int offset = offset_and_info.first;
-    const compiler::LoopInfo& loop_info = offset_and_info.second;
-    const compiler::BytecodeLivenessState* liveness = GetInLivenessFor(offset);
-    DCHECK_NULL(merge_states_[offset]);
-    if (FLAG_trace_maglev_graph_building) {
-      std::cout << "- Creating loop merge state at @" << offset << std::endl;
-    }
-    merge_states_[offset] = MergePointInterpreterFrameState::NewForLoop(
-        *compilation_unit_, offset, NumPredecessors(offset), liveness,
-        &loop_info);
-  }
-
-  if (bytecode().handler_table_size() > 0) {
-    HandlerTable table(*bytecode().object());
-    for (int i = 0; i < table.NumberOfRangeEntries(); i++) {
-      int offset = table.GetRangeHandler(i);
-      const compiler::BytecodeLivenessState* liveness =
-          GetInLivenessFor(offset);
-      DCHECK_EQ(NumPredecessors(offset), 0);
-      DCHECK_NULL(merge_states_[offset]);
-      if (FLAG_trace_maglev_graph_building) {
-        std::cout << "- Creating exception merge state at @" << offset
-                  << std::endl;
-      }
-      merge_states_[offset] = MergePointInterpreterFrameState::NewForCatchBlock(
-          *compilation_unit_, liveness, offset);
-    }
-  }
 }
 
 void MaglevGraphBuilder::StartPrologue() {
@@ -157,6 +127,38 @@ void MaglevGraphBuilder::BuildRegisterFrameInitialization() {
   }
 }
 
+void MaglevGraphBuilder::BuildMergeStates() {
+  for (auto& offset_and_info : bytecode_analysis().GetLoopInfos()) {
+    int offset = offset_and_info.first;
+    const compiler::LoopInfo& loop_info = offset_and_info.second;
+    const compiler::BytecodeLivenessState* liveness = GetInLivenessFor(offset);
+    DCHECK_NULL(merge_states_[offset]);
+    if (FLAG_trace_maglev_graph_building) {
+      std::cout << "- Creating loop merge state at @" << offset << std::endl;
+    }
+    merge_states_[offset] = MergePointInterpreterFrameState::NewForLoop(
+        *compilation_unit_, offset, NumPredecessors(offset), liveness,
+        &loop_info);
+  }
+
+  if (bytecode().handler_table_size() > 0) {
+    HandlerTable table(*bytecode().object());
+    for (int i = 0; i < table.NumberOfRangeEntries(); i++) {
+      int offset = table.GetRangeHandler(i);
+      const compiler::BytecodeLivenessState* liveness =
+          GetInLivenessFor(offset);
+      DCHECK_EQ(NumPredecessors(offset), 0);
+      DCHECK_NULL(merge_states_[offset]);
+      if (FLAG_trace_maglev_graph_building) {
+        std::cout << "- Creating exception merge state at @" << offset
+                  << std::endl;
+      }
+      merge_states_[offset] = MergePointInterpreterFrameState::NewForCatchBlock(
+          *compilation_unit_, liveness, offset, graph_, is_inline());
+    }
+  }
+}
+
 namespace {
 template <Operation kOperation>
 struct NodeForOperationHelper;
@@ -1849,6 +1851,7 @@ void MaglevGraphBuilder::InlineCallFromRegisters(
   // TODO(leszeks): Also correctly set up the closure and context slots, instead
   // of using InitialValue.
   inner_graph_builder.BuildRegisterFrameInitialization();
+  inner_graph_builder.BuildMergeStates();
   BasicBlock* inlined_prologue = inner_graph_builder.EndPrologue();
 
   // Set the entry JumpToInlined to jump to the prologue block.

src/maglev/maglev-graph-builder.h

@@ -44,10 +44,15 @@ class MaglevGraphBuilder {
 
     StartPrologue();
     for (int i = 0; i < parameter_count(); i++) {
-      SetArgument(i, AddNewNode<InitialValue>(
-                         {}, interpreter::Register::FromParameterIndex(i)));
+      // TODO(v8:7700): Consider creating InitialValue nodes lazily.
+      InitialValue* v = AddNewNode<InitialValue>(
+          {}, interpreter::Register::FromParameterIndex(i));
+      DCHECK_EQ(graph()->parameters().size(), static_cast<size_t>(i));
+      graph()->parameters().push_back(v);
+      SetArgument(i, v);
     }
     BuildRegisterFrameInitialization();
+    BuildMergeStates();
     EndPrologue();
     BuildBody();
   }
@@ -56,6 +61,7 @@ class MaglevGraphBuilder {
   void SetArgument(int i, ValueNode* value);
   ValueNode* GetTaggedArgument(int i);
   void BuildRegisterFrameInitialization();
+  void BuildMergeStates();
   BasicBlock* EndPrologue();
 
   void BuildBody() {

src/maglev/maglev-graph.h

@@ -32,6 +32,7 @@ class Graph final : public ZoneObject {
         smi_(zone),
         int_(zone),
         float_(zone),
+        parameters_(zone),
         constants_(zone) {}
 
   BasicBlock* operator[](int i) { return blocks_[i]; }
@@ -65,6 +66,7 @@ class Graph final : public ZoneObject {
   ZoneMap<int, SmiConstant*>& smi() { return smi_; }
   ZoneMap<int, Int32Constant*>& int32() { return int_; }
   ZoneMap<double, Float64Constant*>& float64() { return float_; }
+  ZoneVector<InitialValue*>& parameters() { return parameters_; }
   compiler::ZoneRefMap<compiler::ObjectRef, Constant*>& constants() {
     return constants_;
   }
@@ -82,6 +84,7 @@ class Graph final : public ZoneObject {
   ZoneMap<int, SmiConstant*> smi_;
   ZoneMap<int, Int32Constant*> int_;
   ZoneMap<double, Float64Constant*> float_;
+  ZoneVector<InitialValue*> parameters_;
   compiler::ZoneRefMap<compiler::ObjectRef, Constant*> constants_;
   Float64Constant* nan_ = nullptr;
 };

src/maglev/maglev-interpreter-frame-state.cc

@@ -0,0 +1,64 @@
+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/maglev/maglev-interpreter-frame-state.h"
+
+#include "src/maglev/maglev-compilation-info.h"
+#include "src/maglev/maglev-graph.h"
+
+namespace v8 {
+namespace internal {
+namespace maglev {
+
+// static
+MergePointInterpreterFrameState*
+MergePointInterpreterFrameState::NewForCatchBlock(
+    const MaglevCompilationUnit& unit,
+    const compiler::BytecodeLivenessState* liveness, int handler_offset,
+    Graph* graph, bool is_inline) {
+  Zone* const zone = unit.zone();
+  MergePointInterpreterFrameState* state =
+      zone->New<MergePointInterpreterFrameState>(
+          unit, 0, 0, nullptr, BasicBlockType::kExceptionHandlerStart,
+          liveness);
+  auto& frame_state = state->frame_state_;
+  // If the accumulator is live, the ExceptionPhi associated to it is the
+  // first one in the block. That ensures it gets kReturnValue0 in the
+  // register allocator. See
+  // StraightForwardRegisterAllocator::AllocateRegisters.
+  if (frame_state.liveness()->AccumulatorIsLive()) {
+    frame_state.accumulator(unit) = state->NewExceptionPhi(
+        zone, interpreter::Register::virtual_accumulator(), handler_offset);
+  }
+  frame_state.ForEachParameter(
+      unit, [&](ValueNode*& entry, interpreter::Register reg) {
+        if (!is_inline && reg.is_receiver()) {
+          // The receiver is a special case for a fairly silly reason:
+          // OptimizedFrame::Summarize requires the receiver (and the function)
+          // to be in a stack slot, since it's value must be available even
+          // though we're not deoptimizing (and thus register states are not
+          // available). Exception phis could be allocated in a register.
+          // Since the receiver is immutable, simply reuse its InitialValue
+          // node.
+          // For inlined functions / nested graph generation, this a) doesn't
+          // work (there's no receiver stack slot); and b) isn't necessary
+          // (Summarize only looks at noninlined functions).
+          entry = graph->parameters()[0];
+        } else {
+          entry = state->NewExceptionPhi(zone, reg, handler_offset);
+        }
+      });
+  frame_state.context(unit) = state->NewExceptionPhi(
+      zone, interpreter::Register::current_context(), handler_offset);
+  frame_state.ForEachLocal(
+      unit, [&](ValueNode*& entry, interpreter::Register reg) {
+        entry = state->NewExceptionPhi(zone, reg, handler_offset);
+      });
+  state->known_node_aspects_ = zone->New<KnownNodeAspects>(zone);
+  return state;
+}
+
+}  // namespace maglev
+}  // namespace internal
+}  // namespace v8

src/maglev/maglev-interpreter-frame-state.h

@@ -481,36 +481,9 @@ class MergePointInterpreterFrameState {
   }
 
   static MergePointInterpreterFrameState* NewForCatchBlock(
-      const MaglevCompilationUnit& info,
-      const compiler::BytecodeLivenessState* liveness, int handler_offset) {
-    MergePointInterpreterFrameState* state =
-        info.zone()->New<MergePointInterpreterFrameState>(
-            info, 0, 0, nullptr, BasicBlockType::kExceptionHandlerStart,
-            liveness);
-    auto& frame_state = state->frame_state_;
-    // If the accumulator is live, the ExceptionPhi associated to it is the
-    // first one in the block. That ensures it gets kReturnValue0 in the
-    // register allocator. See
-    // StraightForwardRegisterAllocator::AllocateRegisters.
-    if (frame_state.liveness()->AccumulatorIsLive()) {
-      frame_state.accumulator(info) = state->NewExceptionPhi(
-          info.zone(), interpreter::Register::virtual_accumulator(),
-          handler_offset);
-    }
-    frame_state.ForEachParameter(
-        info, [&](ValueNode*& entry, interpreter::Register reg) {
-          entry = state->NewExceptionPhi(info.zone(), reg, handler_offset);
-        });
-    frame_state.context(info) = state->NewExceptionPhi(
-        info.zone(), interpreter::Register::current_context(), handler_offset);
-    frame_state.ForEachLocal(
-        info, [&](ValueNode*& entry, interpreter::Register reg) {
-          entry = state->NewExceptionPhi(info.zone(), reg, handler_offset);
-        });
-    state->known_node_aspects_ =
-        info.zone()->New<KnownNodeAspects>(info.zone());
-    return state;
-  }
+      const MaglevCompilationUnit& unit,
+      const compiler::BytecodeLivenessState* liveness, int handler_offset,
+      Graph* graph, bool is_inline);
 
   // Merges an unmerged framestate with a possibly merged framestate into |this|
   // framestate.

src/maglev/maglev-ir.cc

@@ -2827,9 +2827,15 @@ void ChangeInt32ToFloat64::GenerateCode(MaglevAssembler* masm,
 void Phi::AllocateVreg(MaglevVregAllocationState* vreg_state) {
   // Phi inputs are processed in the post-process, once loop phis' inputs'
   // v-regs are allocated.
-  result().SetUnallocated(
-      compiler::UnallocatedOperand::REGISTER_OR_SLOT_OR_CONSTANT,
-      vreg_state->AllocateVirtualRegister());
+
+  // We have to pass a policy, but it is later ignored during register
+  // allocation. See StraightForwardRegisterAllocator::AllocateRegisters
+  // which has special handling for Phis.
+  static const compiler::UnallocatedOperand::ExtendedPolicy kIgnoredPolicy =
+      compiler::UnallocatedOperand::REGISTER_OR_SLOT_OR_CONSTANT;
+
+  result().SetUnallocated(kIgnoredPolicy,
+                          vreg_state->AllocateVirtualRegister());
 }
 // TODO(verwaest): Remove after switching the register allocator.
 void Phi::AllocateVregInPostProcess(MaglevVregAllocationState* vreg_state) {