[llvm] [AMDGPU][GlobalISel][SelectionDAG] Refactor control-flow intrinsic branch matching (PR #219173)
Alexander Hück via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 27 04:03:08 PDT 2026
https://github.com/ahueck created https://github.com/llvm/llvm-project/pull/219173
This is PR 1 of 4 in a series adding `llvm.is.debugging.enabled` intrinsic to LLVM IR and lowering this intrinsic for AMDGPU targets.
A RFC post will follow.
## Changes of this PR
Refactor AMDGPU control-flow intrinsic branch matching in both GlobalISel and SelectionDAG:
- Separate structural matching from MIR mutation in GlobalISel, ensuring failed matches leave the MIR unchanged.
- Normalize direct, wrapped, and negated branch conditions into explicit true and false targets.
- Centralize fallthrough-edge redirection and condition-wrapper cleanup.
- Add MIR coverage for a failed match involving a negated condition and invalid branch layout.
PR 3 reuses these matching and branch-redirection helpers when fusing `llvm.is.debugging.enabled` branch uses into `s_cbranch_cdbgsys_or_user`. This avoids certain code duplication by having common functionality outlined.
>From a0a8085981d024e89ad8c1ebad2b8730ab707f98 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..0f0812a5cac36 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 61e8be4e9f5dfb6ce64cf9ee1ab6e78af9a7715c 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