[regalloc] Fix hint position cache

Attempt to fix regressions introduced by:
https://chromium-review.googlesource.com/c/v8/v8/+/2235117
{current_hint_position_} is not precise enough and can be null even if
the range contains hints.
Instead, repurpose it during register allocation so that it always holds
the last hint position found for this top level live range. This ensures
that each use position is visited at most once even when the range is
split.

R=neis@chromium.org
CC=​sigurds@chromium.org

Bug: v8:10533, chromium:1093435
Change-Id: I21f3f12f061c3e4c7e845d161b19de7499200c0c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2239568
Commit-Queue: Thibaud Michaud <thibaudm@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68319}

src/compiler/backend/register-allocator.cc

@@ -473,12 +473,41 @@ RegisterKind LiveRange::kind() const {
   return IsFloatingPoint(representation()) ? FP_REGISTERS : GENERAL_REGISTERS;
 }
 
-UsePosition* LiveRange::FirstHintPosition(int* register_index) const {
-  if (!TopLevel()->current_hint_position()) return nullptr;
-  for (UsePosition* pos = first_pos_; pos != nullptr; pos = pos->next()) {
-    if (pos->HintRegister(register_index)) return pos;
+UsePosition* LiveRange::FirstHintPosition(int* register_index) {
+  if (!first_pos_) return nullptr;
+  if (current_hint_position_) {
+    if (current_hint_position_->pos() < first_pos_->pos()) {
+      current_hint_position_ = first_pos_;
+    }
+    if (current_hint_position_->pos() > End()) {
+      current_hint_position_ = nullptr;
+    }
   }
-  return nullptr;
+  bool needs_revisit = false;
+  UsePosition* pos = current_hint_position_;
+  for (; pos != nullptr; pos = pos->next()) {
+    if (pos->HintRegister(register_index)) {
+      break;
+    }
+    // Phi and use position hints can be assigned during allocation which
+    // would invalidate the cached hint position. Make sure we revisit them.
+    needs_revisit = needs_revisit ||
+                    pos->hint_type() == UsePositionHintType::kPhi ||
+                    pos->hint_type() == UsePositionHintType::kUsePos;
+  }
+  if (!needs_revisit) {
+    current_hint_position_ = pos;
+  }
+#ifdef DEBUG
+  UsePosition* pos_check = first_pos_;
+  for (; pos_check != nullptr; pos_check = pos_check->next()) {
+    if (pos_check->HasHint()) {
+      break;
+    }
+  }
+  CHECK_EQ(pos, pos_check);
+#endif
+  return pos;
 }
 
 UsePosition* LiveRange::NextUsePosition(LifetimePosition start) const {
@@ -685,6 +714,7 @@ UsePosition* LiveRange::DetachAt(LifetimePosition position, LiveRange* result,
     first_pos_ = nullptr;
   }
   result->first_pos_ = use_after;
+  result->current_hint_position_ = current_hint_position_;
 
   // Discard cached iteration state. It might be pointing
   // to the use that no longer belongs to this live range.
@@ -694,6 +724,7 @@ UsePosition* LiveRange::DetachAt(LifetimePosition position, LiveRange* result,
   if (connect_hints == ConnectHints && use_before != nullptr &&
       use_after != nullptr) {
     use_after->SetHint(use_before);
+    result->current_hint_position_ = use_after;
   }
 #ifdef DEBUG
   VerifyChildStructure();
@@ -2408,8 +2439,6 @@ void LiveRangeBuilder::ProcessInstructions(const InstructionBlock* block,
           if (to_range->is_phi()) {
             phi_vreg = to_vreg;
             if (to_range->is_non_loop_phi()) {
-              DCHECK_EQ(to_range->current_hint_position(),
-                        to_range->FirstHintPosition());
               hint = to_range->current_hint_position();
               hint_type = hint == nullptr ? UsePositionHintType::kNone
                                           : UsePositionHintType::kUsePos;
@@ -2663,6 +2692,7 @@ void LiveRangeBuilder::BuildLiveRanges() {
         pos->set_type(new_type, true);
       }
     }
+    range->ResetCurrentHintPosition();
   }
   for (auto preassigned : data()->preassigned_slot_ranges()) {
     TopLevelLiveRange* range = preassigned.first;

src/compiler/backend/register-allocator.h

@@ -618,8 +618,9 @@ class V8_EXPORT_PRIVATE LiveRange : public NON_EXPORTED_BASE(ZoneObject) {
   LiveRange* SplitAt(LifetimePosition position, Zone* zone);
 
   // Returns nullptr when no register is hinted, otherwise sets register_index.
-  UsePosition* FirstHintPosition(int* register_index) const;
-  UsePosition* FirstHintPosition() const {
+  // Uses {current_hint_position_} as a cache, and tries to update it.
+  UsePosition* FirstHintPosition(int* register_index);
+  UsePosition* FirstHintPosition() {
     int register_index;
     return FirstHintPosition(&register_index);
   }
@@ -655,6 +656,7 @@ class V8_EXPORT_PRIVATE LiveRange : public NON_EXPORTED_BASE(ZoneObject) {
                             const InstructionOperand& spill_op);
   void SetUseHints(int register_index);
   void UnsetUseHints() { SetUseHints(kUnassignedRegister); }
+  void ResetCurrentHintPosition() { current_hint_position_ = first_pos_; }
 
   void Print(const RegisterConfiguration* config, bool with_children) const;
   void Print(bool with_children) const;
@@ -702,8 +704,7 @@ class V8_EXPORT_PRIVATE LiveRange : public NON_EXPORTED_BASE(ZoneObject) {
   mutable UsePosition* last_processed_use_;
   // Cache the last position splintering stopped at.
   mutable UsePosition* splitting_pointer_;
-  // This is used as a cache in BuildLiveRanges, and to quickly check for the
-  // absence of hints during register allocation.
+  // This is used as a cache in BuildLiveRanges and during register allocation.
   UsePosition* current_hint_position_;
   LiveRangeBundle* bundle_ = nullptr;
   // Next interval start, relative to the current linear scan position.