[llvm] [AMDGPU] Fix combineMasks dropping condition (PR #203180)
Arseniy Obolenskiy via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 4 20:46:44 PDT 2026
https://github.com/aobolensk updated https://github.com/llvm/llvm-project/pull/203180
>From 3a7a3091e812e3cc7a6c9c71423302fd3578b210 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Thu, 11 Jun 2026 08:23:55 +0200
Subject: [PATCH 1/3] [AMDGPU] Fix combineMasks dropping condition
The problem is around `in S_AND exec, (S_AND x, x)` case
When the nested mask op is the outer S_AND/S_OR first operand with two identical operands, combineMasks kept exec instead of a nested operand, folding to S_AND exec, exec and dropping the condition
---
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp | 21 +++++++----
.../lower-control-flow-other-terminators.mir | 36 +++++++++++++++++++
2 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
index 9cc86e84407b1..51e370b3e5d3f 100644
--- a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
@@ -607,15 +607,24 @@ void SILowerControlFlow::combineMasks(MachineInstr &MI) {
findMaskOperands(MI, 2, Ops);
if (Ops.size() != 3) return;
- unsigned UniqueOpndIdx;
- if (Ops[0].isIdenticalTo(Ops[1])) UniqueOpndIdx = 2;
- else if (Ops[0].isIdenticalTo(Ops[2])) UniqueOpndIdx = 1;
- else if (Ops[1].isIdenticalTo(Ops[2])) UniqueOpndIdx = 1;
- else return;
+ // Ops holds the exec operand and the two operands of the nested op.
+ // Always keep a nested operand, never the exec operand.
+ unsigned ExecIdx = OpToReplace == 1 ? 2 : 0;
+ unsigned NestedLHS = OpToReplace == 1 ? 0 : 1;
+ unsigned NestedRHS = NestedLHS + 1;
+
+ unsigned KeepIdx;
+ if (Ops[ExecIdx].isIdenticalTo(Ops[NestedLHS]))
+ KeepIdx = NestedRHS;
+ else if (Ops[ExecIdx].isIdenticalTo(Ops[NestedRHS]) ||
+ Ops[NestedLHS].isIdenticalTo(Ops[NestedRHS]))
+ KeepIdx = NestedLHS;
+ else
+ return;
Register Reg = MI.getOperand(OpToReplace).getReg();
MI.removeOperand(OpToReplace);
- MI.addOperand(Ops[UniqueOpndIdx]);
+ MI.addOperand(Ops[KeepIdx]);
if (MRI->use_empty(Reg))
MRI->getUniqueVRegDef(Reg)->eraseFromParent();
}
diff --git a/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir b/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir
index eaf398fd51723..6feabac4bb2fd 100644
--- a/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir
+++ b/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir
@@ -270,3 +270,39 @@ body: |
S_BRANCH %bb.2
...
+
+# combineMasks must keep a nested operand, not the exec leaf, when the nested
+# S_AND is the first operand and has identical operands (S_AND %0, %0). The
+# result must be S_AND exec, %0, not S_AND exec, exec.
+
+---
+name: combine_masks_nested_first_operand
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: combine_masks_nested_first_operand
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.0(0x40000000), %bb.1(0x40000000)
+ ; CHECK-NEXT: liveins: $sgpr0_sgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:sreg_64 = COPY $sgpr0_sgpr1
+ ; CHECK-NEXT: [[S_AND_B64_:%[0-9]+]]:sreg_64 = S_AND_B64 $exec, [[COPY]], implicit-def $scc
+ ; CHECK-NEXT: $exec = S_ANDN2_B64_term $exec, [[S_AND_B64_]], implicit-def $scc
+ ; CHECK-NEXT: S_CBRANCH_EXECNZ %bb.0, implicit $exec
+ ; CHECK-NEXT: S_BRANCH %bb.1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: S_ENDPGM 0
+ bb.0:
+ successors: %bb.0, %bb.1
+ liveins: $sgpr0_sgpr1
+
+ %0:sreg_64 = COPY $sgpr0_sgpr1
+ %1:sreg_64 = S_AND_B64 %0, %0, implicit-def dead $scc
+ %2:sreg_64 = S_AND_B64 %1, $exec, implicit-def $scc
+ SI_LOOP %2, %bb.0, implicit-def $exec, implicit-def $scc, implicit $exec
+ S_BRANCH %bb.1
+
+ bb.1:
+ S_ENDPGM 0
+
+...
>From ab5c064b32af72131cbf6b614365b8dadf1cd983 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Wed, 29 Jul 2026 07:11:43 +0200
Subject: [PATCH 2/3] Address comment
---
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp | 50 +++++++++++--------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
index 648f81af8ae12..417b2bb3e8338 100644
--- a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
@@ -602,31 +602,41 @@ void SILowerControlFlow::findMaskOperands(MachineInstr &MI, unsigned OpNo,
// One of the operands is exec mask.
void SILowerControlFlow::combineMasks(MachineInstr &MI) {
assert(MI.getNumExplicitOperands() == 3);
- SmallVector<MachineOperand, 4> Ops;
- unsigned OpToReplace = 1;
- findMaskOperands(MI, 1, Ops);
- if (Ops.size() == 1) OpToReplace = 2; // First operand can be exec or its copy
- findMaskOperands(MI, 2, Ops);
- if (Ops.size() != 3) return;
-
- // Ops holds the exec operand and the two operands of the nested op.
- // Always keep a nested operand, never the exec operand.
- unsigned ExecIdx = OpToReplace == 1 ? 2 : 0;
- unsigned NestedLHS = OpToReplace == 1 ? 0 : 1;
- unsigned NestedRHS = NestedLHS + 1;
-
- unsigned KeepIdx;
- if (Ops[ExecIdx].isIdenticalTo(Ops[NestedLHS]))
- KeepIdx = NestedRHS;
- else if (Ops[ExecIdx].isIdenticalTo(Ops[NestedRHS]) ||
- Ops[NestedLHS].isIdenticalTo(Ops[NestedRHS]))
- KeepIdx = NestedLHS;
+ SmallVector<MachineOperand, 2> Src1, Src2;
+ findMaskOperands(MI, 1, Src1);
+ findMaskOperands(MI, 2, Src2);
+
+ // Exactly one of the two operands must resolve to the nested LHS and RHS.
+ // Another one must resolve to a single value, exec or its copy.
+ unsigned OpToReplace;
+ MachineOperand *Leaf, *NestedLHS, *NestedRHS;
+ if (Src1.size() == 2 && Src2.size() == 1) {
+ OpToReplace = 1;
+ NestedLHS = &Src1[0];
+ NestedRHS = &Src1[1];
+ Leaf = &Src2[0];
+ } else if (Src1.size() == 1 && Src2.size() == 2) {
+ OpToReplace = 2;
+ Leaf = &Src1[0];
+ NestedLHS = &Src2[0];
+ NestedRHS = &Src2[1];
+ } else {
+ return;
+ }
+
+ // Always keep a nested operand, never the leaf operand.
+ MachineOperand *KeepOp;
+ if (Leaf->isIdenticalTo(*NestedLHS))
+ KeepOp = NestedRHS;
+ else if (Leaf->isIdenticalTo(*NestedRHS) ||
+ NestedLHS->isIdenticalTo(*NestedRHS))
+ KeepOp = NestedLHS;
else
return;
Register Reg = MI.getOperand(OpToReplace).getReg();
MI.removeOperand(OpToReplace);
- MI.addOperand(Ops[KeepIdx]);
+ MI.addOperand(*KeepOp);
if (MRI->use_empty(Reg))
MRI->getUniqueVRegDef(Reg)->eraseFromParent();
}
>From b720fb4d9f1030ad4951eb26300eb5a18faa38ee Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Tue, 4 Aug 2026 15:42:15 +0200
Subject: [PATCH 3/3] Address comments
---
llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp | 27 +++++++-------
.../lower-control-flow-other-terminators.mir | 36 +++++++++++++++++++
2 files changed, 50 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
index 417b2bb3e8338..25694a53f35dd 100644
--- a/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
+++ b/llvm/lib/Target/AMDGPU/SILowerControlFlow.cpp
@@ -102,7 +102,7 @@ class SILowerControlFlow {
MachineBasicBlock *emitEndCf(MachineInstr &MI);
void findMaskOperands(MachineInstr &MI, unsigned OpNo,
- SmallVectorImpl<MachineOperand> &Src) const;
+ SmallVectorImpl<MachineOperand *> &Src) const;
void combineMasks(MachineInstr &MI);
@@ -569,11 +569,12 @@ MachineBasicBlock *SILowerControlFlow::emitEndCf(MachineInstr &MI) {
// Returns replace operands for a logical operation, either single result
// for exec or two operands if source was another equivalent operation.
-void SILowerControlFlow::findMaskOperands(MachineInstr &MI, unsigned OpNo,
- SmallVectorImpl<MachineOperand> &Src) const {
+void SILowerControlFlow::findMaskOperands(
+ MachineInstr &MI, unsigned OpNo,
+ SmallVectorImpl<MachineOperand *> &Src) const {
MachineOperand &Op = MI.getOperand(OpNo);
if (!Op.isReg() || !Op.getReg().isVirtual()) {
- Src.push_back(Op);
+ Src.push_back(&Op);
return;
}
@@ -590,10 +591,10 @@ void SILowerControlFlow::findMaskOperands(MachineInstr &MI, unsigned OpNo,
!(I->isCopy() && I->getOperand(0).getReg() != LMC.ExecReg))
return;
- for (const auto &SrcOp : Def->explicit_operands())
+ for (MachineOperand &SrcOp : Def->explicit_operands())
if (SrcOp.isReg() && SrcOp.isUse() &&
(SrcOp.getReg().isVirtual() || SrcOp.getReg() == LMC.ExecReg))
- Src.push_back(SrcOp);
+ Src.push_back(&SrcOp);
}
// Search and combine pairs of equivalent instructions, like
@@ -602,7 +603,7 @@ void SILowerControlFlow::findMaskOperands(MachineInstr &MI, unsigned OpNo,
// One of the operands is exec mask.
void SILowerControlFlow::combineMasks(MachineInstr &MI) {
assert(MI.getNumExplicitOperands() == 3);
- SmallVector<MachineOperand, 2> Src1, Src2;
+ SmallVector<MachineOperand *, 2> Src1, Src2;
findMaskOperands(MI, 1, Src1);
findMaskOperands(MI, 2, Src2);
@@ -612,14 +613,14 @@ void SILowerControlFlow::combineMasks(MachineInstr &MI) {
MachineOperand *Leaf, *NestedLHS, *NestedRHS;
if (Src1.size() == 2 && Src2.size() == 1) {
OpToReplace = 1;
- NestedLHS = &Src1[0];
- NestedRHS = &Src1[1];
- Leaf = &Src2[0];
+ NestedLHS = Src1[0];
+ NestedRHS = Src1[1];
+ Leaf = Src2[0];
} else if (Src1.size() == 1 && Src2.size() == 2) {
OpToReplace = 2;
- Leaf = &Src1[0];
- NestedLHS = &Src2[0];
- NestedRHS = &Src2[1];
+ Leaf = Src1[0];
+ NestedLHS = Src2[0];
+ NestedRHS = Src2[1];
} else {
return;
}
diff --git a/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir b/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir
index 396a2e5fc6858..84b348fd86bb0 100644
--- a/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir
+++ b/llvm/test/CodeGen/AMDGPU/lower-control-flow-other-terminators.mir
@@ -306,3 +306,39 @@ body: |
S_ENDPGM 0
...
+
+# combineMasks must keep a nested operand, not the exec leaf, when the nested
+# S_AND is the second operand and has identical operands (S_AND %0, %0). The
+# result must be S_AND exec, %0, not S_AND exec, exec.
+
+---
+name: combine_masks_nested_second_operand
+tracksRegLiveness: true
+body: |
+ ; CHECK-LABEL: name: combine_masks_nested_second_operand
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.0(0x40000000), %bb.1(0x40000000)
+ ; CHECK-NEXT: liveins: $sgpr0_sgpr1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:sreg_64 = COPY $sgpr0_sgpr1
+ ; CHECK-NEXT: [[S_AND_B64_:%[0-9]+]]:sreg_64 = S_AND_B64 $exec, [[COPY]], implicit-def $scc
+ ; CHECK-NEXT: $exec = S_ANDN2_B64_term $exec, [[S_AND_B64_]], implicit-def $scc
+ ; CHECK-NEXT: S_CBRANCH_EXECNZ %bb.0, implicit $exec
+ ; CHECK-NEXT: S_BRANCH %bb.1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: S_ENDPGM 0
+ bb.0:
+ successors: %bb.0, %bb.1
+ liveins: $sgpr0_sgpr1
+
+ %0:sreg_64 = COPY $sgpr0_sgpr1
+ %1:sreg_64 = S_AND_B64 %0, %0, implicit-def dead $scc
+ %2:sreg_64 = S_AND_B64 $exec, %1, implicit-def $scc
+ SI_LOOP %2, %bb.0, implicit-def $exec, implicit-def $scc, implicit $exec
+ S_BRANCH %bb.1
+
+ bb.1:
+ S_ENDPGM 0
+
+...
More information about the llvm-commits
mailing list