[llvm] [AMDGPU] Fix combineMasks dropping condition (PR #203180)
Arseniy Obolenskiy via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 28 22:11:55 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/2] [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/2] 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();
}
More information about the llvm-commits
mailing list