[maglev] Remove diff encoding of checkpoints

Remove StoreToFrame and the general diff encoding for checkpoints, and
instead make all Checkpoints immediately copy the live part of the
interpreter frame state.

This means that we don't need to recreate the frame state during graph
processing, and we don't have to copy the checkpoint's state for storing
in the deferred DeoptimizationInfo.

In theory the diff encoding was meant to save zone memory for unused
checkpoints, and for checkpoints that don't differ much from each other.
However,

  a) We expect to do most checkpoint elimination during graph building,
     so the assumption that many checkpoints will be unused seems less
     probable, and

  b) We need to copy the checkpoint's frame state for emitting deopts,
     so we don't actually end up avoiding doing the copies.

So, we can simplify things by removing this complexity.

Bug: v8:7700
Change-Id: Iff9743fabbf7a017cccf0ece76a797c571764ea6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3545178Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79602}

src/maglev/maglev-code-gen-state.h

@@ -24,12 +24,12 @@ class InterpreterFrameState;
 class DeoptimizationInfo {
  public:
   DeoptimizationInfo(BytecodeOffset bytecode_position,
-                     InterpreterFrameState* checkpoint_state)
+                     const CompactInterpreterFrameState* checkpoint_state)
       : bytecode_position(bytecode_position),
         checkpoint_state(checkpoint_state) {}
 
   BytecodeOffset bytecode_position;
-  InterpreterFrameState* checkpoint_state;
+  const CompactInterpreterFrameState* checkpoint_state;
   Label entry_label;
   int index = -1;
 };

src/maglev/maglev-code-generator.cc

@@ -43,8 +43,6 @@ using StackToRegisterMoves =
 
 class MaglevCodeGeneratingNodeProcessor {
  public:
-  static constexpr bool kNeedsCheckpointStates = true;
-
   explicit MaglevCodeGeneratingNodeProcessor(MaglevCodeGenState* code_gen_state)
       : code_gen_state_(code_gen_state) {}
 
@@ -397,19 +395,22 @@ class MaglevCodeGeneratorImpl final {
         deopt_info->bytecode_position, kFunctionLiteralIndex,
         code_gen_state_.register_count(), return_offset, return_count);
 
-    auto* liveness = code_gen_state_.bytecode_analysis().GetInLivenessFor(
-        deopt_info->bytecode_position.ToInt());
-
     // Closure
     int closure_index = DeoptStackSlotIndexFromFPOffset(
         StandardFrameConstants::kFunctionOffset);
     translation_array_builder_.StoreStackSlot(closure_index);
 
     // Parameters
-    for (int i = 0; i < code_gen_state_.parameter_count(); ++i) {
-      interpreter::Register reg = interpreter::Register::FromParameterIndex(i);
-      translation_array_builder_.StoreStackSlot(DeoptStackSlotFromStackSlot(
-          deopt_info->checkpoint_state->get(reg)->spill_slot()));
+    {
+      int i = 0;
+      deopt_info->checkpoint_state->ForEachParameter(
+          *code_gen_state_.compilation_unit(),
+          [&](ValueNode* value, interpreter::Register reg) {
+            DCHECK_EQ(reg.ToParameterIndex(), i);
+            translation_array_builder_.StoreStackSlot(
+                DeoptStackSlotFromStackSlot(value->spill_slot()));
+            i++;
+          });
     }
 
     // Context
@@ -418,22 +419,42 @@ class MaglevCodeGeneratorImpl final {
     translation_array_builder_.StoreStackSlot(context_index);
 
     // Locals
-    for (int i = 0; i < code_gen_state_.register_count(); ++i) {
-      interpreter::Register reg(i);
-      if (liveness->RegisterIsLive(i)) {
-        translation_array_builder_.StoreStackSlot(DeoptStackSlotFromStackSlot(
-            deopt_info->checkpoint_state->get(reg)->spill_slot()));
-      } else {
+    {
+      int i = 0;
+      deopt_info->checkpoint_state->ForEachLocal(
+          *code_gen_state_.compilation_unit(),
+          [&](ValueNode* value, interpreter::Register reg) {
+            DCHECK_LE(i, reg.index());
+            while (i < reg.index()) {
+              translation_array_builder_.StoreLiteral(
+                  kOptimizedOutConstantIndex);
+              i++;
+            }
+            DCHECK_EQ(i, reg.index());
+            translation_array_builder_.StoreStackSlot(
+                DeoptStackSlotFromStackSlot(value->spill_slot()));
+            i++;
+          });
+      while (i < code_gen_state_.register_count()) {
         translation_array_builder_.StoreLiteral(kOptimizedOutConstantIndex);
+        i++;
       }
     }
 
     // Accumulator
-    if (liveness->AccumulatorIsLive()) {
-      translation_array_builder_.StoreStackSlot(
-          deopt_info->checkpoint_state->accumulator()->spill_slot().index());
-    } else {
-      translation_array_builder_.StoreLiteral(kOptimizedOutConstantIndex);
+    {
+      // TODO(leszeks): Bit ugly to use a did_emit boolean here rather than
+      // explicitly checking for accumulator liveness.
+      bool did_emit = false;
+      deopt_info->checkpoint_state->ForAccumulator(
+          *code_gen_state_.compilation_unit(), [&](ValueNode* value) {
+            translation_array_builder_.StoreStackSlot(
+                DeoptStackSlotFromStackSlot(value->spill_slot()));
+            did_emit = true;
+          });
+      if (!did_emit) {
+        translation_array_builder_.StoreLiteral(kOptimizedOutConstantIndex);
+      }
     }
   }
 

src/maglev/maglev-compiler.cc

@@ -46,8 +46,6 @@ namespace maglev {
 
 class NumberingProcessor {
  public:
-  static constexpr bool kNeedsCheckpointStates = false;
-
   void PreProcessGraph(MaglevCompilationUnit*, Graph* graph) { node_id_ = 1; }
   void PostProcessGraph(MaglevCompilationUnit*, Graph* graph) {}
   void PreProcessBasicBlock(MaglevCompilationUnit*, BasicBlock* block) {}
@@ -62,8 +60,6 @@ class NumberingProcessor {
 
 class UseMarkingProcessor {
  public:
-  static constexpr bool kNeedsCheckpointStates = true;
-
   void PreProcessGraph(MaglevCompilationUnit*, Graph* graph) {}
   void PostProcessGraph(MaglevCompilationUnit*, Graph* graph) {}
   void PreProcessBasicBlock(MaglevCompilationUnit*, BasicBlock* block) {}
@@ -106,23 +102,15 @@ class UseMarkingProcessor {
 
  private:
   void MarkCheckpointNodes(NodeBase* node, const ProcessingState& state) {
-    const InterpreterFrameState* checkpoint_state =
-        state.checkpoint_frame_state();
+    const CompactInterpreterFrameState* checkpoint_state =
+        state.checkpoint()->frame();
     int use_id = node->id();
 
-    for (int i = 0; i < state.parameter_count(); i++) {
-      interpreter::Register reg = interpreter::Register::FromParameterIndex(i);
-      ValueNode* node = checkpoint_state->get(reg);
-      if (node) node->mark_use(use_id, nullptr);
-    }
-    for (int i = 0; i < state.register_count(); i++) {
-      interpreter::Register reg = interpreter::Register(i);
-      ValueNode* node = checkpoint_state->get(reg);
-      if (node) node->mark_use(use_id, nullptr);
-    }
-    if (checkpoint_state->accumulator()) {
-      checkpoint_state->accumulator()->mark_use(use_id, nullptr);
-    }
+    checkpoint_state->ForEachValue(
+        *state.compilation_unit(),
+        [use_id](ValueNode* node, interpreter::Register reg) {
+          if (node) node->mark_use(use_id, nullptr);
+        });
   }
 };
 

src/maglev/maglev-graph-builder.h

@@ -188,14 +188,14 @@ class MaglevGraphBuilder {
       return;
     }
     current_interpreter_frame_.set(target, value);
-    AddNewNode<StoreToFrame>({}, value, target);
   }
 
   void AddCheckpoint() {
     // TODO(v8:7700): Verify this calls the initializer list overload.
-    AddNewNode<Checkpoint>({}, BytecodeOffset(iterator_.current_offset()),
-                           GetInLiveness()->AccumulatorIsLive(),
-                           GetAccumulator());
+    AddNewNode<Checkpoint>(
+        {}, BytecodeOffset(iterator_.current_offset()),
+        zone()->New<CompactInterpreterFrameState>(
+            *compilation_unit_, GetInLiveness(), current_interpreter_frame_));
     has_valid_checkpoint_ = true;
   }
 

src/maglev/maglev-graph-printer.h

@@ -26,9 +26,6 @@ class ProcessingState;
 
 class MaglevPrintingVisitor {
  public:
-  // Could be interesting to print checkpoints too.
-  static constexpr bool kNeedsCheckpointStates = false;
-
   explicit MaglevPrintingVisitor(std::ostream& os);
 
   void PreProcessGraph(MaglevCompilationUnit*, Graph* graph);

src/maglev/maglev-ir.cc

@@ -164,6 +164,10 @@ struct CopyForDeferredHelper<const InterpreterFrameState*> {
         *compilation_unit, *frame_state);
   }
 };
+// CompactInterpreterFrameState is copied by value.
+template <>
+struct CopyForDeferredHelper<const CompactInterpreterFrameState*>
+    : public CopyForDeferredByValue<const CompactInterpreterFrameState*> {};
 
 template <typename T>
 T CopyForDeferred(MaglevCompilationUnit* compilation_unit, T&& value) {
@@ -260,14 +264,10 @@ void JumpToDeferredIf(Condition cond, MaglevCodeGenState* code_gen_state,
 
 DeoptimizationInfo* CreateEagerDeopt(
     MaglevCodeGenState* code_gen_state, BytecodeOffset bytecode_position,
-    const InterpreterFrameState* checkpoint_state) {
+    const CompactInterpreterFrameState* checkpoint_state) {
   Zone* zone = code_gen_state->compilation_unit()->zone();
-  DeoptimizationInfo* deopt_info = zone->New<DeoptimizationInfo>(
-      bytecode_position,
-      // TODO(leszeks): Right now we unconditionally copy the IFS. If we made
-      // checkpoint states already always be copies, we could remove this copy.
-      zone->New<InterpreterFrameState>(*code_gen_state->compilation_unit(),
-                                       *checkpoint_state));
+  DeoptimizationInfo* deopt_info =
+      zone->New<DeoptimizationInfo>(bytecode_position, checkpoint_state);
 
   code_gen_state->PushNonLazyDeopt(deopt_info);
   return deopt_info;
@@ -275,7 +275,7 @@ DeoptimizationInfo* CreateEagerDeopt(
 
 void EmitEagerDeoptIf(Condition cond, MaglevCodeGenState* code_gen_state,
                       BytecodeOffset bytecode_position,
-                      const InterpreterFrameState* checkpoint_state) {
+                      const CompactInterpreterFrameState* checkpoint_state) {
   DeoptimizationInfo* deopt_info =
       CreateEagerDeopt(code_gen_state, bytecode_position, checkpoint_state);
   __ RecordComment("-- Jump to eager deopt");
@@ -287,7 +287,7 @@ void EmitEagerDeoptIf(Condition cond, MaglevCodeGenState* code_gen_state,
   DCHECK(node->properties().can_deopt());
   EmitEagerDeoptIf(cond, code_gen_state,
                    state.checkpoint()->bytecode_position(),
-                   state.checkpoint_frame_state());
+                   state.checkpoint()->frame());
 }
 
 // ---
@@ -378,7 +378,7 @@ void Checkpoint::GenerateCode(MaglevCodeGenState* code_gen_state,
                               const ProcessingState& state) {}
 void Checkpoint::PrintParams(std::ostream& os,
                              MaglevGraphLabeller* graph_labeller) const {
-  os << "(" << PrintNodeLabel(graph_labeller, accumulator()) << ")";
+  os << "(" << ToString(*frame()->liveness()) << ")";
 }
 
 void SoftDeopt::AllocateVreg(MaglevVregAllocationState* vreg_state,
@@ -500,7 +500,7 @@ void CheckMaps::GenerateCode(MaglevCodeGenState* code_gen_state,
         not_equal, code_gen_state,
         [](MaglevCodeGenState* code_gen_state, Label* return_label,
            Register object, CheckMaps* node, BytecodeOffset checkpoint_position,
-           const InterpreterFrameState* checkpoint_state_snapshot,
+           const CompactInterpreterFrameState* checkpoint_state_snapshot,
            Register map_tmp) {
           DeoptimizationInfo* deopt = CreateEagerDeopt(
               code_gen_state, checkpoint_position, checkpoint_state_snapshot);
@@ -530,7 +530,7 @@ void CheckMaps::GenerateCode(MaglevCodeGenState* code_gen_state,
           __ jmp(&deopt->entry_label);
         },
         object, this, state.checkpoint()->bytecode_position(),
-        state.checkpoint_frame_state(), map_tmp);
+        state.checkpoint()->frame(), map_tmp);
   } else {
     EmitEagerDeoptIf(not_equal, code_gen_state, this, state);
   }
@@ -623,16 +623,6 @@ void LoadNamedGeneric::PrintParams(std::ostream& os,
   os << "(" << name_ << ")";
 }
 
-void StoreToFrame::AllocateVreg(MaglevVregAllocationState* vreg_state,
-                                const ProcessingState& state) {}
-void StoreToFrame::GenerateCode(MaglevCodeGenState* code_gen_state,
-                                const ProcessingState& state) {}
-void StoreToFrame::PrintParams(std::ostream& os,
-                               MaglevGraphLabeller* graph_labeller) const {
-  os << "(" << target().ToString() << " ← "
-     << PrintNodeLabel(graph_labeller, value()) << ")";
-}
-
 void GapMove::AllocateVreg(MaglevVregAllocationState* vreg_state,
                            const ProcessingState& state) {
   UNREACHABLE();
@@ -742,9 +732,7 @@ void Phi::AllocateVregInPostProcess(MaglevVregAllocationState* vreg_state) {
   }
 }
 void Phi::GenerateCode(MaglevCodeGenState* code_gen_state,
-                       const ProcessingState& state) {
-  DCHECK_EQ(state.interpreter_frame_state()->get(owner()), this);
-}
+                       const ProcessingState& state) {}
 void Phi::PrintParams(std::ostream& os,
                       MaglevGraphLabeller* graph_labeller) const {
   os << "(" << owner().ToString() << ")";

src/maglev/maglev-ir.h

@@ -28,6 +28,7 @@ class ProcessingState;
 class MaglevCodeGenState;
 class MaglevGraphLabeller;
 class MaglevVregAllocationState;
+class CompactInterpreterFrameState;
 
 // Nodes are either
 // 1. side-effecting or value-holding SSA nodes in the body of basic blocks, or
@@ -80,7 +81,6 @@ class MaglevVregAllocationState;
   V(GapMove)         \
   V(SoftDeopt)       \
   V(StoreField)      \
-  V(StoreToFrame)    \
   VALUE_NODE_LIST(V)
 
 #define CONDITIONAL_CONTROL_NODE_LIST(V) \
@@ -870,25 +870,21 @@ class Checkpoint : public FixedInputNodeT<0, Checkpoint> {
 
  public:
   explicit Checkpoint(size_t input_count, BytecodeOffset bytecode_position,
-                      bool accumulator_is_live, ValueNode* accumulator)
+                      CompactInterpreterFrameState* frame)
       : Base(input_count),
         bytecode_position_(bytecode_position),
-        accumulator_(accumulator_is_live ? accumulator : nullptr) {}
+        frame_(frame) {}
 
   BytecodeOffset bytecode_position() const { return bytecode_position_; }
-  bool is_used() const { return IsUsedBit::decode(bit_field_); }
-  void SetUsed() { bit_field_ = IsUsedBit::update(bit_field_, true); }
-  ValueNode* accumulator() const { return accumulator_; }
+  const CompactInterpreterFrameState* frame() const { return frame_; }
 
   void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
   void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
 
  private:
-  using IsUsedBit = NextBitField<bool, 1>;
-
   const BytecodeOffset bytecode_position_;
-  ValueNode* const accumulator_;
+  const CompactInterpreterFrameState* const frame_;
 };
 
 class SoftDeopt : public FixedInputNodeT<0, SoftDeopt> {
@@ -1022,26 +1018,6 @@ class LoadNamedGeneric : public FixedInputValueNodeT<2, LoadNamedGeneric> {
   const compiler::NameRef name_;
 };
 
-class StoreToFrame : public FixedInputNodeT<0, StoreToFrame> {
-  using Base = FixedInputNodeT<0, StoreToFrame>;
-
- public:
-  StoreToFrame(size_t input_count, ValueNode* value,
-               interpreter::Register target)
-      : Base(input_count), value_(value), target_(target) {}
-
-  interpreter::Register target() const { return target_; }
-  ValueNode* value() const { return value_; }
-
-  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
-  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
-  void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
-
- private:
-  ValueNode* const value_;
-  const interpreter::Register target_;
-};
-
 class GapMove : public FixedInputNodeT<0, GapMove> {
   using Base = FixedInputNodeT<0, GapMove>;
 

src/maglev/maglev-regalloc.cc

@@ -270,8 +270,7 @@ void StraightForwardRegisterAllocator::AllocateRegisters(Graph* graph) {
               compiler::AllocatedOperand::cast(allocation));
           if (FLAG_trace_maglev_regalloc) {
             printing_visitor_->Process(
-                phi, ProcessingState(compilation_unit_, block_it_, nullptr,
-                                     nullptr, nullptr));
+                phi, ProcessingState(compilation_unit_, block_it_, nullptr));
             printing_visitor_->os()
                 << "phi (new reg) " << phi->result().operand() << std::endl;
           }
@@ -285,8 +284,7 @@ void StraightForwardRegisterAllocator::AllocateRegisters(Graph* graph) {
         phi->result().SetAllocated(phi->spill_slot());
         if (FLAG_trace_maglev_regalloc) {
           printing_visitor_->Process(
-              phi, ProcessingState(compilation_unit_, block_it_, nullptr,
-                                   nullptr, nullptr));
+              phi, ProcessingState(compilation_unit_, block_it_, nullptr));
           printing_visitor_->os()
               << "phi (stack) " << phi->result().operand() << std::endl;
         }
@@ -344,8 +342,7 @@ void StraightForwardRegisterAllocator::AllocateNode(Node* node) {
 
   if (FLAG_trace_maglev_regalloc) {
     printing_visitor_->Process(
-        node, ProcessingState(compilation_unit_, block_it_, nullptr, nullptr,
-                              nullptr));
+        node, ProcessingState(compilation_unit_, block_it_, nullptr));
     printing_visitor_->os() << "live regs: ";
     PrintLiveRegs();
     printing_visitor_->os() << "\n";
@@ -514,8 +511,7 @@ void StraightForwardRegisterAllocator::AllocateControlNode(ControlNode* node,
 
   if (FLAG_trace_maglev_regalloc) {
     printing_visitor_->Process(
-        node, ProcessingState(compilation_unit_, block_it_, nullptr, nullptr,
-                              nullptr));
+        node, ProcessingState(compilation_unit_, block_it_, nullptr));
   }
 }
 
@@ -528,8 +524,7 @@ void StraightForwardRegisterAllocator::TryAllocateToInput(Phi* phi) {
         phi->result().SetAllocated(ForceAllocate(reg, phi));
         if (FLAG_trace_maglev_regalloc) {
           printing_visitor_->Process(
-              phi, ProcessingState(compilation_unit_, block_it_, nullptr,
-                                   nullptr, nullptr));
+              phi, ProcessingState(compilation_unit_, block_it_, nullptr));
           printing_visitor_->os()
               << "phi (reuse) " << input.operand() << std::endl;
         }

src/maglev/maglev-vreg-allocator.h

@@ -26,8 +26,6 @@ class MaglevVregAllocationState {
 
 class MaglevVregAllocator {
  public:
-  static constexpr bool kNeedsCheckpointStates = true;
-
   void PreProcessGraph(MaglevCompilationUnit*, Graph* graph) {}
   void PostProcessGraph(MaglevCompilationUnit*, Graph* graph) {
     for (BasicBlock* block : *graph) {