Skip to content

Commit 1e7def2

Browse files
committed
[regalloc][LiveRegMatrix] Fix LiveInterval dangling pointers in LiveRegMatrix.
1 parent 125af56 commit 1e7def2

File tree

7 files changed

+91
-5
lines changed

7 files changed

+91
-5
lines changed

llvm/include/llvm/CodeGen/LiveIntervalUnion.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ class LiveIntervalUnion {
9393
// Remove a live virtual register's segments from this union.
9494
void extract(const LiveInterval &VirtReg, const LiveRange &Range);
9595

96+
// Remove all segments referencing VirtReg. This may be used if the register
97+
// isn't used anymore.
98+
void clearAllSegmentsReferencing(const LiveInterval &VirtReg);
99+
96100
// Remove all inserted virtual registers.
97101
void clear() { Segments.clear(); ++Tag; }
98102

llvm/include/llvm/CodeGen/LiveRegMatrix.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@ class LiveRegMatrix {
135135
/// the assignment and updates VirtRegMap accordingly.
136136
void unassign(const LiveInterval &VirtReg);
137137

138+
/// Unassign a virtual register by register number.
139+
/// Unlike unassign(LiveInterval&), this safely handles cases where
140+
/// the LiveInterval may be in an inconsistent state or about to be deleted.
141+
void unassign(Register VirtReg);
142+
138143
/// Returns true if the given \p PhysReg has any live intervals assigned.
139144
bool isPhysRegUsed(MCRegister PhysReg) const;
140145

@@ -170,6 +175,10 @@ class LiveRegMatrix {
170175
}
171176

172177
Register getOneVReg(unsigned PhysReg) const;
178+
179+
/// This checks that each LiveInterval referenced in LiveIntervalUnion
180+
/// actually exists in LiveIntervals and is not a dangling pointer.
181+
bool isValid() const;
173182
};
174183

175184
class LiveRegMatrixWrapperLegacy : public MachineFunctionPass {

llvm/lib/CodeGen/InlineSpiller.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ class HoistSpillHelper : private LiveRangeEdit::Delegate {
8686
const TargetInstrInfo &TII;
8787
const TargetRegisterInfo &TRI;
8888
const MachineBlockFrequencyInfo &MBFI;
89+
LiveRegMatrix &Matrix;
8990

9091
InsertPointAnalysis IPA;
9192

@@ -129,16 +130,17 @@ class HoistSpillHelper : private LiveRangeEdit::Delegate {
129130

130131
public:
131132
HoistSpillHelper(const Spiller::RequiredAnalyses &Analyses,
132-
MachineFunction &mf, VirtRegMap &vrm)
133+
MachineFunction &mf, VirtRegMap &vrm, LiveRegMatrix &matrix)
133134
: MF(mf), LIS(Analyses.LIS), LSS(Analyses.LSS), MDT(Analyses.MDT),
134135
VRM(vrm), MRI(mf.getRegInfo()), TII(*mf.getSubtarget().getInstrInfo()),
135136
TRI(*mf.getSubtarget().getRegisterInfo()), MBFI(Analyses.MBFI),
136-
IPA(LIS, mf.getNumBlockIDs()) {}
137+
Matrix(matrix), IPA(LIS, mf.getNumBlockIDs()) {}
137138

138139
void addToMergeableSpills(MachineInstr &Spill, int StackSlot,
139140
Register Original);
140141
bool rmFromMergeableSpills(MachineInstr &Spill, int StackSlot);
141142
void hoistAllSpills();
143+
bool LRE_CanEraseVirtReg(Register) override;
142144
void LRE_DidCloneVirtReg(Register, Register) override;
143145
};
144146

@@ -191,7 +193,7 @@ class InlineSpiller : public Spiller {
191193
: MF(MF), LIS(Analyses.LIS), LSS(Analyses.LSS), VRM(VRM),
192194
MRI(MF.getRegInfo()), TII(*MF.getSubtarget().getInstrInfo()),
193195
TRI(*MF.getSubtarget().getRegisterInfo()), Matrix(Matrix),
194-
HSpiller(Analyses, MF, VRM), VRAI(VRAI) {}
196+
HSpiller(Analyses, MF, VRM, *Matrix), VRAI(VRAI) {}
195197

196198
void spill(LiveRangeEdit &, AllocationOrder *Order = nullptr) override;
197199
ArrayRef<Register> getSpilledRegs() override { return RegsToSpill; }
@@ -1750,6 +1752,16 @@ void HoistSpillHelper::hoistAllSpills() {
17501752
}
17511753
}
17521754

1755+
/// Called before a virtual register is erased from LiveIntervals.
1756+
/// Forcibly remove the register from LiveRegMatrix before it's deleted,
1757+
/// preventing dangling pointers.
1758+
bool HoistSpillHelper::LRE_CanEraseVirtReg(Register VirtReg) {
1759+
if (VRM.hasPhys(VirtReg)) {
1760+
Matrix.unassign(VirtReg);
1761+
}
1762+
return true; // Allow deletion to proceed
1763+
}
1764+
17531765
/// For VirtReg clone, the \p New register should have the same physreg or
17541766
/// stackslot as the \p old register.
17551767
void HoistSpillHelper::LRE_DidCloneVirtReg(Register New, Register Old) {

llvm/lib/CodeGen/LiveIntervalUnion.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,19 @@ void LiveIntervalUnion::extract(const LiveInterval &VirtReg,
7979
}
8080
}
8181

82+
void LiveIntervalUnion::clearAllSegmentsReferencing(
83+
const LiveInterval &VirtReg) {
84+
++Tag;
85+
86+
// Remove all segments referencing VirtReg.
87+
for (SegmentIter SegPos = Segments.begin(); SegPos.valid();) {
88+
if (SegPos.value() == &VirtReg)
89+
SegPos.erase();
90+
else
91+
++SegPos;
92+
}
93+
}
94+
8295
void
8396
LiveIntervalUnion::print(raw_ostream &OS, const TargetRegisterInfo *TRI) const {
8497
if (empty()) {

llvm/lib/CodeGen/LiveRegMatrix.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212

1313
#include "llvm/CodeGen/LiveRegMatrix.h"
1414
#include "RegisterCoalescer.h"
15+
#include "llvm/ADT/DenseSet.h"
1516
#include "llvm/ADT/Statistic.h"
1617
#include "llvm/CodeGen/LiveInterval.h"
1718
#include "llvm/CodeGen/LiveIntervalUnion.h"
1819
#include "llvm/CodeGen/LiveIntervals.h"
1920
#include "llvm/CodeGen/MachineFunction.h"
21+
#include "llvm/CodeGen/MachineRegisterInfo.h"
2022
#include "llvm/CodeGen/TargetRegisterInfo.h"
2123
#include "llvm/CodeGen/TargetSubtargetInfo.h"
2224
#include "llvm/CodeGen/VirtRegMap.h"
@@ -142,6 +144,20 @@ void LiveRegMatrix::unassign(const LiveInterval &VirtReg) {
142144
LLVM_DEBUG(dbgs() << '\n');
143145
}
144146

147+
void LiveRegMatrix::unassign(Register VirtReg) {
148+
Register PhysReg = VRM->getPhys(VirtReg);
149+
LLVM_DEBUG(dbgs() << "unassigning " << printReg(VirtReg, TRI) << " from "
150+
<< printReg(PhysReg, TRI) << '\n');
151+
VRM->clearVirt(VirtReg);
152+
153+
assert(LIS->hasInterval(VirtReg));
154+
const LiveInterval &LI = LIS->getInterval(VirtReg);
155+
for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
156+
Matrix[Unit].clearAllSegmentsReferencing(LI);
157+
}
158+
++NumUnassigned;
159+
}
160+
145161
bool LiveRegMatrix::isPhysRegUsed(MCRegister PhysReg) const {
146162
for (MCRegUnit Unit : TRI->regunits(PhysReg)) {
147163
if (!Matrix[Unit].empty())
@@ -290,6 +306,33 @@ Register LiveRegMatrix::getOneVReg(unsigned PhysReg) const {
290306
return MCRegister::NoRegister;
291307
}
292308

309+
bool LiveRegMatrix::isValid() const {
310+
// Build set of all valid LiveInterval pointers from LiveIntervals.
311+
DenseSet<LiveInterval *> ValidIntervals;
312+
for (unsigned RegIdx = 0, NumRegs = VRM->getRegInfo().getNumVirtRegs();
313+
RegIdx < NumRegs; ++RegIdx) {
314+
Register VReg = Register::index2VirtReg(RegIdx);
315+
// Only track assigned registers since unassigned ones won't be in Matrix
316+
if (VRM->hasPhys(VReg) && LIS->hasInterval(VReg))
317+
ValidIntervals.insert(&LIS->getInterval(VReg));
318+
}
319+
320+
// Now scan all LiveIntervalUnions in the matrix and verify each pointer
321+
unsigned NumDanglingPointers = 0;
322+
for (unsigned I = 0, Size = Matrix.size(); I != Size; ++I) {
323+
MCRegUnit Unit = static_cast<MCRegUnit>(I);
324+
for (const LiveInterval *LI : Matrix[Unit]) {
325+
if (!ValidIntervals.contains(LI)) {
326+
++NumDanglingPointers;
327+
dbgs() << "ERROR: LiveInterval pointer is not found in LiveIntervals:\n"
328+
<< " Register Unit: " << printRegUnit(Unit, TRI) << '\n'
329+
<< " LiveInterval pointer: " << LI << '\n';
330+
}
331+
}
332+
}
333+
return NumDanglingPointers == 0;
334+
}
335+
293336
AnalysisKey LiveRegMatrixAnalysis::Key;
294337

295338
LiveRegMatrix LiveRegMatrixAnalysis::run(MachineFunction &MF,

llvm/lib/CodeGen/RegAllocBase.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ void RegAllocBase::allocatePhysRegs() {
155155

156156
void RegAllocBase::postOptimization() {
157157
spiller().postOptimization();
158+
159+
// Verify LiveRegMatrix after spilling (no dangling pointers).
160+
assert(Matrix->isValid() && "LiveRegMatrix validation failed");
161+
158162
for (auto *DeadInst : DeadRemats) {
159163
LIS->RemoveMachineInstrFromMaps(*DeadInst);
160164
DeadInst->eraseFromParent();

llvm/lib/Target/AMDGPU/SIPreAllocateWWMRegs.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,11 +153,12 @@ void SIPreAllocateWWMRegs::rewriteRegs(MachineFunction &MF) {
153153
SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
154154

155155
for (unsigned Reg : RegsToRewrite) {
156-
LIS->removeInterval(Reg);
157-
158156
const Register PhysReg = VRM->getPhys(Reg);
159157
assert(PhysReg != 0);
160158

159+
Matrix->unassign(Reg);
160+
LIS->removeInterval(Reg);
161+
161162
MFI->reserveWWMRegister(PhysReg);
162163
}
163164

0 commit comments

Comments
 (0)