[llvm] [AMDGPU] Fix SIFoldOperands miscompiling values that leave a divergent loop (PR #203256)
Arseniy Obolenskiy via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 11 05:52:47 PDT 2026
https://github.com/aobolensk updated https://github.com/llvm/llvm-project/pull/203256
>From 0e12fc521b8892c662f67a8e6561a9db5d5d0ee9 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Thu, 11 Jun 2026 14:46:48 +0200
Subject: [PATCH] [AMDGPU] Fix SIFoldOperands miscompiling values that leave a
divergent loop
A scalar value latched per-lane inside a divergent loop was being folded into a use after the loop, so every lane wrongly read the same value
---
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | 51 +++++--
llvm/test/CodeGen/AMDGPU/do-not-fold-copy.mir | 129 ++++++++++++++++++
2 files changed, 172 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index cd057355b1f1d..dd34095789b35 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -18,7 +18,9 @@
#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/CodeGen/MachineFunction.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/CodeGen/MachineLoopInfo.h"
#include "llvm/CodeGen/MachineOperand.h"
+#include "llvm/InitializePasses.h"
#define DEBUG_TYPE "si-fold-operands"
using namespace llvm;
@@ -179,6 +181,7 @@ class SIFoldOperandsImpl {
const SIRegisterInfo *TRI;
const GCNSubtarget *ST;
const SIMachineFunctionInfo *MFI;
+ const MachineLoopInfo *MLI;
bool frameIndexMayFold(const MachineInstr &UseMI, int OpNo,
const FoldableDef &OpToFold) const;
@@ -219,6 +222,8 @@ class SIFoldOperandsImpl {
const FoldableDef &OpToFold) const;
bool isUseSafeToFold(const MachineInstr &MI,
const MachineOperand &UseMO) const;
+ bool isRegFoldSafeAcrossLoopExit(const FoldableDef &OpToFold,
+ const MachineInstr &UseMI) const;
const TargetRegisterClass *getRegSeqInit(
MachineInstr &RegSeq,
@@ -265,7 +270,7 @@ class SIFoldOperandsImpl {
public:
SIFoldOperandsImpl() = default;
- bool run(MachineFunction &MF);
+ bool run(MachineFunction &MF, const MachineLoopInfo *MLI);
};
class SIFoldOperandsLegacy : public MachineFunctionPass {
@@ -277,13 +282,17 @@ class SIFoldOperandsLegacy : public MachineFunctionPass {
bool runOnMachineFunction(MachineFunction &MF) override {
if (skipFunction(MF.getFunction()))
return false;
- return SIFoldOperandsImpl().run(MF);
+ const MachineLoopInfo *MLI =
+ &getAnalysis<MachineLoopInfoWrapperPass>().getLI();
+ return SIFoldOperandsImpl().run(MF, MLI);
}
StringRef getPassName() const override { return "SI Fold Operands"; }
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.setPreservesCFG();
+ AU.addRequired<MachineLoopInfoWrapperPass>();
+ AU.addPreserved<MachineLoopInfoWrapperPass>();
MachineFunctionPass::getAnalysisUsage(AU);
}
@@ -294,8 +303,11 @@ class SIFoldOperandsLegacy : public MachineFunctionPass {
} // End anonymous namespace.
-INITIALIZE_PASS(SIFoldOperandsLegacy, DEBUG_TYPE, "SI Fold Operands", false,
- false)
+INITIALIZE_PASS_BEGIN(SIFoldOperandsLegacy, DEBUG_TYPE, "SI Fold Operands",
+ false, false)
+INITIALIZE_PASS_DEPENDENCY(MachineLoopInfoWrapperPass)
+INITIALIZE_PASS_END(SIFoldOperandsLegacy, DEBUG_TYPE, "SI Fold Operands", false,
+ false)
char SIFoldOperandsLegacy::ID = 0;
@@ -970,6 +982,22 @@ bool SIFoldOperandsImpl::isUseSafeToFold(const MachineInstr &MI,
return !TII->isSDWA(MI);
}
+// An SGPR->VGPR copy inside a divergent loop latches each lane value as it
+// exits. Folding its scalar source into a use after the loop would make every
+// lane read the same reconverged value, so do not fold across the loop exit.
+bool SIFoldOperandsImpl::isRegFoldSafeAcrossLoopExit(
+ const FoldableDef &OpToFold, const MachineInstr &UseMI) const {
+ if (!OpToFold.isReg())
+ return true;
+ const MachineInstr *DefMI = OpToFold.DefMI;
+ if (!DefMI || !DefMI->isCopy() ||
+ !TRI->isVGPR(*MRI, DefMI->getOperand(0).getReg()) ||
+ !TRI->isSGPRReg(*MRI, OpToFold.getReg()))
+ return true;
+ const MachineLoop *DefLoop = MLI->getLoopFor(DefMI->getParent());
+ return !DefLoop || DefLoop->contains(UseMI.getParent());
+}
+
static MachineOperand *lookUpCopyChain(const SIInstrInfo &TII,
const MachineRegisterInfo &MRI,
Register SrcReg) {
@@ -1194,6 +1222,9 @@ void SIFoldOperandsImpl::foldOperand(
if (!isUseSafeToFold(*UseMI, *UseOp))
return;
+ if (!isRegFoldSafeAcrossLoopExit(OpToFold, *UseMI))
+ return;
+
// FIXME: Fold operands with subregs.
if (UseOp->isReg() && OpToFold.isReg()) {
if (UseOp->isImplicit())
@@ -2804,13 +2835,14 @@ bool SIFoldOperandsImpl::tryOptimizeAGPRPhis(MachineBasicBlock &MBB) {
return Changed;
}
-bool SIFoldOperandsImpl::run(MachineFunction &MF) {
+bool SIFoldOperandsImpl::run(MachineFunction &MF, const MachineLoopInfo *MLI) {
this->MF = &MF;
MRI = &MF.getRegInfo();
ST = &MF.getSubtarget<GCNSubtarget>();
TII = ST->getInstrInfo();
TRI = &TII->getRegisterInfo();
MFI = MF.getInfo<SIMachineFunctionInfo>();
+ this->MLI = MLI;
// omod is ignored by hardware if IEEE bit is enabled. omod also does not
// correctly handle signed zeros.
@@ -2865,15 +2897,18 @@ bool SIFoldOperandsImpl::run(MachineFunction &MF) {
return Changed;
}
-PreservedAnalyses SIFoldOperandsPass::run(MachineFunction &MF,
- MachineFunctionAnalysisManager &) {
+PreservedAnalyses
+SIFoldOperandsPass::run(MachineFunction &MF,
+ MachineFunctionAnalysisManager &MFAM) {
MFPropsModifier _(*this, MF);
- bool Changed = SIFoldOperandsImpl().run(MF);
+ const MachineLoopInfo *MLI = &MFAM.getResult<MachineLoopAnalysis>(MF);
+ bool Changed = SIFoldOperandsImpl().run(MF, MLI);
if (!Changed) {
return PreservedAnalyses::all();
}
auto PA = getMachineFunctionPassPreservedAnalyses();
PA.preserveSet<CFGAnalyses>();
+ PA.preserve<MachineLoopAnalysis>();
return PA;
}
diff --git a/llvm/test/CodeGen/AMDGPU/do-not-fold-copy.mir b/llvm/test/CodeGen/AMDGPU/do-not-fold-copy.mir
index 5c206da8c544f..37f76d66b4ceb 100644
--- a/llvm/test/CodeGen/AMDGPU/do-not-fold-copy.mir
+++ b/llvm/test/CodeGen/AMDGPU/do-not-fold-copy.mir
@@ -54,3 +54,132 @@ body: |
%11:vgpr_32 = V_SET_INACTIVE_B32 0, %9, 0, 0, killed %10, implicit $exec
S_ENDPGM 0
...
+
+# An SGPR->VGPR copy with no implicit $exec read, inserted in a divergent loop
+# to latch a per-lane value, is read after the loop. SIFoldOperands must not
+# fold the scalar source into that exit use: it escapes the loop, so the fold
+# would drop the per-lane snapshot.
+---
+name: do_not_fold_sgpr_to_vgpr_copy_escaping_loop
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: do_not_fold_sgpr_to_vgpr_copy_escaping_loop
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $sgpr0, $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:sreg_32 = COPY $sgpr0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+ ; CHECK-NEXT: [[S_MOV_B64_:%[0-9]+]]:sreg_64 = S_MOV_B64 0
+ ; CHECK-NEXT: [[S_MOV_B32_:%[0-9]+]]:sreg_32 = S_MOV_B32 0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[PHI:%[0-9]+]]:sreg_64 = PHI [[S_MOV_B64_]], %bb.0, %5, %bb.1
+ ; CHECK-NEXT: [[PHI1:%[0-9]+]]:sreg_32 = PHI [[S_MOV_B32_]], %bb.0, %7, %bb.1
+ ; CHECK-NEXT: [[S_XOR_B32_:%[0-9]+]]:sreg_32 = S_XOR_B32 [[COPY]], [[PHI1]], implicit-def dead $scc
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:vgpr_32 = COPY [[S_XOR_B32_]]
+ ; CHECK-NEXT: [[S_ADD_I32_:%[0-9]+]]:sreg_32 = S_ADD_I32 [[PHI1]], 1, implicit-def dead $scc
+ ; CHECK-NEXT: [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 [[COPY1]], [[S_ADD_I32_]], implicit $exec
+ ; CHECK-NEXT: [[SI_IF_BREAK:%[0-9]+]]:sreg_64 = SI_IF_BREAK [[V_CMP_EQ_U32_e64_]], [[PHI]], implicit-def dead $scc
+ ; CHECK-NEXT: SI_LOOP [[SI_IF_BREAK]], %bb.1, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; CHECK-NEXT: S_BRANCH %bb.2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: SI_END_CF [[SI_IF_BREAK]], implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; CHECK-NEXT: [[V_ADD_U32_e64_:%[0-9]+]]:vgpr_32 = V_ADD_U32_e64 [[COPY2]], 1, 0, implicit $exec
+ ; CHECK-NEXT: $vgpr0 = COPY [[V_ADD_U32_e64_]]
+ ; CHECK-NEXT: SI_RETURN implicit $vgpr0
+ bb.0:
+ successors: %bb.1
+ liveins: $sgpr0, $vgpr0
+
+ %0:sreg_32 = COPY $sgpr0
+ %7:vgpr_32 = COPY $vgpr0
+ %8:sreg_64 = S_MOV_B64 0
+ %9:sreg_32 = S_MOV_B32 0
+
+ bb.1:
+ successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+
+ %1:sreg_64 = PHI %8, %bb.0, %4, %bb.1
+ %2:sreg_32 = PHI %9, %bb.0, %6, %bb.1
+ %3:sreg_32 = S_XOR_B32 %0, %2, implicit-def dead $scc
+ %5:vgpr_32 = COPY %3
+ %6:sreg_32 = S_ADD_I32 %2, 1, implicit-def dead $scc
+ %10:sreg_64 = V_CMP_EQ_U32_e64 %7, %6, implicit $exec
+ %4:sreg_64 = SI_IF_BREAK %10, %1, implicit-def dead $scc
+ SI_LOOP %4, %bb.1, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.2
+
+ bb.2:
+ SI_END_CF %4, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ %13:vgpr_32 = V_ADD_U32_e64 %5, 1, 0, implicit $exec
+ $vgpr0 = COPY %13
+ SI_RETURN implicit $vgpr0
+...
+
+# Same latch, but the loop-exit use is itself a COPY. The scalar source must
+# not be propagated through that exit copy either (separate fold path).
+---
+name: do_not_fold_sgpr_to_vgpr_copy_escaping_loop_via_copy_use
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: do_not_fold_sgpr_to_vgpr_copy_escaping_loop_via_copy_use
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.1(0x80000000)
+ ; CHECK-NEXT: liveins: $sgpr0, $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:sreg_32 = COPY $sgpr0
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+ ; CHECK-NEXT: [[S_MOV_B64_:%[0-9]+]]:sreg_64 = S_MOV_B64 0
+ ; CHECK-NEXT: [[S_MOV_B32_:%[0-9]+]]:sreg_32 = S_MOV_B32 0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[PHI:%[0-9]+]]:sreg_64 = PHI [[S_MOV_B64_]], %bb.0, %5, %bb.1
+ ; CHECK-NEXT: [[PHI1:%[0-9]+]]:sreg_32 = PHI [[S_MOV_B32_]], %bb.0, %7, %bb.1
+ ; CHECK-NEXT: [[S_XOR_B32_:%[0-9]+]]:sreg_32 = S_XOR_B32 [[COPY]], [[PHI1]], implicit-def dead $scc
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:vgpr_32 = COPY [[S_XOR_B32_]]
+ ; CHECK-NEXT: [[S_ADD_I32_:%[0-9]+]]:sreg_32 = S_ADD_I32 [[PHI1]], 1, implicit-def dead $scc
+ ; CHECK-NEXT: [[V_CMP_EQ_U32_e64_:%[0-9]+]]:sreg_64 = V_CMP_EQ_U32_e64 [[COPY1]], [[S_ADD_I32_]], implicit $exec
+ ; CHECK-NEXT: [[SI_IF_BREAK:%[0-9]+]]:sreg_64 = SI_IF_BREAK [[V_CMP_EQ_U32_e64_]], [[PHI]], implicit-def dead $scc
+ ; CHECK-NEXT: SI_LOOP [[SI_IF_BREAK]], %bb.1, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; CHECK-NEXT: S_BRANCH %bb.2
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: SI_END_CF [[SI_IF_BREAK]], implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ ; CHECK-NEXT: [[V_ADD_U32_e64_:%[0-9]+]]:vgpr_32 = V_ADD_U32_e64 [[COPY2]], 1, 0, implicit $exec
+ ; CHECK-NEXT: $vgpr0 = COPY [[V_ADD_U32_e64_]]
+ ; CHECK-NEXT: SI_RETURN implicit $vgpr0
+ bb.0:
+ successors: %bb.1
+ liveins: $sgpr0, $vgpr0
+
+ %0:sreg_32 = COPY $sgpr0
+ %7:vgpr_32 = COPY $vgpr0
+ %8:sreg_64 = S_MOV_B64 0
+ %9:sreg_32 = S_MOV_B32 0
+
+ bb.1:
+ successors: %bb.2(0x04000000), %bb.1(0x7c000000)
+
+ %1:sreg_64 = PHI %8, %bb.0, %4, %bb.1
+ %2:sreg_32 = PHI %9, %bb.0, %6, %bb.1
+ %3:sreg_32 = S_XOR_B32 %0, %2, implicit-def dead $scc
+ %5:vgpr_32 = COPY %3
+ %6:sreg_32 = S_ADD_I32 %2, 1, implicit-def dead $scc
+ %10:sreg_64 = V_CMP_EQ_U32_e64 %7, %6, implicit $exec
+ %4:sreg_64 = SI_IF_BREAK %10, %1, implicit-def dead $scc
+ SI_LOOP %4, %bb.1, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ S_BRANCH %bb.2
+
+ bb.2:
+ SI_END_CF %4, implicit-def dead $exec, implicit-def dead $scc, implicit $exec
+ %11:vgpr_32 = COPY %5
+ %13:vgpr_32 = V_ADD_U32_e64 %11, 1, 0, implicit $exec
+ $vgpr0 = COPY %13
+ SI_RETURN implicit $vgpr0
+...
More information about the llvm-commits
mailing list