Skip to content

Commit 7fb1186

Browse files
oskarwirgaoskarwirga
authored
Refactor recomputeLiveIns to operate on whole CFG (#79498) (#79641)
Currently, the way that recomputeLiveIns works is that it will recompute the livein registers for that MachineBasicBlock but it matters what order you call recomputeLiveIn which can result in incorrect register allocations down the line. Now we do not recompute the entire CFG but we do ensure that the newly added MBB do reach convergence. This fixes a register allocation bug introduced in AArch64 stack probing. (cherry picked from commit ff4636a)
1 parent 0328b87 commit 7fb1186

File tree

12 files changed

+98
-46
lines changed

12 files changed

+98
-46
lines changed

‎llvm/include/llvm/CodeGen/LivePhysRegs.h‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,18 @@ void addLiveIns(MachineBasicBlock &MBB, const LivePhysRegs &LiveRegs);
193193
void computeAndAddLiveIns(LivePhysRegs &LiveRegs,
194194
MachineBasicBlock &MBB);
195195

196-
/// Convenience function for recomputing live-in's for \p MBB.
197-
static inline void recomputeLiveIns(MachineBasicBlock &MBB) {
196+
/// Convenience function for recomputing live-in's for a MBB. Returns true if
197+
/// any changes were made.
198+
static inline bool recomputeLiveIns(MachineBasicBlock &MBB) {
198199
LivePhysRegs LPR;
200+
auto oldLiveIns = MBB.getLiveIns();
201+
199202
MBB.clearLiveIns();
200203
computeAndAddLiveIns(LPR, MBB);
204+
MBB.sortUniqueLiveIns();
205+
206+
auto newLiveIns = MBB.getLiveIns();
207+
return oldLiveIns != newLiveIns;
201208
}
202209

203210
} // end namespace llvm

‎llvm/include/llvm/CodeGen/MachineBasicBlock.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ class MachineBasicBlock
111111

112112
RegisterMaskPair(MCPhysReg PhysReg, LaneBitmask LaneMask)
113113
: PhysReg(PhysReg), LaneMask(LaneMask) {}
114+
115+
bool operator==(const RegisterMaskPair &other) const {
116+
return PhysReg == other.PhysReg && LaneMask == other.LaneMask;
117+
}
114118
};
115119

116120
private:
@@ -473,6 +477,8 @@ class MachineBasicBlock
473477
/// Remove entry from the livein set and return iterator to the next.
474478
livein_iterator removeLiveIn(livein_iterator I);
475479

480+
std::vector<RegisterMaskPair> getLiveIns() const { return LiveIns; }
481+
476482
class liveout_iterator {
477483
public:
478484
using iterator_category = std::input_iterator_tag;

‎llvm/lib/CodeGen/BranchFolding.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,8 +2048,10 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
20482048
FBB->erase(FBB->begin(), FIB);
20492049

20502050
if (UpdateLiveIns) {
2051-
recomputeLiveIns(*TBB);
2052-
recomputeLiveIns(*FBB);
2051+
bool anyChange = false;
2052+
do {
2053+
anyChange = recomputeLiveIns(*TBB) || recomputeLiveIns(*FBB);
2054+
} while (anyChange);
20532055
}
20542056

20552057
++NumHoist;

‎llvm/lib/Target/AArch64/AArch64FrameLowering.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4339,8 +4339,10 @@ AArch64FrameLowering::inlineStackProbeLoopExactMultiple(
43394339
ExitMBB->transferSuccessorsAndUpdatePHIs(&MBB);
43404340
MBB.addSuccessor(LoopMBB);
43414341
// Update liveins.
4342-
recomputeLiveIns(*LoopMBB);
4343-
recomputeLiveIns(*ExitMBB);
4342+
bool anyChange = false;
4343+
do {
4344+
anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*LoopMBB);
4345+
} while (anyChange);
43444346

43454347
return ExitMBB->begin();
43464348
}

‎llvm/lib/Target/AArch64/AArch64InstrInfo.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9587,9 +9587,13 @@ AArch64InstrInfo::probedStackAlloc(MachineBasicBlock::iterator MBBI,
95879587

95889588
// Update liveins.
95899589
if (MF.getRegInfo().reservedRegsFrozen()) {
9590-
recomputeLiveIns(*LoopTestMBB);
9591-
recomputeLiveIns(*LoopBodyMBB);
9592-
recomputeLiveIns(*ExitMBB);
9590+
bool anyChange = false;
9591+
do {
9592+
anyChange = recomputeLiveIns(*ExitMBB) ||
9593+
recomputeLiveIns(*LoopBodyMBB) ||
9594+
recomputeLiveIns(*LoopTestMBB);
9595+
} while (anyChange);
9596+
;
95939597
}
95949598

95959599
return ExitMBB->begin();

‎llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,12 +1806,13 @@ void ARMLowOverheadLoops::Expand(LowOverheadLoop &LoLoop) {
18061806
PostOrderLoopTraversal DFS(LoLoop.ML, *MLI);
18071807
DFS.ProcessLoop();
18081808
const SmallVectorImpl<MachineBasicBlock*> &PostOrder = DFS.getOrder();
1809-
for (auto *MBB : PostOrder) {
1810-
recomputeLiveIns(*MBB);
1811-
// FIXME: For some reason, the live-in print order is non-deterministic for
1812-
// our tests and I can't out why... So just sort them.
1813-
MBB->sortUniqueLiveIns();
1814-
}
1809+
bool anyChange = false;
1810+
do {
1811+
anyChange = false;
1812+
for (auto *MBB : PostOrder) {
1813+
anyChange = recomputeLiveIns(*MBB) || anyChange;
1814+
}
1815+
} while (anyChange);
18151816

18161817
for (auto *MBB : reverse(PostOrder))
18171818
recomputeLivenessFlags(*MBB);

‎llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,10 @@ bool PPCExpandAtomicPseudo::expandAtomicRMW128(
208208
.addMBB(LoopMBB);
209209
CurrentMBB->addSuccessor(LoopMBB);
210210
CurrentMBB->addSuccessor(ExitMBB);
211-
recomputeLiveIns(*LoopMBB);
212-
recomputeLiveIns(*ExitMBB);
211+
bool anyChange = false;
212+
do {
213+
anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*LoopMBB);
214+
} while (anyChange);
213215
NMBBI = MBB.end();
214216
MI.eraseFromParent();
215217
return true;
@@ -286,9 +288,11 @@ bool PPCExpandAtomicPseudo::expandAtomicCmpSwap128(
286288
CurrentMBB->addSuccessor(LoopCmpMBB);
287289
CurrentMBB->addSuccessor(ExitMBB);
288290

289-
recomputeLiveIns(*LoopCmpMBB);
290-
recomputeLiveIns(*CmpSuccMBB);
291-
recomputeLiveIns(*ExitMBB);
291+
bool anyChange = false;
292+
do {
293+
anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*CmpSuccMBB) ||
294+
recomputeLiveIns(*LoopCmpMBB);
295+
} while (anyChange);
292296
NMBBI = MBB.end();
293297
MI.eraseFromParent();
294298
return true;

‎llvm/lib/Target/PowerPC/PPCFrameLowering.cpp‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,8 +1441,11 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
14411441
ProbeLoopBodyMBB->addSuccessor(ProbeLoopBodyMBB);
14421442
}
14431443
// Update liveins.
1444-
recomputeLiveIns(*ProbeLoopBodyMBB);
1445-
recomputeLiveIns(*ProbeExitMBB);
1444+
bool anyChange = false;
1445+
do {
1446+
anyChange = recomputeLiveIns(*ProbeExitMBB) ||
1447+
recomputeLiveIns(*ProbeLoopBodyMBB);
1448+
} while (anyChange);
14461449
return ProbeExitMBB;
14471450
};
14481451
// For case HasBP && MaxAlign > 1, we have to realign the SP by performing
@@ -1534,8 +1537,10 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
15341537
buildDefCFAReg(*ExitMBB, ExitMBB->begin(), SPReg);
15351538
}
15361539
// Update liveins.
1537-
recomputeLiveIns(*LoopMBB);
1538-
recomputeLiveIns(*ExitMBB);
1540+
bool anyChange = false;
1541+
do {
1542+
anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*LoopMBB);
1543+
} while (anyChange);
15391544
}
15401545
}
15411546
++NumPrologProbed;

‎llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -840,8 +840,10 @@ void SystemZELFFrameLowering::inlineStackProbe(
840840
StackAllocMI->eraseFromParent();
841841
if (DoneMBB != nullptr) {
842842
// Compute the live-in lists for the new blocks.
843-
recomputeLiveIns(*DoneMBB);
844-
recomputeLiveIns(*LoopMBB);
843+
bool anyChange = false;
844+
do {
845+
anyChange = recomputeLiveIns(*DoneMBB) || recomputeLiveIns(*LoopMBB);
846+
} while (anyChange);
845847
}
846848
}
847849

@@ -1439,8 +1441,10 @@ void SystemZXPLINKFrameLowering::inlineStackProbe(
14391441
StackAllocMI->eraseFromParent();
14401442

14411443
// Compute the live-in lists for the new blocks.
1442-
recomputeLiveIns(*NextMBB);
1443-
recomputeLiveIns(*StackExtMBB);
1444+
bool anyChange = false;
1445+
do {
1446+
anyChange = recomputeLiveIns(*StackExtMBB) || recomputeLiveIns(*NextMBB);
1447+
} while (anyChange);
14441448
}
14451449

14461450
bool SystemZXPLINKFrameLowering::hasFP(const MachineFunction &MF) const {

‎llvm/lib/Target/X86/X86FrameLowering.cpp‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -885,8 +885,10 @@ void X86FrameLowering::emitStackProbeInlineGenericLoop(
885885
}
886886

887887
// Update Live In information
888-
recomputeLiveIns(*testMBB);
889-
recomputeLiveIns(*tailMBB);
888+
bool anyChange = false;
889+
do {
890+
anyChange = recomputeLiveIns(*tailMBB) || recomputeLiveIns(*testMBB);
891+
} while (anyChange);
890892
}
891893

892894
void X86FrameLowering::emitStackProbeInlineWindowsCoreCLR64(
@@ -1378,10 +1380,11 @@ void X86FrameLowering::BuildStackAlignAND(MachineBasicBlock &MBB,
13781380
footMBB->addSuccessor(&MBB);
13791381
}
13801382

1381-
recomputeLiveIns(*headMBB);
1382-
recomputeLiveIns(*bodyMBB);
1383-
recomputeLiveIns(*footMBB);
1384-
recomputeLiveIns(MBB);
1383+
bool anyChange = false;
1384+
do {
1385+
anyChange = recomputeLiveIns(*footMBB) || recomputeLiveIns(*bodyMBB) ||
1386+
recomputeLiveIns(*headMBB) || recomputeLiveIns(MBB);
1387+
} while (anyChange);
13851388
}
13861389
} else {
13871390
MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(AndOp), Reg)

0 commit comments

Comments
 (0)