[llvm] [X86] Fix commuteSelect miscompile with double-used condition value (PR #219436)
Timur Golubovich via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 8 06:10:00 PDT 2026
https://github.com/timurgol007 updated https://github.com/llvm/llvm-project/pull/219436
>From 0e2887e639b26a0920b13fbb7ca8658817261c99 Mon Sep 17 00:00:00 2001
From: Timur Golubovich <timur.golubovich at intel.com>
Date: Thu, 27 Aug 2026 17:30:07 +0200
Subject: [PATCH 1/2] [X86] Fix commuteSelect miscompile with double-used
condition value
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.
---
llvm/lib/Target/X86/X86ISelLowering.cpp | 16 +----
.../CodeGen/X86/avx512-masked-op-fusion.ll | 67 +++++++++++++++++++
2 files changed, 69 insertions(+), 14 deletions(-)
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
+}
>From 681e3e84c61ce1e0cca5eb1eb8dc8cdbef85b2ff Mon Sep 17 00:00:00 2001
From: Timur Golubovich <timur.golubovich at intel.com>
Date: Tue, 8 Sep 2026 15:02:15 +0200
Subject: [PATCH 2/2] beautify the test
---
llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll b/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
index aa3e7d0167c80..70cce997b517c 100644
--- a/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
+++ b/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
@@ -57,11 +57,10 @@ exit:
; 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) {
+define <8 x i32> @commute_select_existing_inverse_cmp(<8 x i32> %src) nounwind {
; CHECK-LABEL: commute_select_existing_inverse_cmp:
-; CHECK: # %bb.0: # %entry
+; CHECK: # %bb.0:
; 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
@@ -78,9 +77,7 @@ define <8 x i32> @commute_select_existing_inverse_cmp(<8 x i32> %src) {
; 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>
@@ -99,7 +96,7 @@ entry:
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: # %bb.0:
; CHECK-NEXT: vpcmpnltd %zmm1, %zmm0, %k1
; CHECK-NEXT: vpcmpgtd %zmm0, %zmm1, %k0
; CHECK-NEXT: vpxor %xmm5, %xmm4, %xmm1
@@ -109,7 +106,6 @@ define <16 x i32> @commute_select_cond_used_as_value(<16 x i32> %a, <16 x i32> %
; 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
More information about the llvm-commits
mailing list