[llvm] [X86] Fix commuteSelect miscompile with double-used condition value (PR #219436)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 28 03:46:02 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86
Author: Timur Golubovich (timurgol007)
<details>
<summary>Changes</summary>
The multi-use loop iterated `Cond->users()` which yields duplicates when a select uses the condition in multiple operand positions. This caused a double-commute and iterator corruption, leaving other selects with an inverted condition but unswapped operands.
Remove the in-place mutation loop and return a new select node. The DAG combiner visits each select independently and `getSetCC` CSEs the inverted condition so all commuted selects share it.
---
Full diff: https://github.com/llvm/llvm-project/pull/219436.diff
2 Files Affected:
- (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+2-14)
- (modified) llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll (+67)
``````````diff
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index c8a38aa1bb154..e92a1566fd35f 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -48701,8 +48701,7 @@ static SDValue commuteSelect(SDNode *N, SelectionDAG &DAG, const SDLoc &DL,
return SDValue();
// For multi-use setcc, check that all users are vselects that benefit.
- bool CondHasOneUse = Cond.hasOneUse();
- if (!CondHasOneUse) {
+ if (!Cond.hasOneUse()) {
if (!llvm::all_of(Cond->users(), [&](SDNode *User) {
SDValue UserLHS, UserRHS;
return sd_match(User, m_VSelect(m_Specific(Cond), m_Value(UserLHS),
@@ -48717,18 +48716,7 @@ static SDValue commuteSelect(SDNode *N, SelectionDAG &DAG, const SDLoc &DL,
// (vselect M, L, R) -> (vselect ~M, R, L)
ISD::CondCode NewCC = ISD::getSetCCInverse(CC, X.getValueType());
SDValue NewCond = DAG.getSetCC(SDLoc(Cond), Cond.getValueType(), X, Y, NewCC);
- if (CondHasOneUse)
- return DAG.getSelect(DL, LHS.getValueType(), NewCond, RHS, LHS);
-
- // Invert the setcc for all users and commute all vselects.
- for (SDNode *User : llvm::make_early_inc_range(Cond->users())) {
- SDValue UserLHS = User->getOperand(1);
- SDValue UserRHS = User->getOperand(2);
- [[maybe_unused]] SDNode *Updated =
- DAG.UpdateNodeOperands(User, NewCond, UserRHS, UserLHS);
- assert(Updated == User && "Unexpected CSE in commuteSelect");
- }
- return SDValue(N, 0);
+ return DAG.getSelect(DL, LHS.getValueType(), NewCond, RHS, LHS);
}
/// Do target-specific dag combines on SELECT and VSELECT nodes.
diff --git a/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll b/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
index b11ad0df8bbe8..aa3e7d0167c80 100644
--- a/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
+++ b/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
@@ -52,3 +52,70 @@ exit:
store <16 x float> %res_max, ptr %pMax, align 64
ret void
}
+
+; Verify that commuteSelect does not miscompile when the inverted setcc already
+; exists (CSE). Both icmp eq and icmp ne are present so getSetCCInverse must
+; not create an infinite loop or corrupt operands.
+
+define <8 x i32> @commute_select_existing_inverse_cmp(<8 x i32> %src) {
+; CHECK-LABEL: commute_select_existing_inverse_cmp:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: subq $56, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 64
+; CHECK-NEXT: vmovdqa {{.*#+}} ymm1 = [0,1,2,3,4,5,6,7]
+; CHECK-NEXT: vmovdqu %ymm0, {{[-0-9]+}}(%r{{[sb]}}p) # 32-byte Spill
+; CHECK-NEXT: vpcmpneqd %ymm1, %ymm0, %k1
+; CHECK-NEXT: kmovw %k1, {{[-0-9]+}}(%r{{[sb]}}p) # 2-byte Spill
+; CHECK-NEXT: vpcmpeqd %ymm1, %ymm0, %ymm2
+; CHECK-NEXT: vpcmpeqd %ymm1, %ymm1, %ymm1
+; CHECK-NEXT: vpsubd %ymm1, %ymm2, %ymm1 {%k1} {z}
+; CHECK-NEXT: xorl %eax, %eax
+; CHECK-NEXT: vpxor %xmm0, %xmm0, %xmm0
+; CHECK-NEXT: xorl %edi, %edi
+; CHECK-NEXT: xorl %esi, %esi
+; CHECK-NEXT: callq *%rax
+; CHECK-NEXT: vpxor %xmm0, %xmm0, %xmm0
+; CHECK-NEXT: kmovw {{[-0-9]+}}(%r{{[sb]}}p), %k1 # 2-byte Reload
+; CHECK-NEXT: vpsubd {{[-0-9]+}}(%r{{[sb]}}p), %ymm0, %ymm0 {%k1} {z} # 32-byte Folded Reload
+; CHECK-NEXT: addq $56, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 8
+; CHECK-NEXT: retq
+entry:
+ %eq = icmp eq <8 x i32> %src, <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+ %ne = icmp ne <8 x i32> %src, <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+ %ne_ext = sext <8 x i1> %ne to <8 x i32>
+ %sel1 = select <8 x i1> %eq, <8 x i32> zeroinitializer, <8 x i32> %ne_ext
+ %neg1 = sub <8 x i32> zeroinitializer, %sel1
+ %cast = bitcast <8 x i32> %neg1 to <4 x i64>
+ tail call void null(<4 x i64> zeroinitializer, <4 x i64> %cast, ptr null, i32 0)
+ %sel2 = select <8 x i1> %eq, <8 x i32> zeroinitializer, <8 x i32> %src
+ %neg2 = sub <8 x i32> zeroinitializer, %sel2
+ ret <8 x i32> %neg2
+}
+
+; Verify that commuteSelect handles a setcc used both as the condition and as a
+; value operand (double-use). The double-use select stays uncommmuted while the
+; other select still gets the masked-add optimization.
+
+define <16 x i32> @commute_select_cond_used_as_value(<16 x i32> %a, <16 x i32> %b, <16 x i32> %c, <16 x i32> %d, <16 x i1> %mask1, <16 x i1> %mask2, ptr %out) {
+; CHECK-LABEL: commute_select_cond_used_as_value:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: vpcmpnltd %zmm1, %zmm0, %k1
+; CHECK-NEXT: vpcmpgtd %zmm0, %zmm1, %k0
+; CHECK-NEXT: vpxor %xmm5, %xmm4, %xmm1
+; CHECK-NEXT: vpsllw $7, %xmm1, %xmm1
+; CHECK-NEXT: vpmovb2m %xmm1, %k2
+; CHECK-NEXT: korw %k2, %k0, %k0
+; CHECK-NEXT: vpaddd %zmm3, %zmm2, %zmm0 {%k1}
+; CHECK-NEXT: kmovw %k0, (%rdi)
+; CHECK-NEXT: retq
+entry:
+ %cmp = icmp slt <16 x i32> %a, %b
+ %mask_or = xor <16 x i1> %mask1, %mask2
+ %sel_mask = select <16 x i1> %cmp, <16 x i1> %cmp, <16 x i1> %mask_or
+ %add = add <16 x i32> %c, %d
+ %sel_val = select <16 x i1> %cmp, <16 x i32> %a, <16 x i32> %add
+ %bits = bitcast <16 x i1> %sel_mask to i16
+ store i16 %bits, ptr %out
+ ret <16 x i32> %sel_val
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/219436
More information about the llvm-commits
mailing list