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

Jay Foad via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 30 02:37:26 PST 2024


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

>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/4] [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 72f4a6876b6cb..2b437a0649aac 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 8b80c6ccb4389..762aab2ec2557 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/4] [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 7cb90af5ff173..93b9566b21370 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 9998ce9e8dad8..c99f11c8565a8 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(

>From d3b8da5b05b85a6dd71c558854bb2736b3dcc210 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Mon, 29 Jan 2024 15:31:25 +0000
Subject: [PATCH 3/4] clang-format

---
 llvm/include/llvm/CodeGen/SlotIndexes.h | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 2b437a0649aac..75fae0de04e3d 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -57,26 +57,20 @@ class raw_ostream;
     unsigned index;
 
   public:
-    IndexListEntry(MachineInstr *mi, unsigned index) : mi(mi, 0), index(index) {
-    }
+    IndexListEntry(MachineInstr *mi, unsigned index)
+        : mi(mi, 0), index(index) {}
 
-    MachineInstr* getInstr() const { return mi.getPointer(); }
-    void setInstr(MachineInstr *mi) {
-      this->mi.setPointer(mi);
-    }
+    MachineInstr *getInstr() const { return mi.getPointer(); }
+    void setInstr(MachineInstr *mi) { this->mi.setPointer(mi); }
 
     unsigned getIndex() const { return index; }
     void setIndex(unsigned index) {
       this->index = index;
     }
 
-    void setPoison() {
-      mi.setInt(PoisonVal);
-    }
+    void setPoison() { mi.setInt(PoisonVal); }
 
-    bool isPoisoned() const {
-      return mi.getInt();
-    }
+    bool isPoisoned() const { return mi.getInt(); }
   };
 
   template <>
@@ -305,9 +299,7 @@ class raw_ostream;
       return SlotIndex(&*--listEntry()->getIterator(), getSlot());
     }
 
-    bool isPoisoned() const {
-      return listEntry()->isPoisoned();
-    }
+    bool isPoisoned() const { return listEntry()->isPoisoned(); }
   };
 
   inline raw_ostream& operator<<(raw_ostream &os, SlotIndex li) {

>From ea80926e0007ec9afc44c4de18635da7b2263958 Mon Sep 17 00:00:00 2001
From: Jay Foad <jay.foad at amd.com>
Date: Tue, 30 Jan 2024 10:34:18 +0000
Subject: [PATCH 4/4] Tweak

---
 llvm/include/llvm/CodeGen/SlotIndexes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/CodeGen/SlotIndexes.h b/llvm/include/llvm/CodeGen/SlotIndexes.h
index 75fae0de04e3d..bdf4a466ddb80 100644
--- a/llvm/include/llvm/CodeGen/SlotIndexes.h
+++ b/llvm/include/llvm/CodeGen/SlotIndexes.h
@@ -53,7 +53,7 @@ class raw_ostream;
     static constexpr unsigned PoisonVal = 1;
 #endif
 
-    PointerIntPair<MachineInstr *, PoisonBits, unsigned> mi;
+    PointerIntPair<MachineInstr *, PoisonBits> mi;
     unsigned index;
 
   public:



More information about the llvm-commits mailing list