[llvm] [DAGCombiner] Only fold shift+mask compare to rotate if the shift amount divides the bit width (PR #220743)
Stanislav Bardyuk via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 11 02:39:26 PDT 2026
https://github.com/kodlan updated https://github.com/llvm/llvm-project/pull/220743
>From 1090ec96a9386610371639a664ead33a28ac81d9 Mon Sep 17 00:00:00 2001
From: Stanislav Bardyuk <sbardyuk at google.com>
Date: Wed, 2 Sep 2026 21:43:47 +0000
Subject: [PATCH 1/2] [DAGCombiner] Only fold shift+mask compare to rotate if
the shift amount divides the bit width
visitSETCC can turn (X << S) == (X & -(1 << S)) (or the srl variant) into
rotl(X, S) == X, and the other way around, when the target prefers one
form. The two forms are only equivalent when S divides the bit width. The
check used isPowerOf2(S), which is the same thing for power-of-two widths
but not for e.g. i3 with S = 2, where x86 then produced a wrong compare.
Check NumBits % S == 0 instead and fix the comments that described the old
rule. No change for legal scalar types.
Fixes #220542
---
llvm/include/llvm/CodeGen/TargetLowering.h | 6 +-
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 8 +-
llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll | 74 +++++++++++++++++++
3 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index b2dcb38df0243..04b8582ff3dee 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -929,13 +929,13 @@ class LLVM_ABI TargetLoweringBase {
// Given:
// (icmp eq/ne (and X, C0), (shift X, C1))
// or
- // (icmp eq/ne X, (rotate X, CPow2))
+ // (icmp eq/ne X, (rotate X, C1))
// If C0 is a mask or shifted mask and the shift amt (C1) isolates the
// remaining bits (i.e something like `(x64 & UINT32_MAX) == (x64 >> 32)`)
// Do we prefer the shift to be shift-right, shift-left, or rotate.
- // Note: Its only valid to convert the rotate version to the shift version iff
- // the shift-amt (`C1`) is a power of 2 (including 0).
+ // Note: It's only valid to convert between the rotate and shift versions iff
+ // the shift-amt (`C1`) divides the bit width.
// If ShiftOpc (current Opcode) is returned, do nothing.
virtual unsigned preferedOpcodeForCmpEqPiecesOfOperand(
EVT VT, unsigned ShiftOpc, bool MayTransformRotate,
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index e08eaf1ba81d4..c2608b4006041 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -14980,7 +14980,7 @@ SDValue DAGCombiner::visitSETCC(SDNode *N) {
// If C0 is a mask or shifted mask and the shift amt (C1) isolates the
// remaining bits (i.e something like `(x64 & UINT32_MAX) == (x64 >> 32)`)
// Then:
- // If C1 is a power of 2, then the rotate and shift+and versions are
+ // If C1 divides the bit width, then the rotate and shift+and versions are
// equivilent, so we can interchange them depending on target preference.
// Otherwise, if we have the shift+and version we can interchange srl/shl
// which inturn affects the constant C0. We can use this to get better
@@ -15048,9 +15048,13 @@ SDValue DAGCombiner::visitSETCC(SDNode *N) {
ShiftOpc == ISD::SHL ? (~*AndCMask).isMask() : AndCMask->isMask();
}
+ // The rotate and shift+and forms are only equivalent if the shift
+ // amount divides the bit width.
+ bool MayTransformRotate =
+ !ShiftCAmt->isZero() && NumBits % ShiftCAmt->getZExtValue() == 0;
// See if target prefers another shift/rotate opcode.
unsigned NewShiftOpc = TLI.preferedOpcodeForCmpEqPiecesOfOperand(
- OpVT, ShiftOpc, ShiftCAmt->isPowerOf2(), *ShiftCAmt, AndCMask);
+ OpVT, ShiftOpc, MayTransformRotate, *ShiftCAmt, AndCMask);
// Transform is valid and we have a new preference.
if (CanTransform && NewShiftOpc != ShiftOpc) {
SDValue NewShiftOrRotate =
diff --git a/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll b/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll
index 227de9ad0ab69..590eb9e0f5f6e 100644
--- a/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll
+++ b/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll
@@ -1017,6 +1017,80 @@ define i32 @issue108722(i32 %0) {
ret i32 %4
}
+; The rotate form is only equivalent when the shift amount divides the bit
+; width; 2 does not divide 3, so these must keep the shift+mask form.
+define i1 @shl_to_rotl_eq_i3_s2_fail(i3 %x) {
+; CHECK-LABEL: shl_to_rotl_eq_i3_s2_fail:
+; CHECK: # %bb.0:
+; CHECK-NEXT: # kill: def $edi killed $edi def $rdi
+; CHECK-NEXT: leal (,%rdi,4), %eax
+; CHECK-NEXT: andb $4, %al
+; CHECK-NEXT: andb $4, %dil
+; CHECK-NEXT: cmpb %dil, %al
+; CHECK-NEXT: sete %al
+; CHECK-NEXT: retq
+ %shl = shl i3 %x, 2
+ %and = and i3 %x, -4
+ %r = icmp eq i3 %shl, %and
+ ret i1 %r
+}
+
+define i1 @shr_to_rotl_eq_i3_s2_fail(i3 %x) {
+; CHECK-LABEL: shr_to_rotl_eq_i3_s2_fail:
+; CHECK: # %bb.0:
+; CHECK-NEXT: movl %edi, %eax
+; CHECK-NEXT: andb $4, %al
+; CHECK-NEXT: shlb $2, %dil
+; CHECK-NEXT: andb $4, %dil
+; CHECK-NEXT: cmpb %dil, %al
+; CHECK-NEXT: sete %al
+; CHECK-NEXT: retq
+ %shr = lshr i3 %x, 2
+ %and = and i3 %x, 1
+ %r = icmp eq i3 %shr, %and
+ ret i1 %r
+}
+
+; 3 divides 6, so the rotate form is still fine on a non-power-of-two width.
+define i1 @shr_to_rotl_eq_i6_s3(i6 %x) {
+; CHECK-LABEL: shr_to_rotl_eq_i6_s3:
+; CHECK: # %bb.0:
+; CHECK-NEXT: movl %edi, %eax
+; CHECK-NEXT: andb $63, %al
+; CHECK-NEXT: shlb $3, %dil
+; CHECK-NEXT: movl %eax, %ecx
+; CHECK-NEXT: shrb $3, %cl
+; CHECK-NEXT: orb %dil, %cl
+; CHECK-NEXT: andb $63, %cl
+; CHECK-NEXT: cmpb %cl, %al
+; CHECK-NEXT: sete %al
+; CHECK-NEXT: retq
+ %shr = lshr i6 %x, 3
+ %and = and i6 %x, 7
+ %r = icmp eq i6 %shr, %and
+ ret i1 %r
+}
+
+define i64 @issue220542(i64 %0) {
+; CHECK-LABEL: issue220542:
+; CHECK: # %bb.0:
+; CHECK-NEXT: shrl $8, %edi
+; CHECK-NEXT: leal (,%rdi,4), %eax
+; CHECK-NEXT: andb $4, %al
+; CHECK-NEXT: andb $4, %dil
+; CHECK-NEXT: xorl %ecx, %ecx
+; CHECK-NEXT: cmpb %dil, %al
+; CHECK-NEXT: setne %cl
+; CHECK-NEXT: leaq 1(%rcx,%rcx), %rax
+; CHECK-NEXT: retq
+ %2 = lshr i64 %0, 8
+ %3 = trunc i64 %2 to i3
+ %4 = shl i3 %3, 2
+ %5 = and i3 %3, -4
+ %6 = icmp eq i3 %4, %5
+ %7 = select i1 %6, i64 1, i64 3
+ ret i64 %7
+}
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
; CHECK-AVX: {{.*}}
>From baf699189482716c6ddaab3ac1205f4cd0fb3def Mon Sep 17 00:00:00 2001
From: Stanislav Bardyuk <sbardyuk at google.com>
Date: Fri, 11 Sep 2026 09:39:04 +0000
Subject: [PATCH 2/2] [X86] Do not prefer the rotate form for illegal scalar
types
The rotate form of the shift+mask compare only pays off when the target
has a rotate for the type. On an illegal type such as i6 the rotl is
expanded back into shifts during promotion and the result is worse than
the shift+mask compare it replaced. Only prefer the rotate for legal
scalar types. Also fix a typo and rename the test to pr220542 as
requested in review.
---
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 2 +-
llvm/lib/Target/X86/X86ISelLowering.cpp | 5 +++--
llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll | 16 +++++++---------
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index c2608b4006041..dde70a856e2ab 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -14981,7 +14981,7 @@ SDValue DAGCombiner::visitSETCC(SDNode *N) {
// remaining bits (i.e something like `(x64 & UINT32_MAX) == (x64 >> 32)`)
// Then:
// If C1 divides the bit width, then the rotate and shift+and versions are
- // equivilent, so we can interchange them depending on target preference.
+ // equivalent, so we can interchange them depending on target preference.
// Otherwise, if we have the shift+and version we can interchange srl/shl
// which inturn affects the constant C0. We can use this to get better
// constants again determined by target preference.
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 5d45082a9e550..352906fb20c2d 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -3834,9 +3834,10 @@ unsigned X86TargetLowering::preferedOpcodeForCmpEqPiecesOfOperand(
// best. Otherwise its not clear what the best so just don't make changed.
PreferRotate = Subtarget.hasAVX512() && (VT.getScalarType() == MVT::i32 ||
VT.getScalarType() == MVT::i64);
- } else {
+ } else if (isTypeLegal(VT)) {
// For scalar, if we have bmi prefer rotate for rorx. Otherwise prefer
- // rotate unless we have a zext mask+shr.
+ // rotate unless we have a zext mask+shr. Rotates on illegal types are
+ // expanded to shifts, so never prefer them there.
PreferRotate = Subtarget.hasBMI2();
if (!PreferRotate) {
unsigned MaskBits =
diff --git a/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll b/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll
index 590eb9e0f5f6e..9e329648a6cf0 100644
--- a/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll
+++ b/llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll
@@ -1051,18 +1051,16 @@ define i1 @shr_to_rotl_eq_i3_s2_fail(i3 %x) {
ret i1 %r
}
-; 3 divides 6, so the rotate form is still fine on a non-power-of-two width.
+; 3 divides 6, so the rotate form would be valid here, but i6 has no legal
+; rotate (it would be expanded back to shifts), so keep the shift+mask form.
define i1 @shr_to_rotl_eq_i6_s3(i6 %x) {
; CHECK-LABEL: shr_to_rotl_eq_i6_s3:
; CHECK: # %bb.0:
; CHECK-NEXT: movl %edi, %eax
-; CHECK-NEXT: andb $63, %al
+; CHECK-NEXT: andb $56, %al
; CHECK-NEXT: shlb $3, %dil
-; CHECK-NEXT: movl %eax, %ecx
-; CHECK-NEXT: shrb $3, %cl
-; CHECK-NEXT: orb %dil, %cl
-; CHECK-NEXT: andb $63, %cl
-; CHECK-NEXT: cmpb %cl, %al
+; CHECK-NEXT: andb $56, %dil
+; CHECK-NEXT: cmpb %dil, %al
; CHECK-NEXT: sete %al
; CHECK-NEXT: retq
%shr = lshr i6 %x, 3
@@ -1071,8 +1069,8 @@ define i1 @shr_to_rotl_eq_i6_s3(i6 %x) {
ret i1 %r
}
-define i64 @issue220542(i64 %0) {
-; CHECK-LABEL: issue220542:
+define i64 @pr220542(i64 %0) {
+; CHECK-LABEL: pr220542:
; CHECK: # %bb.0:
; CHECK-NEXT: shrl $8, %edi
; CHECK-NEXT: leal (,%rdi,4), %eax
More information about the llvm-commits
mailing list