[X86] Handle multi-use setcc in commuteSelect by timurgol007 · Pull Request #213645 · llvm/llvm-project

timurgol007 · 2026-08-03T11:11:35Z

When a setcc condition is shared between multiple vselects, commuteSelect previously bailed out due to the m_OneUse constraint. Extend it to check all users of the setcc and, if they all benefit from commuting, invert the condition once with ReplaceAllUsesOfValueWith and swap all vselect operands.

This fixes a write-mask fusion regression where masked min/max reductions produced unfused vminps + vmovaps {%k} instead of a single write-masked vminps {%k}.

timurgol007 · 2026-08-03T11:15:06Z

This is why the test is a regression test. Without the patch, the inner loop compiles to:

define void @masked_min_max(ptr %pSrc, ptr %pMsk, i64 %n, ptr %pMin, ptr %pMax) {
; CHECK-LABEL: masked_min_max:
; CHECK:       # %bb.0: # %entry
; CHECK-NEXT:    vbroadcastss {{.*#+}} zmm1 = [-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf]
; CHECK-NEXT:    vbroadcastss {{.*#+}} zmm0 = [+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf]
; CHECK-NEXT:    xorl %eax, %eax
; CHECK-NEXT:    .p2align 4
; CHECK-NEXT:  .LBB0_1: # %loop
; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
; CHECK-NEXT:    vmovaps %zmm1, %zmm2
; CHECK-NEXT:    vmovaps %zmm0, %zmm1
; CHECK-NEXT:    vmovdqu (%rsi,%rax), %xmm0
; CHECK-NEXT:    vptestnmb %xmm0, %xmm0, %k1
; CHECK-NEXT:    vmovups (%rdi,%rax,4), %zmm3
; CHECK-NEXT:    vminps %zmm3, %zmm1, %zmm0
; CHECK-NEXT:    vmovaps %zmm1, %zmm0 {%k1}
; CHECK-NEXT:    vmaxps %zmm3, %zmm2, %zmm1
; CHECK-NEXT:    vmovaps %zmm2, %zmm1 {%k1}
; CHECK-NEXT:    addq $16, %rax
; CHECK-NEXT:    cmpq %rdx, %rax
; CHECK-NEXT:    jb .LBB0_1
; CHECK-NEXT:  # %bb.2: # %exit
; CHECK-NEXT:    vmovaps %zmm0, (%rcx)
; CHECK-NEXT:    vmovaps %zmm1, (%r8)
; CHECK-NEXT:    vzeroupper
; CHECK-NEXT:    retq
entry:
  br label %loop

loop:
  %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]
  %acc_min = phi <16 x float> [ splat (float 0x7FF0000000000000), %entry ], [ %res_min, %loop ]
  %acc_max = phi <16 x float> [ splat (float 0xFFF0000000000000), %entry ], [ %res_max, %loop ]
  %msk_ptr = getelementptr inbounds i8, ptr %pMsk, i64 %iv
  %msk_bytes = load <16 x i8>, ptr %msk_ptr, align 1
  %cmp = icmp eq <16 x i8> %msk_bytes, zeroinitializer
  %src_ptr = getelementptr inbounds float, ptr %pSrc, i64 %iv
  %src = load <16 x float>, ptr %src_ptr, align 1
  %min = tail call <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float> %acc_min, <16 x float> %src, i32 4)
  %res_min = select <16 x i1> %cmp, <16 x float> %acc_min, <16 x float> %min
  %max = tail call <16 x float> @llvm.x86.avx512.max.ps.512(<16 x float> %acc_max, <16 x float> %src, i32 4)
  %res_max = select <16 x i1> %cmp, <16 x float> %acc_max, <16 x float> %max
  %iv.next = add nuw nsw i64 %iv, 16
  %done = icmp uge i64 %iv.next, %n
  br i1 %done, label %exit, label %loop

exit:
  store <16 x float> %res_min, ptr %pMin, align 64
  store <16 x float> %res_max, ptr %pMax, align 64
  ret void
}

declare <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float>, <16 x float>, i32)
declare <16 x float> @llvm.x86.avx512.max.ps.512(<16 x float>, <16 x float>, i32)

llvmorg-github-actions · 2026-08-03T11:33:38Z

@llvm/pr-subscribers-backend-x86

Author: Timur Golubovich (timurgol007)

Changes

When a setcc condition is shared between multiple vselects, commuteSelect previously bailed out due to the m_OneUse constraint. Extend it to check all users of the setcc and, if they all benefit from commuting, invert the condition once with ReplaceAllUsesOfValueWith and swap all vselect operands.

This fixes a write-mask fusion regression where masked min/max reductions produced unfused vminps + vmovaps {%k} instead of a single write-masked vminps {%k}.

Full diff: https://github.com/llvm/llvm-project/pull/213645.diff

2 Files Affected:

(modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+29-6)
(added) llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll (+57)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index dfda1157a720e..fc6dcce98d289 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -48564,21 +48564,44 @@ static SDValue commuteSelect(SDNode *N, SelectionDAG &DAG, const SDLoc &DL,
 
   ISD::CondCode CC;
   SDValue Cond, X, Y, LHS, RHS;
-  if (!sd_match(N, m_VSelect(m_AllOf(m_Value(Cond),
-                                     m_OneUse(m_SetCC(m_Value(X), m_Value(Y),
-                                                      m_CondCode(CC)))),
-                             m_Value(LHS), m_Value(RHS))))
+  if (!sd_match(
+          N, m_VSelect(m_AllOf(m_Value(Cond),
+                               m_SetCC(m_Value(X), m_Value(Y), m_CondCode(CC))),
+                       m_Value(LHS), m_Value(RHS))))
     return SDValue();
 
   if (canCombineAsMaskOperation(LHS, Subtarget) ||
       !canCombineAsMaskOperation(RHS, Subtarget))
     return SDValue();
 
+  // For multi-use setcc, check that all users are vselects that benefit.
+  if (!Cond.hasOneUse()) {
+    SDValue UserLHS, UserRHS;
+    for (SDNode *User : Cond->users()) {
+      if (!sd_match(User, m_VSelect(m_Specific(Cond), m_Value(UserLHS),
+                                    m_Value(UserRHS))))
+        return SDValue();
+      if (canCombineAsMaskOperation(UserLHS, Subtarget) ||
+          !canCombineAsMaskOperation(UserRHS, Subtarget))
+        return SDValue();
+    }
+  }
+
   // Commute LHS and RHS to create opportunity to select mask instruction.
   // (vselect M, L, R) -> (vselect ~M, R, L)
   ISD::CondCode NewCC = ISD::getSetCCInverse(CC, X.getValueType());
-  Cond = DAG.getSetCC(SDLoc(Cond), Cond.getValueType(), X, Y, NewCC);
-  return DAG.getSelect(DL, LHS.getValueType(), Cond, RHS, LHS);
+  SDValue NewCond = DAG.getSetCC(SDLoc(Cond), Cond.getValueType(), X, Y, NewCC);
+  if (Cond.hasOneUse())
+    return DAG.getSelect(DL, LHS.getValueType(), NewCond, RHS, LHS);
+
+  // Invert the setcc for all users and commute all vselects.
+  DAG.ReplaceAllUsesOfValueWith(Cond, NewCond);
+  for (SDNode *User : NewCond->users()) {
+    SDValue UserLHS = User->getOperand(1);
+    SDValue UserRHS = User->getOperand(2);
+    DAG.UpdateNodeOperands(User, NewCond, UserRHS, UserLHS);
+  }
+  return SDValue(N, 0);
 }
 
 /// 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
new file mode 100644
index 0000000000000..8bf8a83b90ee0
--- /dev/null
+++ b/llvm/test/CodeGen/X86/avx512-masked-op-fusion.ll
@@ -0,0 +1,57 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake-avx512 | FileCheck %s
+
+; Verify that commuteSelect handles multi-use setcc conditions shared between
+; min and max vselects. The setcc should be inverted once and both selects
+; commuted, enabling ISel to emit fused write-masked vminps/vmaxps {%k}.
+
+define void @masked_min_max(ptr %pSrc, ptr %pMsk, i64 %n, ptr %pMin, ptr %pMax) {
+; CHECK-LABEL: masked_min_max:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    vbroadcastss {{.*#+}} zmm0 = [-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf,-Inf]
+; CHECK-NEXT:    vbroadcastss {{.*#+}} zmm1 = [+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf,+Inf]
+; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    .p2align 4
+; CHECK-NEXT:  .LBB0_1: # %loop
+; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    vmovdqu (%rsi,%rax), %xmm2
+; CHECK-NEXT:    vptestmb %xmm2, %xmm2, %k1
+; CHECK-NEXT:    vmovups (%rdi,%rax,4), %zmm2
+; CHECK-NEXT:    vminps %zmm2, %zmm1, %zmm1 {%k1}
+; CHECK-NEXT:    vmaxps %zmm2, %zmm0, %zmm0 {%k1}
+; CHECK-NEXT:    addq $16, %rax
+; CHECK-NEXT:    cmpq %rdx, %rax
+; CHECK-NEXT:    jb .LBB0_1
+; CHECK-NEXT:  # %bb.2: # %exit
+; CHECK-NEXT:    vmovaps %zmm1, (%rcx)
+; CHECK-NEXT:    vmovaps %zmm0, (%r8)
+; CHECK-NEXT:    vzeroupper
+; CHECK-NEXT:    retq
+entry:
+  br label %loop
+
+loop:
+  %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]
+  %acc_min = phi <16 x float> [ splat (float 0x7FF0000000000000), %entry ], [ %res_min, %loop ]
+  %acc_max = phi <16 x float> [ splat (float 0xFFF0000000000000), %entry ], [ %res_max, %loop ]
+  %msk_ptr = getelementptr inbounds i8, ptr %pMsk, i64 %iv
+  %msk_bytes = load <16 x i8>, ptr %msk_ptr, align 1
+  %cmp = icmp eq <16 x i8> %msk_bytes, zeroinitializer
+  %src_ptr = getelementptr inbounds float, ptr %pSrc, i64 %iv
+  %src = load <16 x float>, ptr %src_ptr, align 1
+  %min = tail call <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float> %acc_min, <16 x float> %src, i32 4)
+  %res_min = select <16 x i1> %cmp, <16 x float> %acc_min, <16 x float> %min
+  %max = tail call <16 x float> @llvm.x86.avx512.max.ps.512(<16 x float> %acc_max, <16 x float> %src, i32 4)
+  %res_max = select <16 x i1> %cmp, <16 x float> %acc_max, <16 x float> %max
+  %iv.next = add nuw nsw i64 %iv, 16
+  %done = icmp uge i64 %iv.next, %n
+  br i1 %done, label %exit, label %loop
+
+exit:
+  store <16 x float> %res_min, ptr %pMin, align 64
+  store <16 x float> %res_max, ptr %pMax, align 64
+  ret void
+}
+
+declare <16 x float> @llvm.x86.avx512.min.ps.512(<16 x float>, <16 x float>, i32)
+declare <16 x float> @llvm.x86.avx512.max.ps.512(<16 x float>, <16 x float>, i32)

RKSimon · 2026-08-03T13:24:35Z

+  // For multi-use setcc, check that all users are vselects that benefit.
+  if (!Cond.hasOneUse()) {
+    SDValue UserLHS, UserRHS;
+    for (SDNode *User : Cond->users()) {


use an llvm::all_of pattern ?

@RKSimon, good catch, addressed. I also created a PR with the precommit test so the diff shows the codegen improvement more clearly.

timurgol007 · 2026-08-03T13:26:59Z

This is a pr that adds precommit test: #213669

RKSimon · 2026-08-03T18:04:35Z

@@ -0,0 +1,57 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake-avx512 | FileCheck %s


Suggested change

; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake-avx512 | FileCheck %s

; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=x86-64-v4 | FileCheck %s

RKSimon

ok to rebase once the test lands

…213669) Precommit the test for #213645

When a setcc condition is shared between multiple vselects, commuteSelect previously bailed out due to the m_OneUse constraint. Extend it to check all users of the setcc and, if they all benefit from commuting, invert the condition once with ReplaceAllUsesOfValueWith and swap all vselect operands. This fixes a write-mask fusion regression where masked min/max reductions produced unfused vminps + vmovaps {%k} instead of a single write-masked vminps {%k}.

phoebewang · 2026-08-04T13:28:58Z

    return SDValue();

+  // For multi-use setcc, check that all users are vselects that benefit.
+  if (!Cond.hasOneUse()) {


Nit: Save to a variable to avoid call it twice.

RKSimon

LGTM

…lect

DKLoehr · 2026-08-05T18:02:16Z

We've started to see clang crashes in chromium after this commit. I've pasted the output below, and attached a (very large) crash reproducer. I'm working on minimizing it, but it will take a while since the file is very large.
repro.zip

clang++: /usr/local/google/home/dloehr/src/chromium/src/third_party/llvm/llvm/include/llvm/CodeGen/SelectionDAGNodes.h:1056: const SDValue &llvm::SDNode::getOperand(unsigned int) const: Assertion `Num < NumOperands && "Invalid child # of SDNode!"' failed.

1.      <eof> parser at end of file
2.      Code generation
3.      Running pass 'Function Pass Manager' on module 'shift_test-f1d458.cpp'.
4.      Running pass 'X86 DAG->DAG Instruction Selection' on function '@_ZN3hwy9N_AVX10_212_GLOBAL__N_123TestVariableRoundingShrclIiNS0_4SimdIiLm8ELi0EEEEEvT_T0_'
#0 0x000055d0822aa01b llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (../third_party/llvm-build/Release+Asserts/bin/clang+++0x542d01b)
./shift_test-f1d458.sh: line 2: 2458906 Aborted                    (core dumped)

timurgol007 · 2026-08-05T19:56:14Z

@DKLoehr, thanks for reporting this. I managed to reduce it via llvm-reduce to the following reproducer:

llc -mtriple=x86_64-unknown-linux-gnu -mcpu=x86-64 -mattr=+sse3 -O2 reduced.ll -o /dev/null

target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define fastcc <8 x i32> @test(<8 x i32> %vecinit.i.i.i) #0 {
entry:
  %0 = icmp eq <8 x i32> %vecinit.i.i.i, <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
  %1 = icmp ne <8 x i32> %vecinit.i.i.i, <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
  %elt.min.i249.neg = sext <8 x i1> %1 to <8 x i32>
  %2 = select <8 x i1> %0, <8 x i32> zeroinitializer, <8 x i32> %elt.min.i249.neg
  %sub.i.i.i.i = sub <8 x i32> zeroinitializer, %2
  %3 = bitcast <8 x i32> %sub.i.i.i.i to <4 x i64>
  tail call void null(<4 x i64> zeroinitializer, <4 x i64> %3, ptr null, i32 0)
  %4 = select <8 x i1> %0, <8 x i32> zeroinitializer, <8 x i32> %vecinit.i.i.i
  %sub.i.i.i.i209 = sub <8 x i32> zeroinitializer, %4
  ret <8 x i32> %sub.i.i.i.i209
}

attributes #0 = { "target-features"="+avx10.1" }

This was already fixed by #214092. I had incorrectly assumed the CSE scenario couldn't happen in practice - clearly it can. Sorry for the breakage, and thanks again for the report.

phoebewang · 2026-08-06T03:31:36Z

Maybe add it as a regression test?

mikolaj-pirog requested review from RKSimon and phoebewang August 3, 2026 11:19

llvmorg-github-actions Bot added the backend:X86 label Aug 3, 2026

RKSimon reviewed Aug 3, 2026

View reviewed changes

e-kud mentioned this pull request Aug 3, 2026

[X86] Add test showing missed write-mask fusion for multi-use setcc #213669

Merged

timurgol007 requested a review from RKSimon August 3, 2026 15:50

RKSimon reviewed Aug 3, 2026

View reviewed changes

timurgol007 requested a review from RKSimon August 3, 2026 19:06

RKSimon reviewed Aug 4, 2026

View reviewed changes

e-kud pushed a commit that referenced this pull request Aug 4, 2026

[X86] Add test showing missed write-mask fusion for multi-use setcc (#…

57b06e6

…213669) Precommit the test for #213645

timurgol007 force-pushed the users/timurgol007/fix-multiuse-in-commute-select branch from 5ba2e97 to c4087bd Compare August 4, 2026 12:04

timurgol007 requested a review from RKSimon August 4, 2026 12:05

added all_of algorithm

d452906

timurgol007 force-pushed the users/timurgol007/fix-multiuse-in-commute-select branch from c4087bd to d452906 Compare August 4, 2026 12:14

phoebewang approved these changes Aug 4, 2026

View reviewed changes

reused a variable

2fd3d84

RKSimon approved these changes Aug 4, 2026

View reviewed changes

timurgol007 added 3 commits August 4, 2026 16:33

Merge branch 'main' into users/timurgol007/fix-multiuse-in-commute-se…

2817a4b

…lect

Merge branch 'main' into users/timurgol007/fix-multiuse-in-commute-se…

5acb35c

…lect

Merge branch 'main' into users/timurgol007/fix-multiuse-in-commute-se…

166a621

…lect

mikolaj-pirog merged commit 2178db6 into llvm:main Aug 4, 2026
12 checks passed

Navigation Menu

Uh oh!

[X86] Handle multi-use setcc in commuteSelect - #213645

[X86] Handle multi-use setcc in commuteSelect#213645
mikolaj-pirog merged 6 commits into
llvm:mainfrom
timurgol007:users/timurgol007/fix-multiuse-in-commute-select

timurgol007 commented Aug 3, 2026

Uh oh!

timurgol007 commented Aug 3, 2026

Uh oh!

llvmorg-github-actions Bot commented Aug 3, 2026

Uh oh!

RKSimon Aug 3, 2026

Uh oh!

timurgol007 Aug 3, 2026

Uh oh!

timurgol007 commented Aug 3, 2026

Uh oh!

RKSimon Aug 3, 2026

Uh oh!

RKSimon left a comment

Uh oh!

phoebewang Aug 4, 2026

Uh oh!

RKSimon left a comment

Uh oh!

Uh oh!

DKLoehr commented Aug 5, 2026 •

edited

Loading

Uh oh!

timurgol007 commented Aug 5, 2026 •

edited

Loading

Uh oh!

phoebewang commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -0,0 +1,57 @@
		; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
		; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=skylake-avx512 \| FileCheck %s

Uh oh!

Conversation

timurgol007 commented Aug 3, 2026

Uh oh!

timurgol007 commented Aug 3, 2026

Uh oh!

llvmorg-github-actions Bot commented Aug 3, 2026

Uh oh!

RKSimon Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

timurgol007 Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

timurgol007 commented Aug 3, 2026

Uh oh!

RKSimon Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

phoebewang Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DKLoehr commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timurgol007 commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phoebewang commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DKLoehr commented Aug 5, 2026 •

edited

Loading

timurgol007 commented Aug 5, 2026 •

edited

Loading