[llvm] [LiveDebugVariables] Add basic verification (PR #79846)

[Jay Foad via llvm-commits]

https://github.com/jayfoad created https://github.com/llvm/llvm-project/pull/79846

Add a basic implementation of verifyAnalysis that just checks that the
analysis does not refer to any SlotIndexes for instructions that have
been deleted. This was useful for diagnosing some SlotIndexes-related
problems caused by #67038.- [SlotIndexes] Implement support for poison checks


>From 8e8eddaa0bc9cf04b15ec4b52eca572b46c122b0 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Thu, 19 Oct 2023 16:35:41 +0100
Subject: [PATCH 1/2] [SlotIndexes] Implement support for poison checks

In debug builds, mark slot indexes for deleted instructions as poisoned
and add an isPoisoned method to allow writing assertions elsewhere in
the compiler. This restores some of the functionality that was removed
by 4cf8da94198d.
---
 llvm/include/llvm/CodeGen/SlotIndexes.h | 31 +++++++++++++++++++++----
 llvm/lib/CodeGen/SlotIndexes.cpp        |  2 ++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 72f4a6876b6cb14..2b437a0649aac22 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -43,21 +43,40 @@ class raw_ostream;
   /// SlotIndex & SlotIndexes classes for the public interface to this
   /// information.
   class IndexListEntry : public ilist_node<IndexListEntry> {
-    MachineInstr *mi;
+#if NDEBUG
+    // Disable poison checks such that setPoison will do nothing and isPoisoned
+    // will return false.
+    static constexpr unsigned PoisonBits = 0;
+    static constexpr unsigned PoisonVal = 0;
+#else
+    static constexpr unsigned PoisonBits = 1;
+    static constexpr unsigned PoisonVal = 1;
+#endif
+
+    PointerIntPair<MachineInstr *, PoisonBits, unsigned> mi;
     unsigned index;
 
   public:
-    IndexListEntry(MachineInstr *mi, unsigned index) : mi(mi), index(index) {}
+    IndexListEntry(MachineInstr *mi, unsigned index) : mi(mi, 0), index(index) {
+    }
 
-    MachineInstr* getInstr() const { return mi; }
+    MachineInstr* getInstr() const { return mi.getPointer(); }
     void setInstr(MachineInstr *mi) {
-      this->mi = mi;
+      this->mi.setPointer(mi);
     }
 
     unsigned getIndex() const { return index; }
     void setIndex(unsigned index) {
       this->index = index;
     }
+
+    void setPoison() {
+      mi.setInt(PoisonVal);
+    }
+
+    bool isPoisoned() const {
+      return mi.getInt();
+    }
   };
 
   template <>
@@ -285,6 +304,10 @@ class raw_ostream;
     SlotIndex getPrevIndex() const {
       return SlotIndex(&*--listEntry()->getIterator(), getSlot());
     }
+
+    bool isPoisoned() const {
+      return listEntry()->isPoisoned();
+    }
   };
 
   inline raw_ostream& operator<<(raw_ostream &os, SlotIndex li) {
diff --git a/llvm/lib/CodeGen/SlotIndexes.cpp b/llvm/lib/CodeGen/SlotIndexes.cpp
index 8b80c6ccb438936..762aab2ec2557e3 100644
--- a/llvm/lib/CodeGen/SlotIndexes.cpp
+++ b/llvm/lib/CodeGen/SlotIndexes.cpp
@@ -126,6 +126,7 @@ void SlotIndexes::removeMachineInstrFromMaps(MachineInstr &MI,
   mi2iMap.erase(mi2iItr);
   // FIXME: Eventually we want to actually delete these indexes.
   MIEntry.setInstr(nullptr);
+  MIEntry.setPoison();
 }
 
 void SlotIndexes::removeSingleMachineInstrFromMaps(MachineInstr &MI) {
@@ -152,6 +153,7 @@ void SlotIndexes::removeSingleMachineInstrFromMaps(MachineInstr &MI) {
   } else {
     // FIXME: Eventually we want to actually delete these indexes.
     MIEntry.setInstr(nullptr);
+    MIEntry.setPoison();
   }
 }
 

>From 17295f6ea9d496de1d462d696cec6f4500b06c21 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Thu, 19 Oct 2023 16:46:40 +0100
Subject: [PATCH 2/2] [LiveDebugVariables] Add basic verification

Add a basic implementation of verifyAnalysis that just checks that the
analysis does not refer to any SlotIndexes for instructions that have
been deleted. This was useful for diagnosing some SlotIndexes-related
problems caused by #67038.
---
 llvm/lib/CodeGen/LiveDebugVariables.cpp | 17 +++++++++++++++++
 llvm/lib/CodeGen/LiveDebugVariables.h   |  1 +
 2 files changed, 18 insertions(+)

diff --git a/llvm/lib/CodeGen/LiveDebugVariables.cpp b/llvm/lib/CodeGen/LiveDebugVariables.cpp
index 7cb90af5ff173ed..93b9566b21370ad 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.cpp
+++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp
@@ -492,6 +492,13 @@ class UserValue {
   /// Return DebugLoc of this UserValue.
   const DebugLoc &getDebugLoc() { return dl; }
 
+  void verify() const {
+    for (auto I = locInts.begin(), E = locInts.end(); I != E; ++I) {
+      assert(!I.start().isPoisoned());
+      assert(!I.stop().isPoisoned());
+    }
+  }
+
   void print(raw_ostream &, const TargetRegisterInfo *);
 };
 
@@ -655,6 +662,11 @@ class LDVImpl {
     ModifiedMF = false;
   }
 
+  void verify() const {
+    for (auto [DV, UV] : userVarMap)
+      UV->verify();
+  }
+
   /// Map virtual register to an equivalence class.
   void mapVirtReg(Register VirtReg, UserValue *EC);
 
@@ -1320,6 +1332,11 @@ void LiveDebugVariables::releaseMemory() {
     static_cast<LDVImpl*>(pImpl)->clear();
 }
 
+void LiveDebugVariables::verifyAnalysis() const {
+  if (pImpl)
+    static_cast<LDVImpl *>(pImpl)->verify();
+}
+
 LiveDebugVariables::~LiveDebugVariables() {
   if (pImpl)
     delete static_cast<LDVImpl*>(pImpl);
diff --git a/llvm/lib/CodeGen/LiveDebugVariables.h b/llvm/lib/CodeGen/LiveDebugVariables.h
index 9998ce9e8dad861..c99f11c8565a819 100644
--- a/llvm/lib/CodeGen/LiveDebugVariables.h
+++ b/llvm/lib/CodeGen/LiveDebugVariables.h
@@ -56,6 +56,7 @@ class LLVM_LIBRARY_VISIBILITY LiveDebugVariables : public MachineFunctionPass {
   bool runOnMachineFunction(MachineFunction &) override;
   void releaseMemory() override;
   void getAnalysisUsage(AnalysisUsage &) const override;
+  void verifyAnalysis() const override;
 
   MachineFunctionProperties getSetProperties() const override {
     return MachineFunctionProperties().set(