[maglev] Always use spill slots in lazy deopts

Lazy deopts are always after calls, so force them to spill their inputs.
This would normally be the case anyway, except for deferred calls, which
don't tell the register allocator to spill like normal calls do.

This makes lazy deopt regalloc always spill its inputs and use their
spill slot, but unlike calls, this doesn't additionally clear the
register, so subsequent nodes can continue using the register cached
value without having to reload it.

As drive-bys, fix the Throw* opcodes to have the Throw property, and use
detail::DeepForEachInput in a couple of extra locations (including for
lazy deopts).

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

src/maglev/maglev-compiler.cc

@@ -35,6 +35,7 @@
 #include "src/maglev/maglev-graph-verifier.h"
 #include "src/maglev/maglev-graph.h"
 #include "src/maglev/maglev-interpreter-frame-state.h"
+#include "src/maglev/maglev-ir-inl.h"
 #include "src/maglev/maglev-ir.h"
 #include "src/maglev/maglev-regalloc.h"
 #include "src/maglev/maglev-vreg-allocator.h"
@@ -175,56 +176,24 @@ class UseMarkingProcessor {
     }
   }
 
-  void MarkCheckpointNodes(NodeBase* node, const MaglevCompilationUnit& unit,
-                           const CheckpointedInterpreterState* checkpoint_state,
-                           InputLocation* input_locations,
-                           LoopUsedNodes* loop_used_nodes,
-                           const ProcessingState& state, int& index) {
-    if (checkpoint_state->parent) {
-      MarkCheckpointNodes(node, *unit.caller(), checkpoint_state->parent,
-                          input_locations, loop_used_nodes, state, index);
-    }
-
-    const CompactInterpreterFrameState* register_frame =
-        checkpoint_state->register_frame;
-    int use_id = node->id();
-
-    register_frame->ForEachValue(
-        unit, [&](ValueNode* node, interpreter::Register reg) {
-          MarkUse(node, use_id, &input_locations[index++], loop_used_nodes);
-        });
-  }
   void MarkCheckpointNodes(NodeBase* node, const EagerDeoptInfo* deopt_info,
                            LoopUsedNodes* loop_used_nodes,
                            const ProcessingState& state) {
-    int index = 0;
-    MarkCheckpointNodes(node, deopt_info->unit, &deopt_info->state,
-                        deopt_info->input_locations, loop_used_nodes, state,
-                        index);
+    int use_id = node->id();
+    detail::DeepForEachInput(
+        deopt_info,
+        [&](ValueNode* node, interpreter::Register reg, InputLocation* input) {
+          MarkUse(node, use_id, input, loop_used_nodes);
+        });
   }
   void MarkCheckpointNodes(NodeBase* node, const LazyDeoptInfo* deopt_info,
                            LoopUsedNodes* loop_used_nodes,
                            const ProcessingState& state) {
-    int index = 0;
-
-    if (deopt_info->state.parent) {
-      MarkCheckpointNodes(node, *deopt_info->unit.caller(),
-                          deopt_info->state.parent, deopt_info->input_locations,
-                          loop_used_nodes, state, index);
-    }
-
-    // Handle the top-of-frame info manually, since we have to handle the result
-    // location.
-    const CompactInterpreterFrameState* register_frame =
-        deopt_info->state.register_frame;
     int use_id = node->id();
-
-    register_frame->ForEachValue(
-        deopt_info->unit, [&](ValueNode* node, interpreter::Register reg) {
-          // Skip over the result location.
-          if (deopt_info->IsResultRegister(reg)) return;
-          MarkUse(node, use_id, &deopt_info->input_locations[index++],
-                  loop_used_nodes);
+    detail::DeepForEachInput(
+        deopt_info,
+        [&](ValueNode* node, interpreter::Register reg, InputLocation* input) {
+          MarkUse(node, use_id, input, loop_used_nodes);
         });
   }
 

src/maglev/maglev-ir-inl.h

@@ -30,10 +30,28 @@ void DeepForEachInputImpl(const MaglevCompilationUnit& unit,
 }
 
 template <typename Function>
-void DeepForEachInput(const EagerDeoptInfo* node, Function&& f) {
+void DeepForEachInput(const EagerDeoptInfo* deopt_info, Function&& f) {
   int index = 0;
-  DeepForEachInputImpl(node->unit, &node->state, node->input_locations, index,
-                       f);
+  DeepForEachInputImpl(deopt_info->unit, &deopt_info->state,
+                       deopt_info->input_locations, index, f);
+}
+
+template <typename Function>
+void DeepForEachInput(const LazyDeoptInfo* deopt_info, Function&& f) {
+  int index = 0;
+  if (deopt_info->state.parent) {
+    DeepForEachInputImpl(*deopt_info->unit.caller(), deopt_info->state.parent,
+                         deopt_info->input_locations, index, f);
+  }
+  // Handle the top-of-frame info separately, since we have to skip the result
+  // location.
+  deopt_info->state.register_frame->ForEachValue(
+      deopt_info->unit, [&](ValueNode* node, interpreter::Register reg) {
+        // Skip over the result location since it is irrelevant for lazy deopts
+        // (unoptimized code will recreate the result).
+        if (deopt_info->IsResultRegister(reg)) return;
+        f(node, reg, &deopt_info->input_locations[index++]);
+      });
 }
 
 }  // namespace detail

src/maglev/maglev-ir.cc

@@ -3278,7 +3278,7 @@ void ThrowReferenceErrorIfHole::GenerateCode(MaglevAssembler* masm,
         __ Move(kContextRegister, masm->native_context().object());
         __ Push(node->name().object());
         __ CallRuntime(Runtime::kThrowAccessedUninitializedVariable, 1);
-        masm->DefineLazyDeoptPoint(node->lazy_deopt_info());
+        masm->DefineExceptionHandlerAndLazyDeoptPoint(node);
         __ Abort(AbortReason::kUnexpectedReturnFromThrow);
       },
       this);
@@ -3302,7 +3302,7 @@ void ThrowSuperNotCalledIfHole::GenerateCode(MaglevAssembler* masm,
          ThrowSuperNotCalledIfHole* node) {
         __ Move(kContextRegister, masm->native_context().object());
         __ CallRuntime(Runtime::kThrowSuperNotCalled, 0);
-        masm->DefineLazyDeoptPoint(node->lazy_deopt_info());
+        masm->DefineExceptionHandlerAndLazyDeoptPoint(node);
         __ Abort(AbortReason::kUnexpectedReturnFromThrow);
       },
       this);
@@ -3326,7 +3326,7 @@ void ThrowSuperAlreadyCalledIfNotHole::GenerateCode(
          ThrowSuperAlreadyCalledIfNotHole* node) {
         __ Move(kContextRegister, masm->native_context().object());
         __ CallRuntime(Runtime::kThrowSuperAlreadyCalledError, 0);
-        masm->DefineLazyDeoptPoint(node->lazy_deopt_info());
+        masm->DefineExceptionHandlerAndLazyDeoptPoint(node);
         __ Abort(AbortReason::kUnexpectedReturnFromThrow);
       },
       this);
@@ -3350,7 +3350,7 @@ void ThrowIfNotSuperConstructor::GenerateCode(MaglevAssembler* masm,
         __ Push(ToRegister(node->function()));
         __ Move(kContextRegister, masm->native_context().object());
         __ CallRuntime(Runtime::kThrowNotSuperConstructor, 2);
-        masm->DefineLazyDeoptPoint(node->lazy_deopt_info());
+        masm->DefineExceptionHandlerAndLazyDeoptPoint(node);
         __ Abort(AbortReason::kUnexpectedReturnFromThrow);
       },
       this);

src/maglev/maglev-ir.h

@@ -3625,7 +3625,7 @@ class ThrowReferenceErrorIfHole
       : Base(bitfield), name_(name) {}
 
   static constexpr OpProperties kProperties =
-      OpProperties::LazyDeopt() | OpProperties::DeferredCall();
+      OpProperties::Throw() | OpProperties::DeferredCall();
 
   const compiler::NameRef& name() const { return name_; }
 
@@ -3645,7 +3645,7 @@ class ThrowSuperNotCalledIfHole
   explicit ThrowSuperNotCalledIfHole(uint64_t bitfield) : Base(bitfield) {}
 
   static constexpr OpProperties kProperties =
-      OpProperties::LazyDeopt() | OpProperties::DeferredCall();
+      OpProperties::Throw() | OpProperties::DeferredCall();
 
   Input& value() { return Node::input(0); }
 
@@ -3661,7 +3661,7 @@ class ThrowSuperAlreadyCalledIfNotHole
       : Base(bitfield) {}
 
   static constexpr OpProperties kProperties =
-      OpProperties::LazyDeopt() | OpProperties::DeferredCall();
+      OpProperties::Throw() | OpProperties::DeferredCall();
 
   Input& value() { return Node::input(0); }
 
@@ -3676,7 +3676,7 @@ class ThrowIfNotSuperConstructor
   explicit ThrowIfNotSuperConstructor(uint64_t bitfield) : Base(bitfield) {}
 
   static constexpr OpProperties kProperties =
-      OpProperties::LazyDeopt() | OpProperties::DeferredCall();
+      OpProperties::Throw() | OpProperties::DeferredCall();
 
   Input& constructor() { return Node::input(0); }
   Input& function() { return Node::input(1); }

src/maglev/maglev-regalloc.cc

@@ -513,26 +513,17 @@ void StraightForwardRegisterAllocator::UpdateUse(
 
 void StraightForwardRegisterAllocator::UpdateUse(
     const LazyDeoptInfo& deopt_info) {
-  const CompactInterpreterFrameState* checkpoint_state =
-      deopt_info.state.register_frame;
-  int index = 0;
-  // TODO(leszeks): This is missing parent recursion, fix it.
-  // See also: UpdateUse(EagerDeoptInfo&).
-  checkpoint_state->ForEachValue(
-      deopt_info.unit, [&](ValueNode* node, interpreter::Register reg) {
-        // Skip over the result location since it is irrelevant for lazy deopts
-        // (unoptimized code will recreate the result).
-        if (deopt_info.IsResultRegister(reg)) return;
+  detail::DeepForEachInput(
+      &deopt_info,
+      [&](ValueNode* node, interpreter::Register reg, InputLocation* input) {
         if (v8_flags.trace_maglev_regalloc) {
           printing_visitor_->os()
               << "- using " << PrintNodeLabel(graph_labeller(), node) << "\n";
         }
-        InputLocation* input = &deopt_info.input_locations[index++];
-        // We might have dropped this node without spilling it. Spill it now.
-        if (!node->has_register() && !node->is_loadable()) {
-          Spill(node);
-        }
-        input->InjectLocation(node->allocation());
+        // Lazy deopts always need spilling, and should always be loaded from
+        // their loadable slot.
+        Spill(node);
+        input->InjectLocation(node->loadable_slot());
         UpdateUse(node, input);
       });
 }

test/mjsunit/maglev/regress/regress-1364074.js

+// 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.
+//
+// Flags: --maglev
+
+class Base {
+}
+let Class = class extends Base {
+  constructor() {
+      super();
+  }
+};
+for (let i = 0; i < 100; i++) {
+    Class = class extends Class {
+      constructor() {
+        try {
+          super();
+          super();
+        } catch (e) {}
+      }
+    };
+}
+let instance = new Class();