[llvm] [MachineCSE] Walk the terminator the hoisted copy is placed in front of (PR #215158)
Matt Turner via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 1 21:25:55 PDT 2026
https://github.com/mattst88 updated https://github.com/llvm/llvm-project/pull/215158
>From 51e8051ade853fcca4deb17124fe18a579c52be9 Mon Sep 17 00:00:00 2001
From: Matt Turner <mattst88 at gmail.com>
Date: Sun, 9 Aug 2026 15:59:20 -0400
Subject: [PATCH] [MachineCSE] Walk the terminator the hoisted copy is placed
in front of
Partial redundancy elimination hoists a computation into the nearest
common dominator of the two blocks holding it, in front of that block's
first terminator. When the computation reads a physical register, that
register has to still hold the same value where the copy lands. The walk
that checks this started one instruction past the terminator, so the
terminator itself was never examined, even though it sits between the
copy and the original.
EH_SjLj_Setup is such a terminator, on every target that lowers setjmp
this way (X86, PowerPC, SystemZ). It carries a register mask preserving
nothing, which is what keeps a value from staying in a register across
the setjmp, and it names the block a returning longjmp resumes at, so
its block has a fallthrough successor as well. A computation reading a
physical register was hoisted in front of it, past the clobber that mask
describes.
Pass the position to PhysRegDefsReach as an iterator plus its block, and
start the walk there. This also stops the caller from forming the
position by dereferencing getFirstTerminator(), which is end() for a
block that falls through to its successor. The walk already tolerates a
position at the end of a block it is crossing out of.
isPRECandidate collected physical register uses without consulting
TargetInstrInfo::isIgnorableUse, which hasLivePhysRegDefUses already
consults on the plain CSE path. Ask it here too. Without that, the
implicit EXEC read every AMDGPU VALU instruction carries would block the
hoist at SI_IF and friends, which define EXEC, even though the read is
not a real one: the extra lanes a wider mask computes are discarded. A
VALU whose result does depend on EXEC -- a convergent one, or one
defining an SGPR -- is not ignorable and remains blocked.
The single AArch64 test change is a ptrue that is no longer hoisted,
because the terminator now costs one of the walk's five lookahead steps.
NonLocal is also initialized, having been passed by reference to a
function that writes it only on the cross-block path.
Assisted-by: Claude
---
llvm/include/llvm/CodeGen/TargetInstrInfo.h | 3 +-
llvm/lib/CodeGen/MachineCSE.cpp | 35 +++++++----
...mode-fixed-length-masked-gather-scatter.ll | 3 +-
.../X86/machine-cse-pre-sjlj-setup.mir | 62 +++++++++++++++++++
4 files changed, 90 insertions(+), 13 deletions(-)
create mode 100644 llvm/test/CodeGen/X86/machine-cse-pre-sjlj-setup.mir
diff --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index cb0394ce2e0c6..99005d91fe04f 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -193,7 +193,8 @@ class LLVM_ABI TargetInstrInfo : public MCInstrInfo {
}
/// Given operand \p OpIdx of \p MI is a PhysReg use, return if it can be
- /// ignored for the purpose of instruction rematerialization or sinking.
+ /// ignored for the purpose of moving \p MI: rematerializing it, sinking it,
+ /// or hoisting it.
virtual bool isIgnorableUse(const MachineInstr &MI, unsigned OpIdx) const {
return false;
}
diff --git a/llvm/lib/CodeGen/MachineCSE.cpp b/llvm/lib/CodeGen/MachineCSE.cpp
index 23ddccbdacd7a..46634aae72bff 100644
--- a/llvm/lib/CodeGen/MachineCSE.cpp
+++ b/llvm/lib/CodeGen/MachineCSE.cpp
@@ -109,7 +109,8 @@ class MachineCSEImpl {
const MachineBasicBlock *MBB,
SmallSet<MCRegister, 8> &PhysRefs,
PhysDefVector &PhysDefs, bool &PhysUseDef) const;
- bool PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI,
+ bool PhysRegDefsReach(const MachineBasicBlock *CSMBB,
+ MachineBasicBlock::const_iterator CSI, MachineInstr *MI,
const SmallSet<MCRegister, 8> &PhysRefs,
const PhysDefVector &PhysDefs, bool &NonLocal) const;
bool isCSECandidate(MachineInstr *MI);
@@ -329,7 +330,12 @@ bool MachineCSEImpl::hasLivePhysRegDefUses(const MachineInstr *MI,
return !PhysRefs.empty();
}
-bool MachineCSEImpl::PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI,
+/// Check whether the registers in \p PhysRefs still hold, at \p MI, the values
+/// they held at \p CSI in \p CSMBB. The walk covers [CSI, MI), so \p CSI is
+/// scanned like any other position and may be CSMBB->end().
+bool MachineCSEImpl::PhysRegDefsReach(const MachineBasicBlock *CSMBB,
+ MachineBasicBlock::const_iterator CSI,
+ MachineInstr *MI,
const SmallSet<MCRegister, 8> &PhysRefs,
const PhysDefVector &PhysDefs,
bool &NonLocal) const {
@@ -337,7 +343,6 @@ bool MachineCSEImpl::PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI,
// not in the same basic block as the given instruction. The only exception
// is if the common subexpression is in the sole predecessor block.
const MachineBasicBlock *MBB = MI->getParent();
- const MachineBasicBlock *CSMBB = CSMI->getParent();
bool CrossMBB = false;
if (CSMBB != MBB) {
@@ -352,7 +357,7 @@ bool MachineCSEImpl::PhysRegDefsReach(MachineInstr *CSMI, MachineInstr *MI,
}
CrossMBB = true;
}
- MachineBasicBlock::const_iterator I = CSMI; I = std::next(I);
+ MachineBasicBlock::const_iterator I = CSI;
MachineBasicBlock::const_iterator E = MI;
MachineBasicBlock::const_iterator EE = CSMBB->end();
unsigned LookAheadLeft = LookAheadLimit;
@@ -583,7 +588,9 @@ bool MachineCSEImpl::ProcessBlockCSE(MachineBasicBlock *MBB) {
if (!PhysUseDef) {
unsigned CSVN = VNT.lookup(&MI);
MachineInstr *CSMI = Exps[CSVN];
- if (PhysRegDefsReach(CSMI, &MI, PhysRefs, PhysDefs, CrossMBBPhysDef))
+ MachineBasicBlock::const_iterator CSI(CSMI);
+ if (PhysRegDefsReach(CSMI->getParent(), std::next(CSI), &MI, PhysRefs,
+ PhysDefs, CrossMBBPhysDef))
FoundCSE = true;
}
}
@@ -810,7 +817,10 @@ bool MachineCSEImpl::isPRECandidate(MachineInstr *MI,
if (MO.isReg() && !MO.getReg().isVirtual()) {
if (MO.isDef())
return false;
- else
+ // A use the target considers ignorable is not a real read, so it does
+ // not constrain where the instruction may be hoisted to. AMDGPU uses
+ // this for the implicit EXEC read on VALU instructions.
+ if (!TII->isIgnorableUse(*MI, MI->getOperandNo(&MO)))
PhysRefs.insert(MO.getReg());
}
}
@@ -860,11 +870,15 @@ bool MachineCSEImpl::ProcessBlockPRE(MachineDominatorTree *DT,
// If this instruction uses physical registers then we can only do PRE
// if it's using the value that is live at the place we're hoisting to.
- bool NonLocal;
+ bool NonLocal = false;
PhysDefVector PhysDefs;
+ // The copy lands in front of CMBB's first terminator, so the walk has
+ // to start there: the terminator itself lies between the copy and MI.
+ // getFirstTerminator() is end() when CMBB falls through.
+ auto FirstTerm = CMBB->getFirstTerminator();
if (!PhysRefs.empty() &&
- !PhysRegDefsReach(&*(CMBB->getFirstTerminator()), &MI, PhysRefs,
- PhysDefs, NonLocal))
+ !PhysRegDefsReach(CMBB, FirstTerm, &MI, PhysRefs, PhysDefs,
+ NonLocal))
continue;
assert(MI.getOperand(0).isDef() &&
@@ -873,8 +887,7 @@ bool MachineCSEImpl::ProcessBlockPRE(MachineDominatorTree *DT,
Register NewReg = MRI->cloneVirtualRegister(VReg);
if (!isProfitableToCSE(NewReg, VReg, CMBB, &MI))
continue;
- MachineInstr &NewMI =
- TII->duplicate(*CMBB, CMBB->getFirstTerminator(), MI);
+ MachineInstr &NewMI = TII->duplicate(*CMBB, FirstTerm, MI);
// When hoisting, make sure we don't carry the debug location of
// the original instruction, as that's not correct and can cause
diff --git a/llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-masked-gather-scatter.ll b/llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-masked-gather-scatter.ll
index 593ec1113cc19..649ff578683a8 100644
--- a/llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-masked-gather-scatter.ll
+++ b/llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-masked-gather-scatter.ll
@@ -19,12 +19,12 @@ define <2 x i64> @masked_gather_v2i64(ptr %a, ptr %b) vscale_range(2, 2) {
; CHECK-NEXT: and z0.d, z1.d, z0.d
; CHECK-NEXT: ldr q1, [x1]
; CHECK-NEXT: uaddv d0, p0, z0.d
-; CHECK-NEXT: ptrue p0.d
; CHECK-NEXT: str b0, [sp, #12]
; CHECK-NEXT: ldrb w8, [sp, #12]
; CHECK-NEXT: tbz w8, #0, .LBB0_2
; CHECK-NEXT: // %bb.1: // %cond.load
; CHECK-NEXT: fmov x9, d1
+; CHECK-NEXT: ptrue p0.d
; CHECK-NEXT: ld1rd { z0.d }, p0/z, [x9]
; CHECK-NEXT: tbnz w8, #1, .LBB0_3
; CHECK-NEXT: b .LBB0_4
@@ -37,6 +37,7 @@ define <2 x i64> @masked_gather_v2i64(ptr %a, ptr %b) vscale_range(2, 2) {
; CHECK-NEXT: index z2.d, #0, #1
; CHECK-NEXT: mov z1.d, z1.d[1]
; CHECK-NEXT: mov z3.d, x8
+; CHECK-NEXT: ptrue p0.d
; CHECK-NEXT: fmov x8, d1
; CHECK-NEXT: cmpeq p1.d, p0/z, z2.d, z3.d
; CHECK-NEXT: ldr x8, [x8]
diff --git a/llvm/test/CodeGen/X86/machine-cse-pre-sjlj-setup.mir b/llvm/test/CodeGen/X86/machine-cse-pre-sjlj-setup.mir
new file mode 100644
index 0000000000000..4b533ba654330
--- /dev/null
+++ b/llvm/test/CodeGen/X86/machine-cse-pre-sjlj-setup.mir
@@ -0,0 +1,62 @@
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass=machine-cse -verify-machineinstrs -o - %s | FileCheck %s
+
+# Partial redundancy elimination hoists a computation into the nearest common
+# dominator of the two blocks holding it, in front of that block's first
+# terminator. A computation that reads a physical register may only be hoisted
+# if the register still holds the same value where the copy lands, and the walk
+# that checks that has to start at the terminator rather than after it: the copy
+# goes in front of the terminator, so whatever the terminator clobbers lies
+# between the copy and the original.
+#
+# EH_SjLj_Setup is such a terminator. It carries a register mask preserving
+# nothing, which is what keeps a value from staying in a register across a
+# setjmp, and it names the block a returning longjmp resumes at, so its block
+# has a fallthrough successor as well. The rip-relative lea below must not be
+# hoisted in front of it.
+
+--- |
+ define i64 @foo() {
+ entry:
+ br i1 poison, label %setjmp, label %land
+ setjmp:
+ br label %join
+ land:
+ br label %join
+ join:
+ ret i64 0
+ }
+...
+---
+name: foo
+tracksRegLiveness: true
+body: |
+ bb.0.entry:
+ successors: %bb.1(0x40000000), %bb.3(0x40000000)
+
+ EH_SjLj_Setup %bb.3, csr_noregs
+
+ bb.1.setjmp:
+ successors: %bb.2(0x80000000)
+
+ %0:gr64 = LEA64r $rip, 1, $noreg, 0, $noreg
+
+ bb.2.join:
+ %2:gr64 = PHI %0, %bb.1, %1, %bb.3
+ %3:gr64 = LEA64r $rip, 1, $noreg, 0, $noreg
+ %4:gr64 = ADD64rr %2, %3, implicit-def dead $eflags
+ $rax = COPY %4
+ RET64 implicit $rax
+
+ bb.3.land (machine-block-address-taken):
+ successors: %bb.2(0x80000000)
+
+ %1:gr64 = LEA64r $rip, 1, $noreg, 8, $noreg
+ JMP_1 %bb.2
+...
+
+# CHECK-LABEL: name: foo
+# CHECK: bb.0.entry:
+# CHECK-NOT: LEA64r
+# CHECK: EH_SjLj_Setup %bb.3, csr_noregs
+# CHECK: bb.1.setjmp:
+# CHECK: LEA64r $rip, 1, $noreg, 0, $noreg
More information about the llvm-commits
mailing list