Skip to content

Commit ff4636a

Browse files
oskarwirgaoskarwirga
authored
Refactor recomputeLiveIns to converge on added MachineBasicBlocks (#79940)
This is a fix for the regression seen in #79498 > 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.
1 parent c43fda3 commit ff4636a

12 files changed

+98
-46
lines changed

Diff for: ‎llvm/include/llvm/CodeGen/LivePhysRegs.h

+9-2
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

Diff for: ‎llvm/include/llvm/CodeGen/MachineBasicBlock.h

+6
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;

Diff for: ‎llvm/lib/CodeGen/BranchFolding.cpp

+4-2
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;

Diff for: ‎llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

+4-2
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
}

Diff for: ‎llvm/lib/Target/AArch64/AArch64InstrInfo.cpp

+7-3
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();

Diff for: ‎llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp

+7-6
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);

Diff for: ‎llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp

+9-5
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;

Diff for: ‎llvm/lib/Target/PowerPC/PPCFrameLowering.cpp

+9-4
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;

Diff for: ‎llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

+8-4
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 {

Diff for: ‎llvm/lib/Target/X86/X86FrameLowering.cpp

+9-6
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)

Diff for: ‎llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir

+25-11
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
12
# RUN: llc -verify-machineinstrs -O1 -mtriple=s390x-ibm-linux -o - %s -run-pass=branch-folder | FileCheck %s
23
--- |
34
target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
@@ -15,6 +16,30 @@
1516
name: f1
1617
tracksRegLiveness: true
1718
body: |
19+
; CHECK-LABEL: name: f1
20+
; CHECK: bb.0:
21+
; CHECK-NEXT: successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
22+
; CHECK-NEXT: {{ $}}
23+
; CHECK-NEXT: renamable $r1d = LGRL @b :: (load (s32) from got, align 8)
24+
; CHECK-NEXT: renamable $r1l = LH killed renamable $r1d, 0, $noreg, implicit-def $r1d :: (dereferenceable load (s8) from @b)
25+
; CHECK-NEXT: renamable $r2l = LHI 0
26+
; CHECK-NEXT: renamable $r3d = LGRL @d :: (load (s32) from got, align 8)
27+
; CHECK-NEXT: renamable $r4d = LLILL 0, implicit-def $r4q
28+
; CHECK-NEXT: renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
29+
; CHECK-NEXT: CHI killed renamable $r2l, 0, implicit-def $cc
30+
; CHECK-NEXT: BRC 14, 6, %bb.2, implicit killed $cc
31+
; CHECK-NEXT: {{ $}}
32+
; CHECK-NEXT: bb.1:
33+
; CHECK-NEXT: successors:
34+
; CHECK-NEXT: liveins: $r3d, $r4d, $r1l
35+
; CHECK-NEXT: {{ $}}
36+
; CHECK-NEXT: STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
37+
; CHECK-NEXT: {{ $}}
38+
; CHECK-NEXT: bb.2:
39+
; CHECK-NEXT: liveins: $r3d, $r4d, $r1l
40+
; CHECK-NEXT: {{ $}}
41+
; CHECK-NEXT: STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
42+
; CHECK-NEXT: Return
1843
bb.0:
1944
successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
2045
liveins:
@@ -44,14 +69,3 @@ body: |
4469
Return
4570

4671
...
47-
48-
# CHECK: renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
49-
# CHECK-NEXT: CHI killed renamable $r2l, 0, implicit-def $cc
50-
# CHECK-NEXT: BRC 14, 6, %bb.2, implicit killed $cc
51-
# CHECK-NEXT: {{^ $}}
52-
# CHECK-NEXT: bb.1:
53-
# CHECK-NEXT: successors:
54-
# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
55-
56-
# CHECK: bb.2:
57-
# CHECK-NEXT: liveins: $r1l, $r3d, $r4d

Diff for: ‎llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ body: |
336336
; CHECK-NEXT: {{ $}}
337337
; CHECK-NEXT: bb.4:
338338
; CHECK-NEXT: successors: %bb.5(0x04000000), %bb.2(0x7c000000)
339-
; CHECK-NEXT: liveins: $q0, $r0, $r1, $r2, $r3, $r6, $r12
339+
; CHECK-NEXT: liveins: $d0, $d1, $r0, $r1, $r2, $r3, $r6, $r12
340340
; CHECK-NEXT: {{ $}}
341341
; CHECK-NEXT: renamable $r3, dead $cpsr = nuw nsw tADDi8 killed renamable $r3, 1, 14 /* CC::al */, $noreg
342342
; CHECK-NEXT: renamable $r0 = tADDhirr killed renamable $r0, renamable $r1, 14 /* CC::al */, $noreg

0 commit comments

Comments
 (0)