[llvm-branch-commits] [llvm] 7fb1186 - Refactor recomputeLiveIns to operate on whole CFG (#79498) (#79641)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Feb 6 17:01:05 PST 2024
Author: Oskar Wirga
Date: 2024-02-06T17:01:01-08:00
New Revision: 7fb11860b05092d8aa071d13a1c40ef4ced0d224
URL: https://github.com/llvm/llvm-project/commit/7fb11860b05092d8aa071d13a1c40ef4ced0d224
DIFF: https://github.com/llvm/llvm-project/commit/7fb11860b05092d8aa071d13a1c40ef4ced0d224.diff
LOG: 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 ff4636a4ab00b633c15eb3942c26126ceb2662e6)
Added:
Modified:
llvm/include/llvm/CodeGen/LivePhysRegs.h
llvm/include/llvm/CodeGen/MachineBasicBlock.h
llvm/lib/CodeGen/BranchFolding.cpp
llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/LivePhysRegs.h b/llvm/include/llvm/CodeGen/LivePhysRegs.h
index 76bb34d270a26d..1d40b1cbb0eaa3 100644
--- a/llvm/include/llvm/CodeGen/LivePhysRegs.h
+++ b/llvm/include/llvm/CodeGen/LivePhysRegs.h
@@ -193,11 +193,18 @@ void addLiveIns(MachineBasicBlock &MBB, const LivePhysRegs &LiveRegs);
void computeAndAddLiveIns(LivePhysRegs &LiveRegs,
MachineBasicBlock &MBB);
-/// Convenience function for recomputing live-in's for \p MBB.
-static inline void recomputeLiveIns(MachineBasicBlock &MBB) {
+/// Convenience function for recomputing live-in's for a MBB. Returns true if
+/// any changes were made.
+static inline bool recomputeLiveIns(MachineBasicBlock &MBB) {
LivePhysRegs LPR;
+ auto oldLiveIns = MBB.getLiveIns();
+
MBB.clearLiveIns();
computeAndAddLiveIns(LPR, MBB);
+ MBB.sortUniqueLiveIns();
+
+ auto newLiveIns = MBB.getLiveIns();
+ return oldLiveIns != newLiveIns;
}
} // end namespace llvm
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index c84fd281c6a549..dc2035fa598c46 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -111,6 +111,10 @@ class MachineBasicBlock
RegisterMaskPair(MCPhysReg PhysReg, LaneBitmask LaneMask)
: PhysReg(PhysReg), LaneMask(LaneMask) {}
+
+ bool operator==(const RegisterMaskPair &other) const {
+ return PhysReg == other.PhysReg && LaneMask == other.LaneMask;
+ }
};
private:
@@ -473,6 +477,8 @@ class MachineBasicBlock
/// Remove entry from the livein set and return iterator to the next.
livein_iterator removeLiveIn(livein_iterator I);
+ std::vector<RegisterMaskPair> getLiveIns() const { return LiveIns; }
+
class liveout_iterator {
public:
using iterator_category = std::input_iterator_tag;
diff --git a/llvm/lib/CodeGen/BranchFolding.cpp b/llvm/lib/CodeGen/BranchFolding.cpp
index a9f78358e57b92..ecf7bc30913f51 100644
--- a/llvm/lib/CodeGen/BranchFolding.cpp
+++ b/llvm/lib/CodeGen/BranchFolding.cpp
@@ -2048,8 +2048,10 @@ bool BranchFolder::HoistCommonCodeInSuccs(MachineBasicBlock *MBB) {
FBB->erase(FBB->begin(), FIB);
if (UpdateLiveIns) {
- recomputeLiveIns(*TBB);
- recomputeLiveIns(*FBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*TBB) || recomputeLiveIns(*FBB);
+ } while (anyChange);
}
++NumHoist;
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index d55deec9760092..732e787d2a321d 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -4339,8 +4339,10 @@ AArch64FrameLowering::inlineStackProbeLoopExactMultiple(
ExitMBB->transferSuccessorsAndUpdatePHIs(&MBB);
MBB.addSuccessor(LoopMBB);
// Update liveins.
- recomputeLiveIns(*LoopMBB);
- recomputeLiveIns(*ExitMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*LoopMBB);
+ } while (anyChange);
return ExitMBB->begin();
}
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 13e9d9725cc2ed..9b4bb7c88bc821 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -9587,9 +9587,13 @@ AArch64InstrInfo::probedStackAlloc(MachineBasicBlock::iterator MBBI,
// Update liveins.
if (MF.getRegInfo().reservedRegsFrozen()) {
- recomputeLiveIns(*LoopTestMBB);
- recomputeLiveIns(*LoopBodyMBB);
- recomputeLiveIns(*ExitMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*ExitMBB) ||
+ recomputeLiveIns(*LoopBodyMBB) ||
+ recomputeLiveIns(*LoopTestMBB);
+ } while (anyChange);
+ ;
}
return ExitMBB->begin();
diff --git a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
index 5c1c7046fdbff0..8629551152cb64 100644
--- a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -1806,12 +1806,13 @@ void ARMLowOverheadLoops::Expand(LowOverheadLoop &LoLoop) {
PostOrderLoopTraversal DFS(LoLoop.ML, *MLI);
DFS.ProcessLoop();
const SmallVectorImpl<MachineBasicBlock*> &PostOrder = DFS.getOrder();
- for (auto *MBB : PostOrder) {
- recomputeLiveIns(*MBB);
- // FIXME: For some reason, the live-in print order is non-deterministic for
- // our tests and I can't out why... So just sort them.
- MBB->sortUniqueLiveIns();
- }
+ bool anyChange = false;
+ do {
+ anyChange = false;
+ for (auto *MBB : PostOrder) {
+ anyChange = recomputeLiveIns(*MBB) || anyChange;
+ }
+ } while (anyChange);
for (auto *MBB : reverse(PostOrder))
recomputeLivenessFlags(*MBB);
diff --git a/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp b/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
index aee57a5075ff71..b43eee8fdd8c0f 100644
--- a/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
+++ b/llvm/lib/Target/PowerPC/PPCExpandAtomicPseudoInsts.cpp
@@ -208,8 +208,10 @@ bool PPCExpandAtomicPseudo::expandAtomicRMW128(
.addMBB(LoopMBB);
CurrentMBB->addSuccessor(LoopMBB);
CurrentMBB->addSuccessor(ExitMBB);
- recomputeLiveIns(*LoopMBB);
- recomputeLiveIns(*ExitMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*LoopMBB);
+ } while (anyChange);
NMBBI = MBB.end();
MI.eraseFromParent();
return true;
@@ -286,9 +288,11 @@ bool PPCExpandAtomicPseudo::expandAtomicCmpSwap128(
CurrentMBB->addSuccessor(LoopCmpMBB);
CurrentMBB->addSuccessor(ExitMBB);
- recomputeLiveIns(*LoopCmpMBB);
- recomputeLiveIns(*CmpSuccMBB);
- recomputeLiveIns(*ExitMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*CmpSuccMBB) ||
+ recomputeLiveIns(*LoopCmpMBB);
+ } while (anyChange);
NMBBI = MBB.end();
MI.eraseFromParent();
return true;
diff --git a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
index 245e78641ed654..6792842f8550c1 100644
--- a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
@@ -1441,8 +1441,11 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
ProbeLoopBodyMBB->addSuccessor(ProbeLoopBodyMBB);
}
// Update liveins.
- recomputeLiveIns(*ProbeLoopBodyMBB);
- recomputeLiveIns(*ProbeExitMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*ProbeExitMBB) ||
+ recomputeLiveIns(*ProbeLoopBodyMBB);
+ } while (anyChange);
return ProbeExitMBB;
};
// For case HasBP && MaxAlign > 1, we have to realign the SP by performing
@@ -1534,8 +1537,10 @@ void PPCFrameLowering::inlineStackProbe(MachineFunction &MF,
buildDefCFAReg(*ExitMBB, ExitMBB->begin(), SPReg);
}
// Update liveins.
- recomputeLiveIns(*LoopMBB);
- recomputeLiveIns(*ExitMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*ExitMBB) || recomputeLiveIns(*LoopMBB);
+ } while (anyChange);
}
}
++NumPrologProbed;
diff --git a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
index db19c8881c685a..80c994a32ea96a 100644
--- a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -840,8 +840,10 @@ void SystemZELFFrameLowering::inlineStackProbe(
StackAllocMI->eraseFromParent();
if (DoneMBB != nullptr) {
// Compute the live-in lists for the new blocks.
- recomputeLiveIns(*DoneMBB);
- recomputeLiveIns(*LoopMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*DoneMBB) || recomputeLiveIns(*LoopMBB);
+ } while (anyChange);
}
}
@@ -1439,8 +1441,10 @@ void SystemZXPLINKFrameLowering::inlineStackProbe(
StackAllocMI->eraseFromParent();
// Compute the live-in lists for the new blocks.
- recomputeLiveIns(*NextMBB);
- recomputeLiveIns(*StackExtMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*StackExtMBB) || recomputeLiveIns(*NextMBB);
+ } while (anyChange);
}
bool SystemZXPLINKFrameLowering::hasFP(const MachineFunction &MF) const {
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index c0d358ead2787b..c2f76a3b8abbea 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -885,8 +885,10 @@ void X86FrameLowering::emitStackProbeInlineGenericLoop(
}
// Update Live In information
- recomputeLiveIns(*testMBB);
- recomputeLiveIns(*tailMBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*tailMBB) || recomputeLiveIns(*testMBB);
+ } while (anyChange);
}
void X86FrameLowering::emitStackProbeInlineWindowsCoreCLR64(
@@ -1378,10 +1380,11 @@ void X86FrameLowering::BuildStackAlignAND(MachineBasicBlock &MBB,
footMBB->addSuccessor(&MBB);
}
- recomputeLiveIns(*headMBB);
- recomputeLiveIns(*bodyMBB);
- recomputeLiveIns(*footMBB);
- recomputeLiveIns(MBB);
+ bool anyChange = false;
+ do {
+ anyChange = recomputeLiveIns(*footMBB) || recomputeLiveIns(*bodyMBB) ||
+ recomputeLiveIns(*headMBB) || recomputeLiveIns(MBB);
+ } while (anyChange);
}
} else {
MachineInstr *MI = BuildMI(MBB, MBBI, DL, TII.get(AndOp), Reg)
diff --git a/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir b/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
index 5e100b88ead300..82e3bae97ec0c4 100644
--- a/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
+++ b/llvm/test/CodeGen/SystemZ/branch-folder-hoist-livein.mir
@@ -1,3 +1,4 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
# RUN: llc -verify-machineinstrs -O1 -mtriple=s390x-ibm-linux -o - %s -run-pass=branch-folder | FileCheck %s
--- |
target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
@@ -15,6 +16,30 @@
name: f1
tracksRegLiveness: true
body: |
+ ; CHECK-LABEL: name: f1
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: renamable $r1d = LGRL @b :: (load (s32) from got, align 8)
+ ; CHECK-NEXT: renamable $r1l = LH killed renamable $r1d, 0, $noreg, implicit-def $r1d :: (dereferenceable load (s8) from @b)
+ ; CHECK-NEXT: renamable $r2l = LHI 0
+ ; CHECK-NEXT: renamable $r3d = LGRL @d :: (load (s32) from got, align 8)
+ ; CHECK-NEXT: renamable $r4d = LLILL 0, implicit-def $r4q
+ ; CHECK-NEXT: renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
+ ; CHECK-NEXT: CHI killed renamable $r2l, 0, implicit-def $cc
+ ; CHECK-NEXT: BRC 14, 6, %bb.2, implicit killed $cc
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors:
+ ; CHECK-NEXT: liveins: $r3d, $r4d, $r1l
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: liveins: $r3d, $r4d, $r1l
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: STH renamable $r1l, killed renamable $r3d, 0, $noreg, implicit killed $r4d :: (store (s8) into @d)
+ ; CHECK-NEXT: Return
bb.0:
successors: %bb.2(0x7fffffff), %bb.1(0x00000001)
liveins:
@@ -44,14 +69,3 @@ body: |
Return
...
-
-# CHECK: renamable $r4d = COPY killed renamable $r4d, implicit killed $r4q
-# CHECK-NEXT: CHI killed renamable $r2l, 0, implicit-def $cc
-# CHECK-NEXT: BRC 14, 6, %bb.2, implicit killed $cc
-# CHECK-NEXT: {{^ $}}
-# CHECK-NEXT: bb.1:
-# CHECK-NEXT: successors:
-# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
-
-# CHECK: bb.2:
-# CHECK-NEXT: liveins: $r1l, $r3d, $r4d
diff --git a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir
index 1bd1d6b99e4225..15aa62d5cff6b5 100644
--- a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir
+++ b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/spillingmove.mir
@@ -336,7 +336,7 @@ body: |
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.4:
; CHECK-NEXT: successors: %bb.5(0x04000000), %bb.2(0x7c000000)
- ; CHECK-NEXT: liveins: $q0, $r0, $r1, $r2, $r3, $r6, $r12
+ ; CHECK-NEXT: liveins: $d0, $d1, $r0, $r1, $r2, $r3, $r6, $r12
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $r3, dead $cpsr = nuw nsw tADDi8 killed renamable $r3, 1, 14 /* CC::al */, $noreg
; CHECK-NEXT: renamable $r0 = tADDhirr killed renamable $r0, renamable $r1, 14 /* CC::al */, $noreg
More information about the llvm-branch-commits
mailing list