[llvm] [LiveDebugVariables] Stop holding SlotIndexes for erased instructions (PR #224467)

Petar Jovanovic via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 29 17:14:15 PDT 2026


https://github.com/petar-jovanovic updated https://github.com/llvm/llvm-project/pull/224467

>From 22a246e3e7f66649ce6cc827b38c3a32f327171f Mon Sep 17 00:00:00 2001
From: Petar Jovanovic <petar.jovanovic at amd.com>
Date: Thu, 17 Sep 2026 23:59:32 +0200
Subject: [PATCH 1/2] [SlotIndexes] Add queries for stale indexes

An erased instruction leaves its index list entry in place, making the
index indistinguishable from a block boundary entry. Add
isBlockBoundaryIndex() and isStaleIndex() to tell the two apart, and
canonicalizeIndex() to resolve a stale index to the closest preceding
instruction's register slot, or the block start if none survives.

NFC. No caller yet. LiveDebugVariables is next.
---
 llvm/include/llvm/CodeGen/SlotIndexes.h    |  14 ++
 llvm/lib/CodeGen/SlotIndexes.cpp           |  29 +++
 llvm/unittests/CodeGen/CMakeLists.txt      |   1 +
 llvm/unittests/CodeGen/SlotIndexesTest.cpp | 207 +++++++++++++++++++++
 4 files changed, 251 insertions(+)
 create mode 100644 llvm/unittests/CodeGen/SlotIndexesTest.cpp

diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 5204ef0dd87cb..7747563534f95 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -396,6 +396,20 @@ class raw_ostream;
       return index.listEntry()->getInstr();
     }
 
+    /// Returns true if \p Idx refers to an entry created to mark a basic block
+    /// boundary. Such entries never have an instruction attached.
+    LLVM_ABI bool isBlockBoundaryIndex(SlotIndex Idx) const;
+
+    /// Returns true if \p Idx refers to an instruction that has been erased.
+    bool isStaleIndex(SlotIndex Idx) const {
+      return !getInstructionFromIndex(Idx) && !isBlockBoundaryIndex(Idx);
+    }
+
+    /// Returns the register slot of the closest instruction preceding a stale
+    /// \p Idx, or the start index of its basic block if there is none. Returns
+    /// \p Idx unchanged if it is not stale.
+    LLVM_ABI SlotIndex canonicalizeIndex(SlotIndex Idx) const;
+
     /// Returns the next non-null index, if one exists.
     /// Otherwise returns getLastIndex().
     SlotIndex getNextNonNullIndex(SlotIndex Index) {
diff --git a/llvm/lib/CodeGen/SlotIndexes.cpp b/llvm/lib/CodeGen/SlotIndexes.cpp
index 500c396bc099f..3787c8e032c6a 100644
--- a/llvm/lib/CodeGen/SlotIndexes.cpp
+++ b/llvm/lib/CodeGen/SlotIndexes.cpp
@@ -122,6 +122,35 @@ void SlotIndexes::analyze(MachineFunction &fn) {
   LLVM_DEBUG(mf->print(dbgs(), this));
 }
 
+bool SlotIndexes::isBlockBoundaryIndex(SlotIndex Idx) const {
+  if (getInstructionFromIndex(Idx))
+    return false;
+
+  // Adjacent blocks share a boundary entry, so a boundary is either the start
+  // index of the block Idx falls in or the end index of the last block. A block
+  // dropped by removeMBBFromMaps() is gone from idx2MBBMap, so its start entry
+  // classifies as stale.
+  assert(!idx2MBBMap.empty() && "Index -> MBB mapping is empty");
+  SlotIndex Base = Idx.getBaseIndex();
+  MBBIndexIterator I = std::prev(getMBBUpperBound(Base));
+  return Base == I->first || Base == getMBBEndIdx(I->second);
+}
+
+SlotIndex SlotIndexes::canonicalizeIndex(SlotIndex Idx) const {
+  if (!isStaleIndex(Idx))
+    return Idx;
+
+  // The block start is a boundary entry, so it bounds the walk.
+  SlotIndex BlockStart = getMBBStartIdx(getMBBFromIndex(Idx));
+  IndexList::iterator I = Idx.listEntry()->getIterator();
+  while (&*I != BlockStart.listEntry()) {
+    --I;
+    if (I->getInstr())
+      return SlotIndex(&*I, SlotIndex::Slot_Register);
+  }
+  return BlockStart;
+}
+
 void SlotIndexes::removeMachineInstrFromMaps(MachineInstr &MI,
                                              bool AllowBundled) {
   assert((AllowBundled || !MI.isBundledWithPred()) &&
diff --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index ea8028f4cf979..cee089e335c9b 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -52,6 +52,7 @@ add_llvm_unittest(CodeGenTests
   SelectionDAGCSETest.cpp
   SelectionDAGNodeConstructionTest.cpp
   SelectionDAGPatternMatchTest.cpp
+  SlotIndexesTest.cpp
   TypeTraitsTest.cpp
   TargetOptionsTest.cpp
   TestAsmPrinter.cpp
diff --git a/llvm/unittests/CodeGen/SlotIndexesTest.cpp b/llvm/unittests/CodeGen/SlotIndexesTest.cpp
new file mode 100644
index 0000000000000..1725fbe2190fe
--- /dev/null
+++ b/llvm/unittests/CodeGen/SlotIndexesTest.cpp
@@ -0,0 +1,207 @@
+//===- SlotIndexesTest.cpp ------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/SlotIndexes.h"
+#include "CodeGenTestBase.h"
+#include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/Config/Targets.h"
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class SlotIndexesTest : public CodeGenTestBase {
+public:
+  static void SetUpTestCase() {
+#if LLVM_HAS_AMDGPU_TARGET
+    LLVMInitializeAMDGPUTargetInfo();
+    LLVMInitializeAMDGPUTarget();
+    LLVMInitializeAMDGPUTargetMC();
+#else
+    GTEST_SKIP();
+#endif
+  }
+
+  void SetUp() override { setUpImpl("amdgcn--", "", ""); }
+
+  /// Erases \p MI the way codegen passes do, leaving its entry behind.
+  static void erase(MachineInstr &MI, SlotIndexes &SI) {
+    SI.removeMachineInstrFromMaps(MI);
+    MI.eraseFromParent();
+  }
+};
+
+constexpr StringRef TwoBlockMIR = R"(
+---
+name: func
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    S_NOP 0
+    S_NOP 1
+    S_NOP 2
+
+  bb.1:
+    S_NOP 3
+    S_NOP 4
+    S_ENDPGM 0
+...
+)";
+
+// The first block's start is the first list entry and the last block's end is
+// the only boundary that is not also a block start.
+TEST_F(SlotIndexesTest, BoundariesAreNotStale) {
+  ASSERT_TRUE(parseMIR(TwoBlockMIR));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  for (MachineBasicBlock &MBB : MF) {
+    SlotIndex Start = SI.getMBBStartIdx(&MBB);
+    SlotIndex End = SI.getMBBEndIdx(&MBB);
+    EXPECT_TRUE(SI.isBlockBoundaryIndex(Start));
+    EXPECT_TRUE(SI.isBlockBoundaryIndex(End));
+    EXPECT_FALSE(SI.isStaleIndex(Start));
+    EXPECT_FALSE(SI.isStaleIndex(End));
+    EXPECT_EQ(SI.canonicalizeIndex(Start), Start);
+    EXPECT_EQ(SI.canonicalizeIndex(End), End);
+  }
+}
+
+TEST_F(SlotIndexesTest, LiveIndexesAreUnchanged) {
+  ASSERT_TRUE(parseMIR(TwoBlockMIR));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  for (MachineBasicBlock &MBB : MF) {
+    for (MachineInstr &MI : MBB) {
+      SlotIndex Base = SI.getInstructionIndex(MI);
+      EXPECT_FALSE(SI.isBlockBoundaryIndex(Base));
+      EXPECT_FALSE(SI.isStaleIndex(Base));
+      for (SlotIndex Idx :
+           {Base, Base.getRegSlot(true), Base.getRegSlot(), Base.getDeadSlot()})
+        EXPECT_EQ(SI.canonicalizeIndex(Idx), Idx);
+    }
+  }
+}
+
+TEST_F(SlotIndexesTest, ErasedInstrResolvesToPrecedingInstr) {
+  ASSERT_TRUE(parseMIR(TwoBlockMIR));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  MachineBasicBlock &MBB0 = *MF.getBlockNumbered(0);
+  SlotIndex First = SI.getInstructionIndex(*MBB0.begin());
+  SlotIndex Second = SI.getInstructionIndex(*std::next(MBB0.begin()));
+
+  erase(*std::next(MBB0.begin()), SI);
+
+  EXPECT_TRUE(SI.isStaleIndex(Second));
+  EXPECT_FALSE(SI.isBlockBoundaryIndex(Second));
+  EXPECT_EQ(SI.canonicalizeIndex(Second), First.getRegSlot());
+  // Every slot resolves alike, and the dead slot is left free to grow into.
+  for (SlotIndex Idx : {Second, Second.getRegSlot(true), Second.getRegSlot(),
+                        Second.getDeadSlot()})
+    EXPECT_EQ(SI.canonicalizeIndex(Idx), First.getRegSlot());
+  EXPECT_LT(SI.canonicalizeIndex(Second), First.getDeadSlot());
+}
+
+TEST_F(SlotIndexesTest, RunOfErasedInstrsResolvesToSameIndex) {
+  ASSERT_TRUE(parseMIR(TwoBlockMIR));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  MachineBasicBlock &MBB0 = *MF.getBlockNumbered(0);
+  SlotIndex First = SI.getInstructionIndex(*MBB0.begin());
+  SlotIndex Second = SI.getInstructionIndex(*std::next(MBB0.begin()));
+  SlotIndex Third = SI.getInstructionIndex(*std::next(MBB0.begin(), 2));
+
+  erase(*std::next(MBB0.begin(), 2), SI);
+  erase(*std::next(MBB0.begin()), SI);
+
+  EXPECT_TRUE(SI.isStaleIndex(Second));
+  EXPECT_TRUE(SI.isStaleIndex(Third));
+  EXPECT_EQ(SI.canonicalizeIndex(Second), First.getRegSlot());
+  EXPECT_EQ(SI.canonicalizeIndex(Third), First.getRegSlot());
+}
+
+// The block start is shared with the previous block's end index, so the result
+// must still report as belonging to the erased instruction's own block.
+TEST_F(SlotIndexesTest, ErasedBlockPrefixResolvesToBlockStart) {
+  ASSERT_TRUE(parseMIR(TwoBlockMIR));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  MachineBasicBlock &MBB1 = *MF.getBlockNumbered(1);
+  SlotIndex Start = SI.getMBBStartIdx(&MBB1);
+  SlotIndex FirstIdx = SI.getInstructionIndex(*MBB1.begin());
+
+  erase(*MBB1.begin(), SI);
+
+  SlotIndex Canonical = SI.canonicalizeIndex(FirstIdx);
+  EXPECT_EQ(Canonical, Start);
+  EXPECT_FALSE(SI.isStaleIndex(Canonical));
+  EXPECT_EQ(SI.getMBBFromIndex(Canonical), &MBB1);
+  // There is still a slot above it to grow into.
+  EXPECT_FALSE(SI.isStaleIndex(Canonical.getNextSlot()));
+  EXPECT_GT(Canonical.getNextSlot(), SI.getMBBEndIdx(MF.getBlockNumbered(0)));
+}
+
+TEST_F(SlotIndexesTest, ErasedEntryBlockResolvesToZeroIndex) {
+  ASSERT_TRUE(parseMIR(TwoBlockMIR));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  MachineBasicBlock &MBB0 = *MF.getBlockNumbered(0);
+  SlotIndex Start = SI.getMBBStartIdx(&MBB0);
+  SmallVector<SlotIndex> Indexes;
+  for (MachineInstr &MI : MBB0)
+    Indexes.push_back(SI.getInstructionIndex(MI));
+
+  for (MachineInstr &MI : make_early_inc_range(MBB0))
+    erase(MI, SI);
+
+  for (SlotIndex Idx : Indexes) {
+    EXPECT_TRUE(SI.isStaleIndex(Idx));
+    EXPECT_EQ(SI.canonicalizeIndex(Idx), Start);
+  }
+}
+
+// One block means one entry in the index -> MBB map, so the upper-bound search
+// always lands on its end.
+TEST_F(SlotIndexesTest, SingleBlockFunction) {
+  ASSERT_TRUE(parseMIR(R"(
+---
+name: func
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    S_NOP 0
+    S_ENDPGM 0
+...
+)"));
+  MachineFunction &MF = getMF("func");
+  SlotIndexes &SI = MFAM.getResult<SlotIndexesAnalysis>(MF);
+
+  MachineBasicBlock &MBB = MF.front();
+  SlotIndex Start = SI.getMBBStartIdx(&MBB);
+  SlotIndex Nop = SI.getInstructionIndex(*MBB.begin());
+  SlotIndex End = SI.getInstructionIndex(MBB.back());
+
+  EXPECT_TRUE(SI.isBlockBoundaryIndex(Start));
+  EXPECT_TRUE(SI.isBlockBoundaryIndex(SI.getMBBEndIdx(&MBB)));
+
+  erase(*MBB.begin(), SI);
+  EXPECT_TRUE(SI.isStaleIndex(Nop));
+  EXPECT_EQ(SI.canonicalizeIndex(Nop), Start);
+  EXPECT_FALSE(SI.isStaleIndex(End));
+  EXPECT_TRUE(SI.isBlockBoundaryIndex(SI.getMBBEndIdx(&MBB)));
+}
+
+} // namespace

>From 7c31b617a87c6bc19087ec3be98c8d2b9726199b Mon Sep 17 00:00:00 2001
From: Petar Jovanovic <petar.jovanovic at amd.com>
Date: Fri, 18 Sep 2026 00:17:15 +0200
Subject: [PATCH 2/2] [LiveDebugVariables] Repair stale SlotIndexes

The analysis keeps its indexes from before the first register allocator
until DBG_VALUEs are emitted, by which point passes in between have
erased some of the instructions they point at. Resolve them at the
start of each allocator run and before emitting.

SlotIndexes can then reclaim the entries of erased instructions without
sparing the ones held here, which would have made generated code depend
on -g. Emitted locations are unchanged, except that intervals resolving
to one position now emit a single DBG_VALUE rather than identical
consecutive ones.
---
 .../include/llvm/CodeGen/LiveDebugVariables.h |   8 +
 llvm/lib/CodeGen/LiveDebugVariables.cpp       | 137 ++++++++++++++++++
 llvm/lib/CodeGen/RegAllocGreedy.cpp           |   7 +
 .../test/CodeGen/X86/debug-spilled-snippet.ll |   7 +-
 .../CodeGen/X86/debug-spilled-snippet.mir     |   7 +-
 .../live-debug-vars-stale-slot-indexes.ll     |  63 ++++++++
 .../live-debug-vars-unused-arg-debugonly.mir  |  12 +-
 7 files changed, 233 insertions(+), 8 deletions(-)
 create mode 100644 llvm/test/DebugInfo/AMDGPU/live-debug-vars-stale-slot-indexes.ll

diff --git a/llvm/include/llvm/CodeGen/LiveDebugVariables.h b/llvm/include/llvm/CodeGen/LiveDebugVariables.h
index d9d2a2f6a08e6..c075dd7c6915b 100644
--- a/llvm/include/llvm/CodeGen/LiveDebugVariables.h
+++ b/llvm/include/llvm/CodeGen/LiveDebugVariables.h
@@ -30,6 +30,7 @@ namespace llvm {
 
 template <typename T> class ArrayRef;
 class LiveIntervals;
+class SlotIndexes;
 class VirtRegMap;
 
 class LiveDebugVariables {
@@ -47,6 +48,13 @@ class LiveDebugVariables {
   LLVM_ABI void splitRegister(Register OldReg, ArrayRef<Register> NewRegs,
                               LiveIntervals &LIS);
 
+  /// canonicalizeIndexes - Replace every SlotIndex held by this analysis that
+  /// refers to an erased instruction. Described locations do not change, as a
+  /// stale index already resolves to the same position at the point of use, but
+  /// intervals resolving to one position now emit a single DBG_VALUE rather
+  /// than identical consecutive ones.
+  LLVM_ABI void canonicalizeIndexes(const SlotIndexes &SI);
+
   /// emitDebugValues - Emit new DBG_VALUE instructions reflecting the changes
   /// that happened during register allocation.
   /// @param VRM Rename virtual registers according to map.
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index b4347cad2c1b2..4d9146f830ce6 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -73,6 +73,9 @@ EnableLDV("live-debug-variables", cl::init(true),
 
 STATISTIC(NumInsertedDebugValues, "Number of DBG_VALUEs inserted");
 STATISTIC(NumInsertedDebugLabels, "Number of DBG_LABELs inserted");
+STATISTIC(NumStaleIndexes, "Number of stale SlotIndexes repaired");
+STATISTIC(NumMergedIntervals,
+          "Number of debug value intervals merged while repairing indexes");
 
 char LiveDebugVariablesWrapperLegacy::ID = 0;
 
@@ -472,6 +475,9 @@ class UserValue {
   bool splitRegister(Register OldReg, ArrayRef<Register> NewRegs,
                      LiveIntervals &LIS);
 
+  /// Replace the stale indexes in locInts and trimmedDefs.
+  void canonicalizeIndexes(const SlotIndexes &SI);
+
   /// Rewrite virtual register locations according to the provided virtual
   /// register map. Record the stack slot offsets for the locations that
   /// were spilled.
@@ -520,6 +526,15 @@ class UserLabel {
   void emitDebugLabel(LiveIntervals &LIS, const TargetInstrInfo &TII,
                       BlockSkipInstsMap &BBSkipInstsMap);
 
+  /// Replace loc if it is stale, and report whether it was.
+  bool canonicalizeIndex(const SlotIndexes &SI) {
+    bool WasStale = SI.isStaleIndex(loc);
+    loc = SI.canonicalizeIndex(loc);
+    assert(!SI.isStaleIndex(loc) &&
+           "Canonicalized label still refers to an erased instruction");
+    return WasStale;
+  }
+
   /// Return DebugLoc of this UserLabel.
   const DebugLoc &getDebugLoc() { return dl; }
 
@@ -665,6 +680,9 @@ class LiveDebugVariables::LDVImpl {
   /// Replace all references to OldReg with NewRegs.
   void splitRegister(Register OldReg, ArrayRef<Register> NewRegs);
 
+  /// Replace every stale index held by this analysis.
+  void canonicalizeIndexes(const SlotIndexes &SI);
+
   /// Recreate DBG_VALUE instruction from data structures.
   void emitDebugValues(VirtRegMap *VRM);
 
@@ -1549,6 +1567,122 @@ splitRegister(Register OldReg, ArrayRef<Register> NewRegs, LiveIntervals &LIS) {
     PImpl->splitRegister(OldReg, NewRegs);
 }
 
+//===----------------------------------------------------------------------===//
+//                        Stale Index Canonicalization
+//===----------------------------------------------------------------------===//
+
+void UserValue::canonicalizeIndexes(const SlotIndexes &SI) {
+  unsigned NumStale = 0;
+  for (LocMap::const_iterator I = locInts.begin(); I.valid(); ++I)
+    NumStale += SI.isStaleIndex(I.start()) + SI.isStaleIndex(I.stop());
+  for (SlotIndex Idx : trimmedDefs)
+    NumStale += SI.isStaleIndex(Idx);
+  NumStaleIndexes += NumStale;
+
+  if (NumStale) {
+    // trimmedDefs is looked up by interval start. Remapping it here is safe:
+    // trimmed starts are block slots, so the Stop < Start case below cannot
+    // reach them, and a merge drops a start that then matches nothing.
+    if (!trimmedDefs.empty()) {
+      SmallVector<SlotIndex, 2> Defs(trimmedDefs.begin(), trimmedDefs.end());
+      trimmedDefs.clear();
+      for (SlotIndex Idx : Defs)
+        trimmedDefs.insert(SI.canonicalizeIndex(Idx));
+    }
+
+    // Rebuild rather than move the keys of the existing map: it has to stay
+    // ordered and non-empty at every step, which canonicalization does not
+    // respect.
+    struct CanonicalInterval {
+      SlotIndex Start;
+      SlotIndex Stop;
+      DbgVariableValue Value;
+    };
+    SmallVector<CanonicalInterval, 8> Intervals;
+
+    for (LocMap::const_iterator I = locInts.begin(); I.valid(); ++I) {
+      SlotIndex Start = SI.canonicalizeIndex(I.start());
+      SlotIndex Stop = SI.canonicalizeIndex(I.stop());
+
+      // A stale stop can land below a start that sat on the same instruction's
+      // dead slot. Both resolve to the same insert location.
+      if (Stop < Start)
+        Start = Stop;
+
+      if (!Intervals.empty()) {
+        CanonicalInterval &Prev = Intervals.back();
+        if (Start <= Prev.Start) {
+          // Both DBG_VALUEs would be emitted at the same position, where the
+          // later one overrides the earlier before it covers anything.
+          Prev.Stop = std::max(Prev.Stop, Stop);
+          Prev.Value = I.value();
+          ++NumMergedIntervals;
+          continue;
+        }
+        Prev.Stop = std::min(Prev.Stop, Start);
+      }
+      Intervals.push_back({Start, Stop, I.value()});
+    }
+
+    // The map cannot hold empty intervals. Use the smallest extent there is: a
+    // wider one would span more blocks, and emitDebugValues() emits a DBG_VALUE
+    // per block covered.
+    for (CanonicalInterval &Interval : Intervals) {
+      if (Interval.Stop > Interval.Start)
+        continue;
+      Interval.Stop = Interval.Start.getNextSlot();
+      assert(!SI.isStaleIndex(Interval.Stop) &&
+             "No room left for a canonicalized interval");
+    }
+
+    locInts.clear();
+    for (const CanonicalInterval &Interval : Intervals)
+      locInts.insert(Interval.Start, Interval.Stop, Interval.Value);
+  }
+
+#ifndef NDEBUG
+  for (LocMap::const_iterator I = locInts.begin(); I.valid(); ++I)
+    assert(!SI.isStaleIndex(I.start()) && !SI.isStaleIndex(I.stop()) &&
+           "Canonicalized interval still refers to an erased instruction");
+  for (SlotIndex Idx : trimmedDefs)
+    assert(!SI.isStaleIndex(Idx) &&
+           "Canonicalized trimmed def still refers to an erased instruction");
+#endif
+}
+
+void LiveDebugVariables::LDVImpl::canonicalizeIndexes(const SlotIndexes &SI) {
+  for (auto &userValue : userValues)
+    userValue->canonicalizeIndexes(SI);
+  for (auto &userLabel : userLabels)
+    NumStaleIndexes += userLabel->canonicalizeIndex(SI);
+
+  // emitDebugValues() walks forwards to the next live instruction, which is the
+  // same iterator as inserting after the preceding one, and stays inside
+  // InstrPos::MBB. Canonicalization is monotonic, so entries sharing a slot are
+  // still re-inserted as one batch.
+  for (InstrPos &Stashed : StashedDebugInstrs) {
+    if (!SI.isStaleIndex(Stashed.Idx))
+      continue;
+    ++NumStaleIndexes;
+    Stashed.Idx = SI.canonicalizeIndex(Stashed.Idx);
+    assert(!SI.isStaleIndex(Stashed.Idx) &&
+           "Canonicalized debug instr still refers to an erased instruction");
+  }
+
+#ifndef NDEBUG
+  // PHI positions are block starts, which are boundaries. A block erased by
+  // removeMBBFromMaps() would make one look stale.
+  for (const auto &P : PHIValToPos)
+    assert(!SI.isStaleIndex(P.second.SI) &&
+           "PHI position refers to an erased instruction");
+#endif
+}
+
+void LiveDebugVariables::canonicalizeIndexes(const SlotIndexes &SI) {
+  if (PImpl)
+    PImpl->canonicalizeIndexes(SI);
+}
+
 void UserValue::rewriteLocations(VirtRegMap &VRM, const MachineFunction &MF,
                                  const TargetInstrInfo &TII,
                                  const TargetRegisterInfo &TRI,
@@ -1853,6 +1987,9 @@ void LiveDebugVariables::LDVImpl::emitDebugValues(VirtRegMap *VRM) {
   if (!MF)
     return;
 
+  // Instructions may have been erased since the last allocator run.
+  canonicalizeIndexes(*LIS->getSlotIndexes());
+
   BlockSkipInstsMap BBSkipInstsMap;
   const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
   SpillOffsetMap SpillOffsets;
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index af4fc60fe5ec3..03d19fe732ca7 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -2959,6 +2959,13 @@ bool RAGreedy::run(MachineFunction &mf) {
   if (!hasVirtRegAlloc())
     return false;
 
+  // Passes that ran since the analysis was built may have erased instructions
+  // it holds indexes for. This only makes it clean here and in
+  // emitDebugValues(): splitting stales indexes again mid-run, so reclaiming
+  // erased entries needs its own entry point, not a hook in packIndexes(),
+  // which renumberIndexes() also reaches.
+  DebugVars->canonicalizeIndexes(*Indexes);
+
   // Renumber to get accurate and consistent results from
   // SlotIndexes::getApproxInstrDistance.
   Indexes->packIndexes();
diff --git a/llvm/test/CodeGen/X86/debug-spilled-snippet.ll b/llvm/test/CodeGen/X86/debug-spilled-snippet.ll
index 96d5d9812325f..ef78409bacdc0 100644
--- a/llvm/test/CodeGen/X86/debug-spilled-snippet.ll
+++ b/llvm/test/CodeGen/X86/debug-spilled-snippet.ll
@@ -2,9 +2,12 @@
 
 ; There should be multiple debug values for this variable after regalloc. The
 ; value has been spilled, but we shouldn't lose track of the location because
-; of this.
+; of this. LiveDebugVariables::canonicalizeIndexes() merges the intervals that
+; resolve to one position, so only the last of them emits a DBG_VALUE; the
+; earlier ones were overridden before covering any instruction.
 
-; CHECK-COUNT-4: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
+; CHECK-COUNT-3: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
+; CHECK-NOT: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
 
 define void @main(i32 %call, i32 %xor.i, i1 %tobool4.not, i32 %.pre) #0 !dbg !4 {
 entry:
diff --git a/llvm/test/CodeGen/X86/debug-spilled-snippet.mir b/llvm/test/CodeGen/X86/debug-spilled-snippet.mir
index d4e4a720bf5c3..2aa0d94e8f838 100644
--- a/llvm/test/CodeGen/X86/debug-spilled-snippet.mir
+++ b/llvm/test/CodeGen/X86/debug-spilled-snippet.mir
@@ -2,9 +2,12 @@
 
 # There should be multiple debug values for this variable after regalloc. The
 # value has been spilled, but we shouldn't lose track of the location because
-# of this.
+# of this. LiveDebugVariables::canonicalizeIndexes() merges the intervals that
+# resolve to one position, so only the last of them emits a DBG_VALUE; the
+# earlier ones were overridden before covering any instruction.
 
-# CHECK-COUNT-4: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
+# CHECK-COUNT-3: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
+# CHECK-NOT: DBG_VALUE $ebp, 0, !6, !DIExpression(DW_OP_constu, 16, DW_OP_minus), debug-location !10
 
 --- |
   
diff --git a/llvm/test/DebugInfo/AMDGPU/live-debug-vars-stale-slot-indexes.ll b/llvm/test/DebugInfo/AMDGPU/live-debug-vars-stale-slot-indexes.ll
new file mode 100644
index 0000000000000..2979cba558faa
--- /dev/null
+++ b/llvm/test/DebugInfo/AMDGPU/live-debug-vars-stale-slot-indexes.ll
@@ -0,0 +1,63 @@
+; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx90a -O3 -stop-after=virtregrewriter,2 < %s \
+; RUN:   | FileCheck %s --implicit-check-not=DBG_VALUE
+
+; Check that each variable gets one DBG_VALUE per location it occupies and no
+; more. LiveDebugVariables is built before the SGPR allocator and still holds
+; SlotIndexes when the VGPR allocator spills %v1 several passes later. The two
+; intervals it recorded for %v1 around the spill resolve onto the same position,
+; and canonicalizeIndexes() merges them, so one DBG_VALUE is emitted there.
+
+; CHECK-LABEL: name: partial_copy
+; CHECK:      DBG_VALUE $vgpr0_vgpr1_vgpr2_vgpr3, $noreg, ![[V0:[0-9]+]], !DIExpression()
+; CHECK:      DBG_VALUE $agpr0_agpr1_agpr2_agpr3, $noreg, ![[V0]], !DIExpression()
+; CHECK:      DBG_VALUE $vgpr0_vgpr1, $noreg, ![[V1:[0-9]+]], !DIExpression()
+; CHECK-NEXT: SI_SPILL_AV64_SAVE
+; CHECK-NEXT: DBG_VALUE %stack.0, 0, ![[V1]], !DIExpression()
+; CHECK-NEXT: GLOBAL_STORE_DWORDX4
+; CHECK:      DBG_VALUE $vgpr0_vgpr1_vgpr2_vgpr3, $noreg, ![[MAI:[0-9]+]], !DIExpression()
+; CHECK-NEXT: SI_SPILL_AV64_RESTORE
+; CHECK-NEXT: DBG_VALUE $agpr0_agpr1, $noreg, ![[V1]], !DIExpression()
+
+define amdgpu_kernel void @partial_copy(<4 x i32> %arg) #0 !dbg !5 {
+  call void asm sideeffect "; use $0", "a"(i32 poison), !dbg !13
+  %v0 = call <4 x i32> asm sideeffect "; def $0", "=v"(), !dbg !14
+    #dbg_value(<4 x i32> %v0, !9, !DIExpression(), !14)
+  %v1 = call <2 x i32> asm sideeffect "; def $0", "=v"(), !dbg !15
+    #dbg_value(<2 x i32> %v1, !11, !DIExpression(), !15)
+  %mai = tail call <4 x i32> @llvm.amdgcn.mfma.i32.4x4x4i8(i32 1, i32 2, <4 x i32> %arg, i32 0, i32 0, i32 0), !dbg !16
+    #dbg_value(<4 x i32> %mai, !12, !DIExpression(), !16)
+  store volatile <4 x i32> %v0, ptr addrspace(1) poison, align 16, !dbg !17
+  store volatile <2 x i32> %v1, ptr addrspace(1) poison, align 8, !dbg !18
+  store volatile <4 x i32> %mai, ptr addrspace(1) poison, align 16, !dbg !19
+  ret void, !dbg !20
+}
+
+declare <4 x i32> @llvm.amdgcn.mfma.i32.4x4x4i8(i32, i32, <4 x i32>, i32, i32, i32)
+
+; The VGPR budget is what forces %v1 to be spilled.
+attributes #0 = { "amdgpu-num-vgpr"="5" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "llvm", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "live-debug-vars-stale-slot-indexes.c", directory: "/")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 7, !"Dwarf Version", i32 5}
+!5 = distinct !DISubprogram(name: "partial_copy", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+!6 = !DISubroutineType(types: !2)
+!7 = !DIBasicType(name: "ty128", size: 128, encoding: DW_ATE_unsigned)
+!8 = !{!9, !11, !12}
+!9 = !DILocalVariable(name: "v0", scope: !5, file: !1, line: 2, type: !7)
+!10 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_unsigned)
+!11 = !DILocalVariable(name: "v1", scope: !5, file: !1, line: 3, type: !10)
+!12 = !DILocalVariable(name: "mai", scope: !5, file: !1, line: 4, type: !7)
+!13 = !DILocation(line: 1, column: 1, scope: !5)
+!14 = !DILocation(line: 2, column: 1, scope: !5)
+!15 = !DILocation(line: 3, column: 1, scope: !5)
+!16 = !DILocation(line: 4, column: 1, scope: !5)
+!17 = !DILocation(line: 5, column: 1, scope: !5)
+!18 = !DILocation(line: 6, column: 1, scope: !5)
+!19 = !DILocation(line: 7, column: 1, scope: !5)
+!20 = !DILocation(line: 8, column: 1, scope: !5)
diff --git a/llvm/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir b/llvm/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir
index 0f108cd704ed2..1340de37cafe0 100644
--- a/llvm/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir
+++ b/llvm/test/DebugInfo/MIR/X86/live-debug-vars-unused-arg-debugonly.mir
@@ -152,12 +152,16 @@ body:             |
 # virtual registers # to $edi and $esi, so the ranges for argc/argv should
 # not cover the whole BB.
 #
+# They end at 48r rather than at the COPYs that killed them: the rewriter has
+# removed those as identity copies, so canonicalizeIndexes() resolves their
+# indexes back to the last instruction still in the map.
+#
 # CHECKDBG-LABEL: ********** EMITTING LIVE DEBUG VARIABLES **********
 # CHECKDBG-NEXT: !"argc,5"        [0B;0e): 0 Loc0=$edi
 # CHECKDBG-NEXT:         [0B;0e): 0 %bb.0-160B
 # CHECKDBG-NEXT: !"argv,5"        [0B;0e): 0 Loc0=$rsi
 # CHECKDBG-NEXT:         [0B;0e): 0 %bb.0-160B
-# CHECKDBG-NEXT: !"a0,7"  [16r;64r): 0 Loc0=%2
-# CHECKDBG-NEXT:         [16r;64r): 0 %bb.0-160B
-# CHECKDBG-NEXT: !"a1,8"  [32r;80r): 0 Loc0=%3
-# CHECKDBG-NEXT:         [32r;80r): 0 %bb.0-160B
+# CHECKDBG-NEXT: !"a0,7"  [16r;48r): 0 Loc0=%2
+# CHECKDBG-NEXT:         [16r;48r): 0 %bb.0-160B
+# CHECKDBG-NEXT: !"a1,8"  [32r;48r): 0 Loc0=%3
+# CHECKDBG-NEXT:         [32r;48r): 0 %bb.0-160B



More information about the llvm-commits mailing list