[llvm] [AMDGPU][GlobalISel][SelectionDAG] Refactor control-flow intrinsic branch matching (PR #219173)
Alexander Hück via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 28 02:59:35 PDT 2026
https://github.com/ahueck updated https://github.com/llvm/llvm-project/pull/219173
>From f27cfacb675eff62c040cda83321d2df12bfd5c2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexander=20H=C3=BCck?= <alexander.huck at amd.com>
Date: Mon, 17 Aug 2026 07:28:30 -0400
Subject: [PATCH 1/2] [AMDGPU][GlobalISel] Make control-flow intrinsic matching
non-mutating
Refactor AMDGPULegalizer's control-flow intrinsic handling to separate
structural matching from MIR mutation. Replace verifyCFIntrinsic with an
explicit match-and-commit flow, and share branch fallthrough redirection
across the affected legalization paths.
---
.../lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp | 134 ++++++++++--------
.../GlobalISel/legalize-amdgcn.if-invalid.mir | 27 +++-
2 files changed, 97 insertions(+), 64 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
index 7c1a26f761c96..ed60350c7fc2d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
@@ -4804,47 +4804,77 @@ static bool isNot(const MachineRegisterInfo &MRI, const MachineInstr &MI) {
return ConstVal == -1;
}
-// Return the use branch instruction, otherwise null if the usage is invalid.
-static MachineInstr *
-verifyCFIntrinsic(MachineInstr &MI, MachineRegisterInfo &MRI, MachineInstr *&Br,
- MachineBasicBlock *&UncondBrTarget, bool &Negated) {
+namespace {
+struct CFIntrinsicBranchMatch {
+ MachineInstr *Negation = nullptr;
+ MachineInstr *CondBr = nullptr;
+ MachineInstr *UncondBr = nullptr;
+
+ MachineBasicBlock *ConditionTrueTarget = nullptr;
+ MachineBasicBlock *ConditionFalseTarget = nullptr;
+
+ bool isNegated() const { return Negation != nullptr; }
+
+ // Retarget the explicit branch for the fallthrough edge, or materialize
+ // the branch when that edge was represented by layout fallthrough. The
+ // builder must be positioned at the replacement branch.
+ void redirectFallthroughEdge(MachineIRBuilder &B,
+ MachineBasicBlock &Target) const {
+ if (UncondBr)
+ UncondBr->getOperand(0).setMBB(&Target);
+ else
+ B.buildBr(Target);
+ }
+
+ void eraseDeadNegation(MachineRegisterInfo &MRI) {
+ if (Negation)
+ eraseInstr(*Negation, MRI);
+ }
+};
+} // namespace
+
+static std::optional<CFIntrinsicBranchMatch>
+matchCFIntrinsicBranchUse(MachineInstr &MI, MachineRegisterInfo &MRI) {
Register CondDef = MI.getOperand(0).getReg();
if (!MRI.hasOneNonDBGUse(CondDef))
- return nullptr;
+ return std::nullopt;
MachineBasicBlock *Parent = MI.getParent();
MachineInstr *UseMI = &*MRI.use_instr_nodbg_begin(CondDef);
+ MachineInstr *Negation = nullptr;
if (isNot(MRI, *UseMI)) {
+ Negation = UseMI;
Register NegatedCond = UseMI->getOperand(0).getReg();
if (!MRI.hasOneNonDBGUse(NegatedCond))
- return nullptr;
-
- // We're deleting the def of this value, so we need to remove it.
- eraseInstr(*UseMI, MRI);
+ return std::nullopt;
UseMI = &*MRI.use_instr_nodbg_begin(NegatedCond);
- Negated = true;
}
if (UseMI->getParent() != Parent || UseMI->getOpcode() != AMDGPU::G_BRCOND)
- return nullptr;
+ return std::nullopt;
// Make sure the cond br is followed by a G_BR, or is the last instruction.
+ MachineInstr *UncondBr = nullptr;
+ MachineBasicBlock *OtherTarget = nullptr;
MachineBasicBlock::iterator Next = std::next(UseMI->getIterator());
if (Next == Parent->end()) {
MachineFunction::iterator NextMBB = std::next(Parent->getIterator());
if (NextMBB == Parent->getParent()->end()) // Illegal intrinsic use.
- return nullptr;
- UncondBrTarget = &*NextMBB;
+ return std::nullopt;
+ OtherTarget = &*NextMBB;
} else {
if (Next->getOpcode() != AMDGPU::G_BR)
- return nullptr;
- Br = &*Next;
- UncondBrTarget = Br->getOperand(0).getMBB();
+ return std::nullopt;
+ UncondBr = &*Next;
+ OtherTarget = UncondBr->getOperand(0).getMBB();
}
- return UseMI;
+ MachineBasicBlock *TakenTarget = UseMI->getOperand(1).getMBB();
+ return CFIntrinsicBranchMatch{Negation, UseMI, UncondBr,
+ Negation ? OtherTarget : TakenTarget,
+ Negation ? TakenTarget : OtherTarget};
}
void AMDGPULegalizerInfo::buildLoadInputValue(Register DstReg,
@@ -8230,80 +8260,58 @@ bool AMDGPULegalizerInfo::legalizeIntrinsic(LegalizerHelper &Helper,
return true;
case Intrinsic::amdgcn_if:
case Intrinsic::amdgcn_else: {
- MachineInstr *Br = nullptr;
- MachineBasicBlock *UncondBrTarget = nullptr;
- bool Negated = false;
- if (MachineInstr *BrCond =
- verifyCFIntrinsic(MI, MRI, Br, UncondBrTarget, Negated)) {
- const SIRegisterInfo *TRI
- = static_cast<const SIRegisterInfo *>(MRI.getTargetRegisterInfo());
+ if (auto Match = matchCFIntrinsicBranchUse(MI, MRI)) {
+ const SIRegisterInfo *TRI =
+ static_cast<const SIRegisterInfo *>(MRI.getTargetRegisterInfo());
Register Def = MI.getOperand(1).getReg();
Register Use = MI.getOperand(3).getReg();
- MachineBasicBlock *CondBrTarget = BrCond->getOperand(1).getMBB();
-
- if (Negated)
- std::swap(CondBrTarget, UncondBrTarget);
-
- B.setInsertPt(B.getMBB(), BrCond->getIterator());
+ B.setInsertPt(B.getMBB(), Match->CondBr->getIterator());
if (IntrID == Intrinsic::amdgcn_if) {
B.buildInstr(AMDGPU::SI_IF)
- .addDef(Def)
- .addUse(Use)
- .addMBB(UncondBrTarget);
+ .addDef(Def)
+ .addUse(Use)
+ .addMBB(Match->ConditionFalseTarget);
} else {
B.buildInstr(AMDGPU::SI_ELSE)
.addDef(Def)
.addUse(Use)
- .addMBB(UncondBrTarget);
+ .addMBB(Match->ConditionFalseTarget);
}
- if (Br) {
- Br->getOperand(0).setMBB(CondBrTarget);
- } else {
- // The IRTranslator skips inserting the G_BR for fallthrough cases, but
- // since we're swapping branch targets it needs to be reinserted.
- // FIXME: IRTranslator should probably not do this
- B.buildBr(*CondBrTarget);
- }
+ // The IRTranslator skips inserting the G_BR for fallthrough cases, but
+ // since we're swapping branch targets it needs to be reinserted.
+ // FIXME: IRTranslator should probably not do this
+ Match->redirectFallthroughEdge(B, *Match->ConditionTrueTarget);
MRI.setRegClass(Def, TRI->getWaveMaskRegClass());
MRI.setRegClass(Use, TRI->getWaveMaskRegClass());
+ Match->eraseDeadNegation(MRI);
MI.eraseFromParent();
- BrCond->eraseFromParent();
+ Match->CondBr->eraseFromParent();
return true;
}
return false;
}
case Intrinsic::amdgcn_loop: {
- MachineInstr *Br = nullptr;
- MachineBasicBlock *UncondBrTarget = nullptr;
- bool Negated = false;
- if (MachineInstr *BrCond =
- verifyCFIntrinsic(MI, MRI, Br, UncondBrTarget, Negated)) {
- const SIRegisterInfo *TRI
- = static_cast<const SIRegisterInfo *>(MRI.getTargetRegisterInfo());
-
- MachineBasicBlock *CondBrTarget = BrCond->getOperand(1).getMBB();
- Register Reg = MI.getOperand(2).getReg();
+ if (auto Match = matchCFIntrinsicBranchUse(MI, MRI)) {
+ const SIRegisterInfo *TRI =
+ static_cast<const SIRegisterInfo *>(MRI.getTargetRegisterInfo());
- if (Negated)
- std::swap(CondBrTarget, UncondBrTarget);
+ Register Reg = MI.getOperand(2).getReg();
- B.setInsertPt(B.getMBB(), BrCond->getIterator());
+ B.setInsertPt(B.getMBB(), Match->CondBr->getIterator());
B.buildInstr(AMDGPU::SI_LOOP)
- .addUse(Reg)
- .addMBB(UncondBrTarget);
+ .addUse(Reg)
+ .addMBB(Match->ConditionFalseTarget);
- if (Br)
- Br->getOperand(0).setMBB(CondBrTarget);
- else
- B.buildBr(*CondBrTarget);
+ Match->redirectFallthroughEdge(B, *Match->ConditionTrueTarget);
+ Match->eraseDeadNegation(MRI);
MI.eraseFromParent();
- BrCond->eraseFromParent();
+ Match->CondBr->eraseFromParent();
MRI.setRegClass(Reg, TRI->getWaveMaskRegClass());
return true;
}
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-amdgcn.if-invalid.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-amdgcn.if-invalid.mir
index d351eb6a57446..77040216daca8 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-amdgcn.if-invalid.mir
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-amdgcn.if-invalid.mir
@@ -1,4 +1,5 @@
# RUN: llc -mtriple=amdgpu8.03-mesa-mesa3d -O0 -run-pass=legalizer -global-isel-abort=2 -pass-remarks-missed='gisel*' -filetype=null %s 2>&1 | FileCheck -check-prefix=ERR %s
+# RUN: llc -mtriple=amdgpu8.03-mesa-mesa3d -O0 -run-pass=legalizer -global-isel-abort=0 %s -o - | FileCheck -check-prefix=MIR %s
# Make sure incorrect usage of control flow intrinsics fails to select in case some transform separated the intrinsic from its branch.
@@ -8,7 +9,7 @@
# ERR-NEXT: remark: <unknown>:0:0: unable to legalize instruction: %3:_(s1), %4:_(s64) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.amdgcn.if), %2:_(s1) (in function: brcond_si_if_xor_0)
# ERR-NEXT: remark: <unknown>:0:0: unable to legalize instruction: %3:_(s1), %4:_(s64) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.amdgcn.if), %2:_(s1) (in function: brcond_si_if_or_neg1)
# ERR-NEXT: remark: <unknown>:0:0: unable to legalize instruction: %3:_(s1), %4:_(s64) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.amdgcn.if), %2:_(s1) (in function: brcond_si_if_negated_multi_use)
-
+# ERR-NEXT: remark: <unknown>:0:0: unable to legalize instruction: %3:_(s1), %4:_(s64) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.amdgcn.if), %2:_(s1) (in function: si_if_negated_invalid_branch_layout)
---
name: brcond_si_if_different_block
@@ -134,3 +135,27 @@ body: |
bb.3:
S_NOP 2
...
+
+# A failed match after recognizing the negation must not modify the MIR.
+# MIR-LABEL: name: si_if_negated_invalid_branch_layout
+# MIR: [[NOT:%[0-9]+]]:_(s1) = G_XOR
+# MIR-NEXT: G_BRCOND [[NOT]](s1), %bb.1
+# MIR-NEXT: S_ENDPGM 0
+---
+name: si_if_negated_invalid_branch_layout
+body: |
+ bb.0:
+ successors: %bb.1
+ liveins: $vgpr0, $vgpr1
+ %0:_(s32) = COPY $vgpr0
+ %1:_(s32) = COPY $vgpr1
+ %2:_(s1) = G_ICMP intpred(ne), %0, %1
+ %3:_(s1), %4:_(s64) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.amdgcn.if), %2
+ %5:_(s1) = G_CONSTANT i1 true
+ %6:_(s1) = G_XOR %3, %5
+ G_BRCOND %6, %bb.1
+ S_ENDPGM 0
+
+ bb.1:
+ S_ENDPGM 0
+...
>From 06d95fa53572e8da877a6c513a093a96a8017f84 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexander=20H=C3=BCck?= <alexander.huck at amd.com>
Date: Mon, 17 Aug 2026 11:58:25 -0400
Subject: [PATCH 2/2] [AMDGPU][SelectionDAG] Refactor control-flow branch
matching
Introduce BRCONDMatch to normalize wrapped and negated branch conditions
and record their true and false targets. Use the match in LowerBRCOND to
centralize fallthrough redirection and separate branch matching from
control-flow intrinsic lowering.
---
llvm/lib/Target/AMDGPU/SIISelLowering.cpp | 124 ++++++++++++++--------
1 file changed, 81 insertions(+), 43 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index d9810e3d9fbd9..86cfd885c4f8f 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -8584,6 +8584,72 @@ static SDNode *findUser(SDValue Value, unsigned Opcode) {
return nullptr;
}
+namespace {
+struct BRCONDMatch {
+ SDValue CondBr;
+ SDValue Condition;
+ SDValue ConditionWrapper;
+ SDNode *UncondBr = nullptr;
+
+ SDValue ConditionTrueTarget;
+ SDValue ConditionFalseTarget;
+
+ void redirectFallthroughEdge(SelectionDAG &DAG, SDValue Target) const {
+ if (UncondBr->getOperand(1) == Target)
+ return;
+
+ SDValue NewBr = DAG.getNode(ISD::BR, SDLoc(CondBr), UncondBr->getVTList(),
+ {UncondBr->getOperand(0), Target});
+ DAG.ReplaceAllUsesWith(UncondBr, NewBr.getNode());
+ }
+};
+} // namespace
+
+static std::optional<BRCONDMatch> matchBRCOND(SDValue CondBr) {
+ if (CondBr.getOpcode() != ISD::BRCOND)
+ return std::nullopt;
+
+ SDValue Condition = CondBr.getOperand(1);
+ SDValue ConditionWrapper;
+ bool IsNegated = false;
+
+ switch (Condition.getOpcode()) {
+ case ISD::SETCC: {
+ ISD::CondCode CC = cast<CondCodeSDNode>(Condition.getOperand(2))->get();
+ if (auto *C = dyn_cast<ConstantSDNode>(Condition.getOperand(1));
+ C && (CC == ISD::SETEQ || CC == ISD::SETNE)) {
+ IsNegated = (CC == ISD::SETEQ) == (C->getZExtValue() == 0);
+ ConditionWrapper = Condition;
+ Condition = Condition.getOperand(0);
+ }
+ break;
+ }
+ case ISD::XOR:
+ if (auto *C = dyn_cast<ConstantSDNode>(Condition.getOperand(1));
+ C && C->getZExtValue()) {
+ ConditionWrapper = Condition;
+ Condition = Condition.getOperand(0);
+ IsNegated = true;
+ }
+ break;
+ default:
+ break;
+ }
+
+ SDNode *UncondBr = findUser(CondBr, ISD::BR);
+ if (!UncondBr)
+ return std::nullopt;
+
+ SDValue TakenTarget = CondBr.getOperand(2);
+ SDValue OtherTarget = UncondBr->getOperand(1);
+ return BRCONDMatch{CondBr,
+ Condition,
+ ConditionWrapper,
+ UncondBr,
+ IsNegated ? OtherTarget : TakenTarget,
+ IsNegated ? TakenTarget : OtherTarget};
+}
+
unsigned SITargetLowering::isCFIntrinsic(const SDNode *Intr) const {
if (Intr->getOpcode() == ISD::INTRINSIC_W_CHAIN) {
switch (Intr->getConstantOperandVal(1)) {
@@ -8650,37 +8716,12 @@ bool SITargetLowering::shouldUseLDSConstAddress(const GlobalValue *GV) const {
/// This transforms the control flow intrinsics to get the branch destination as
/// last parameter, also switches branch target with BR if the need arise
SDValue SITargetLowering::LowerBRCOND(SDValue BRCOND, SelectionDAG &DAG) const {
- SDLoc DL(BRCOND);
+ auto Match = matchBRCOND(BRCOND);
+ if (!Match)
+ return BRCOND;
- SDNode *Intr = BRCOND.getOperand(1).getNode();
- SDValue Target = BRCOND.getOperand(2);
- SDNode *BR = nullptr;
- SDNode *SetCC = nullptr;
-
- switch (Intr->getOpcode()) {
- case ISD::SETCC: {
- // As long as we negate the condition everything is fine
- SetCC = Intr;
- Intr = SetCC->getOperand(0).getNode();
- break;
- }
- case ISD::XOR: {
- // Similar to SETCC, if we have (xor c, -1), we will be fine.
- SDValue LHS = Intr->getOperand(0);
- SDValue RHS = Intr->getOperand(1);
- if (auto *C = dyn_cast<ConstantSDNode>(RHS); C && C->getZExtValue()) {
- Intr = LHS.getNode();
- break;
- }
- [[fallthrough]];
- }
- default: {
- // Get the target from BR if we don't negate the condition
- BR = findUser(BRCOND, ISD::BR);
- assert(BR && "brcond missing unconditional branch user");
- Target = BR->getOperand(1);
- }
- }
+ SDLoc DL(Match->CondBr);
+ SDNode *Intr = Match->Condition.getNode();
unsigned CFNode = isCFIntrinsic(Intr);
if (CFNode == 0) {
@@ -8691,18 +8732,20 @@ SDValue SITargetLowering::LowerBRCOND(SDValue BRCOND, SelectionDAG &DAG) const {
bool HaveChain = Intr->getOpcode() == ISD::INTRINSIC_VOID ||
Intr->getOpcode() == ISD::INTRINSIC_W_CHAIN;
- assert(!SetCC ||
- (SetCC->getConstantOperandVal(1) == 1 &&
- cast<CondCodeSDNode>(SetCC->getOperand(2).getNode())->get() ==
- ISD::SETNE));
+ assert((!Match->ConditionWrapper ||
+ Match->ConditionWrapper.getOpcode() != ISD::SETCC ||
+ (Match->ConditionWrapper.getConstantOperandVal(1) == 1 &&
+ cast<CondCodeSDNode>(Match->ConditionWrapper.getOperand(2).getNode())
+ ->get() == ISD::SETNE)) &&
+ "unexpected control flow intrinsic condition wrapper");
// operands of the new intrinsic call
SmallVector<SDValue, 4> Ops;
if (HaveChain)
- Ops.push_back(BRCOND.getOperand(0));
+ Ops.push_back(Match->CondBr.getOperand(0));
Ops.append(Intr->op_begin() + (HaveChain ? 2 : 1), Intr->op_end());
- Ops.push_back(Target);
+ Ops.push_back(Match->ConditionFalseTarget);
ArrayRef<EVT> Res(Intr->value_begin() + 1, Intr->value_end());
@@ -8710,17 +8753,12 @@ SDValue SITargetLowering::LowerBRCOND(SDValue BRCOND, SelectionDAG &DAG) const {
SDNode *Result = DAG.getNode(CFNode, DL, DAG.getVTList(Res), Ops).getNode();
if (!HaveChain) {
- SDValue Ops[] = {SDValue(Result, 0), BRCOND.getOperand(0)};
+ SDValue Ops[] = {SDValue(Result, 0), Match->CondBr.getOperand(0)};
Result = DAG.getMergeValues(Ops, DL).getNode();
}
- if (BR) {
- // Give the branch instruction our target
- SDValue Ops[] = {BR->getOperand(0), BRCOND.getOperand(2)};
- SDValue NewBR = DAG.getNode(ISD::BR, DL, BR->getVTList(), Ops);
- DAG.ReplaceAllUsesWith(BR, NewBR.getNode());
- }
+ Match->redirectFallthroughEdge(DAG, Match->ConditionTrueTarget);
SDValue Chain = SDValue(Result, Result->getNumValues() - 1);
More information about the llvm-commits
mailing list