From 6bc56c509d3ab2a1ee09721d660dd81beaeda32d Mon Sep 17 00:00:00 2001 From: Jay Foad 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 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 { - 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 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 65726f06dedb4..e8173c232516c 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 cc0a5f4f6016b31e14e06363f42e168d2060a286 Mon Sep 17 00:00:00 2001 From: Jay Foad 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 9603c1f01e085..53d99058f9d69 100644 --- a/llvm/lib/CodeGen/LiveDebugVariables.cpp +++ b/llvm/lib/CodeGen/LiveDebugVariables.cpp @@ -491,6 +491,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 *); }; @@ -654,6 +661,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); @@ -1319,6 +1331,11 @@ void LiveDebugVariables::releaseMemory() { static_cast(pImpl)->clear(); } +void LiveDebugVariables::verifyAnalysis() const { + if (pImpl) + static_cast(pImpl)->verify(); +} + LiveDebugVariables::~LiveDebugVariables() { if (pImpl) delete static_cast(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(