[llvm] [regalloc][LiveRegMatrix][AMDGPU] Fix LiveInterval dangling pointers in LiveRegMatrix. (PR #168556)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 18 07:59:21 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: Valery Pykhtin (vpykhtin)
<details>
<summary>Changes</summary>
Spilling code doesn't unassign register from matrix leaving dandling LiveInterval pointers there. This patch fixes this problem.
---
Full diff: https://github.com/llvm/llvm-project/pull/168556.diff
7 Files Affected:
- (modified) llvm/include/llvm/CodeGen/LiveIntervalUnion.h (+4)
- (modified) llvm/include/llvm/CodeGen/LiveRegMatrix.h (+15)
- (modified) llvm/lib/CodeGen/InlineSpiller.cpp (+15-3)
- (modified) llvm/lib/CodeGen/LiveIntervalUnion.cpp (+13)
- (modified) llvm/lib/CodeGen/LiveRegMatrix.cpp (+43)
- (modified) llvm/lib/CodeGen/RegAllocBase.cpp (+4)
- (modified) llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp (+3-2)
``````````diff
diff --git a/llvm/include/llvm/CodeGen/LiveIntervalUnion.h b/llvm/include/llvm/CodeGen/LiveIntervalUnion.h
index 240fa114cf179..182ab5963ffc4 100644
--- a/llvm/include/llvm/CodeGen/LiveIntervalUnion.h
+++ b/llvm/include/llvm/CodeGen/LiveIntervalUnion.h
@@ -93,6 +93,10 @@ class LiveIntervalUnion {
// Remove a live virtual register's segments from this union.
void extract(const LiveInterval &VirtReg, const LiveRange &Range);
+ // Remove all segments referencing VirtReg. This may be used if the register
+ // isn't used anymore.
+ void clear_all_segments_referencing(const LiveInterval &VirtReg);
+
// Remove all inserted virtual registers.
void clear() { Segments.clear(); ++Tag; }
diff --git a/llvm/include/llvm/CodeGen/LiveRegMatrix.h b/llvm/include/llvm/CodeGen/LiveRegMatrix.h
index 35add577d071a..24cf4b55e3ee8 100644
--- a/llvm/include/llvm/CodeGen/LiveRegMatrix.h
+++ b/llvm/include/llvm/CodeGen/LiveRegMatrix.h
@@ -135,6 +135,11 @@ class LiveRegMatrix {
/// the assignment and updates VirtRegMap accordingly.
void unassign(const LiveInterval &VirtReg);
+ /// Unassign a virtual register by register number.
+ /// Unlike unassign(LiveInterval&), this safely handles cases where
+ /// the LiveInterval may be in an inconsistent state or about to be deleted.
+ void unassign(Register VirtReg);
+
/// Returns true if the given \p PhysReg has any live intervals assigned.
bool isPhysRegUsed(MCRegister PhysReg) const;
@@ -170,6 +175,16 @@ class LiveRegMatrix {
}
Register getOneVReg(unsigned PhysReg) const;
+
+ /// Verify that all LiveInterval pointers in the matrix are valid.
+ /// This checks that each LiveInterval referenced in LiveIntervalUnion
+ /// actually exists in LiveIntervals and is not a dangling pointer.
+ /// Returns true if the matrix is valid, false if dangling pointers are found.
+ /// This is primarily useful for debugging heap-use-after-free issues.
+ /// This method uses a lazy approach - it builds a set of valid LiveInterval
+ /// pointers on-demand and has zero runtime/memory overhead during normal
+ /// register allocation.
+ bool isValid() const;
};
class LiveRegMatrixWrapperLegacy : public MachineFunctionPass {
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 68370303a3aef..b8f6377283b88 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -86,6 +86,7 @@ class HoistSpillHelper : private LiveRangeEdit::Delegate {
const TargetInstrInfo &TII;
const TargetRegisterInfo &TRI;
const MachineBlockFrequencyInfo &MBFI;
+ LiveRegMatrix &Matrix;
InsertPointAnalysis IPA;
@@ -129,16 +130,17 @@ class HoistSpillHelper : private LiveRangeEdit::Delegate {
public:
HoistSpillHelper(const Spiller::RequiredAnalyses &Analyses,
- MachineFunction &mf, VirtRegMap &vrm)
+ MachineFunction &mf, VirtRegMap &vrm, LiveRegMatrix &matrix)
: MF(mf), LIS(Analyses.LIS), LSS(Analyses.LSS), MDT(Analyses.MDT),
VRM(vrm), MRI(mf.getRegInfo()), TII(*mf.getSubtarget().getInstrInfo()),
TRI(*mf.getSubtarget().getRegisterInfo()), MBFI(Analyses.MBFI),
- IPA(LIS, mf.getNumBlockIDs()) {}
+ Matrix(matrix), IPA(LIS, mf.getNumBlockIDs()) {}
void addToMergeableSpills(MachineInstr &Spill, int StackSlot,
Register Original);
bool rmFromMergeableSpills(MachineInstr &Spill, int StackSlot);
void hoistAllSpills();
+ bool LRE_CanEraseVirtReg(Register) override;
void LRE_DidCloneVirtReg(Register, Register) override;
};
@@ -191,7 +193,7 @@ class InlineSpiller : public Spiller {
: MF(MF), LIS(Analyses.LIS), LSS(Analyses.LSS), VRM(VRM),
MRI(MF.getRegInfo()), TII(*MF.getSubtarget().getInstrInfo()),
TRI(*MF.getSubtarget().getRegisterInfo()), Matrix(Matrix),
- HSpiller(Analyses, MF, VRM), VRAI(VRAI) {}
+ HSpiller(Analyses, MF, VRM, *Matrix), VRAI(VRAI) {}
void spill(LiveRangeEdit &, AllocationOrder *Order = nullptr) override;
ArrayRef<Register> getSpilledRegs() override { return RegsToSpill; }
@@ -1750,6 +1752,16 @@ void HoistSpillHelper::hoistAllSpills() {
}
}
+/// Called before a virtual register is erased from LiveIntervals.
+/// Forcibly remove the register from LiveRegMatrix before it's deleted,
+/// preventing dangling pointers.
+bool HoistSpillHelper::LRE_CanEraseVirtReg(Register VirtReg) {
+ if (VRM.hasPhys(VirtReg)) {
+ Matrix.unassign(VirtReg);
+ }
+ return true; // Allow deletion to proceed
+}
+
/// For VirtReg clone, the \p New register should have the same physreg or
/// stackslot as the \p old register.
void HoistSpillHelper::LRE_DidCloneVirtReg(Register New, Register Old) {
diff --git a/llvm/lib/CodeGen/LiveIntervalUnion.cpp b/llvm/lib/CodeGen/LiveIntervalUnion.cpp
index eb547c5238432..f5643b9d2ca83 100644
--- a/llvm/lib/CodeGen/LiveIntervalUnion.cpp
+++ b/llvm/lib/CodeGen/LiveIntervalUnion.cpp
@@ -79,6 +79,19 @@ void LiveIntervalUnion::extract(const LiveInterval &VirtReg,
}
}
+void LiveIntervalUnion::clear_all_segments_referencing(
+ const LiveInterval &VirtReg) {
+ ++Tag;
+
+ // Remove all segments referencing VirtReg.
+ for (SegmentIter SegPos = Segments.begin(); SegPos.valid();) {
+ if (SegPos.value() == &VirtReg)
+ SegPos.erase();
+ else
+ ++SegPos;
+ }
+}
+
void
LiveIntervalUnion::print(raw_ostream &OS, const TargetRegisterInfo *TRI) const {
if (empty()) {
diff --git a/llvm/lib/CodeGen/LiveRegMatrix.cpp b/llvm/lib/CodeGen/LiveRegMatrix.cpp
index e7238008d2c69..75d9663ef2263 100644
--- a/llvm/lib/CodeGen/LiveRegMatrix.cpp
+++ b/llvm/lib/CodeGen/LiveRegMatrix.cpp
@@ -12,11 +12,13 @@
#include "llvm/CodeGen/LiveRegMatrix.h"
#include "RegisterCoalescer.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/CodeGen/LiveInterval.h"
#include "llvm/CodeGen/LiveIntervalUnion.h"
#include "llvm/CodeGen/LiveIntervals.h"
#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/TargetRegisterInfo.h"
#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/CodeGen/VirtRegMap.h"
@@ -142,6 +144,20 @@ void LiveRegMatrix::unassign(const LiveInterval &VirtReg) {
LLVM_DEBUG(dbgs() << '\n');
}
+void LiveRegMatrix::unassign(Register VirtReg) {
+ Register PhysReg = VRM->getPhys(VirtReg);
+ LLVM_DEBUG(dbgs() << "unassigning " << printReg(VirtReg, TRI) << " from "
+ << printReg(PhysReg, TRI) << '\n');
+ VRM->clearVirt(VirtReg);
+
+ assert(LIS->hasInterval(VirtReg));
+ const LiveInterval &LI = LIS->getInterval(VirtReg);
+ for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
+ Matrix[Unit].clear_all_segments_referencing(LI);
+ }
+ ++NumUnassigned;
+}
+
bool LiveRegMatrix::isPhysRegUsed(MCRegister PhysReg) const {
for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
if (!Matrix[Unit].empty())
@@ -290,6 +306,33 @@ Register LiveRegMatrix::getOneVReg(unsigned PhysReg) const {
return MCRegister::NoRegister;
}
+bool LiveRegMatrix::isValid() const {
+ // Build set of all valid LiveInterval pointers from LiveIntervals.
+ DenseSet<LiveInterval *> ValidIntervals;
+ for (unsigned RegIdx = 0, NumRegs = VRM->getRegInfo().getNumVirtRegs();
+ RegIdx < NumRegs; ++RegIdx) {
+ Register VReg = Register::index2VirtReg(RegIdx);
+ // Only track assigned registers since unassigned ones won't be in Matrix
+ if (VRM->hasPhys(VReg) && LIS->hasInterval(VReg))
+ ValidIntervals.insert(&LIS->getInterval(VReg));
+ }
+
+ // Now scan all LiveIntervalUnions in the matrix and verify each pointer
+ unsigned NumDanglingPointers = 0;
+ for (unsigned I = 0, Size = Matrix.size(); I != Size; ++I) {
+ MCRegUnit Unit = static_cast<MCRegUnit>(I);
+ for (const LiveInterval *LI : Matrix[Unit]) {
+ if (!ValidIntervals.contains(LI)) {
+ ++NumDanglingPointers;
+ dbgs() << "ERROR: LiveInterval pointer is not found in LiveIntervals:\n"
+ << " Register Unit: " << printRegUnit(Unit, TRI) << '\n'
+ << " LiveInterval pointer: " << LI << '\n';
+ }
+ }
+ }
+ return NumDanglingPointers == 0;
+}
+
AnalysisKey LiveRegMatrixAnalysis::Key;
LiveRegMatrix LiveRegMatrixAnalysis::run(MachineFunction &MF,
diff --git a/llvm/lib/CodeGen/RegAllocBase.cpp b/llvm/lib/CodeGen/RegAllocBase.cpp
index 2400a1feea26e..9ba6007fc1cb9 100644
--- a/llvm/lib/CodeGen/RegAllocBase.cpp
+++ b/llvm/lib/CodeGen/RegAllocBase.cpp
@@ -155,6 +155,10 @@ void RegAllocBase::allocatePhysRegs() {
void RegAllocBase::postOptimization() {
spiller().postOptimization();
+
+ // Verify LiveRegMatrix after spilling (no dangling pointers).
+ assert(Matrix->isValid() && "LiveRegMatrix validation failed");
+
for (auto *DeadInst : DeadRemats) {
LIS->RemoveMachineInstrFromMaps(*DeadInst);
DeadInst->eraseFromParent();
diff --git a/llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp b/llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp
index ecfaa5c70e9d3..eb4232e9a2906 100644
--- a/llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp
+++ b/llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp
@@ -153,11 +153,12 @@ void SIPreAllocateWWMRegs::rewriteRegs(MachineFunction &MF) {
SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
for (unsigned Reg : RegsToRewrite) {
- LIS->removeInterval(Reg);
-
const Register PhysReg = VRM->getPhys(Reg);
assert(PhysReg != 0);
+ Matrix->unassign(Reg);
+ LIS->removeInterval(Reg);
+
MFI->reserveWWMRegister(PhysReg);
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/168556
More information about the llvm-commits
mailing list