[DAGCombiner] Transform `(icmp eq/ne (and X,C0),(shift X,C1))` to use rotate or to getter constants.

[goldstein.w.n]

Summary

If C0 is a mask and C1 shifts out all the masked bits (to
essentially compare two subsets of X), we can arbitrarily re-order
shift as srl or shl.

If C1 (shift amount) is a power of 2, we can replace the and+shift
with a rotate.

Otherwise, based on target preference we can arbitrarily swap shl
and shl in/out to get better constants.

On x86 we can use this re-ordering to:

1. get better and constants for C0 (zero extended moves or avoid imm64).
2. covert srl to shl if shl will be implementable with lea or add (both of which can be preferable).

Proofs: https://alive2.llvm.org/ce/z/qzGM_w

[RKSimon]

This looks similar to optimizeSetCCByHoistingAndByConstFromLogicalShift/shouldProduceAndByConstByHoistingConstFromShiftsLHSOfAnd - can we extend that and perform this as a generic combine?

[goldstein.w.n]

In D152116#4399143, @RKSimon wrote:

This looks similar to optimizeSetCCByHoistingAndByConstFromLogicalShift/shouldProduceAndByConstByHoistingConstFromShiftsLHSOfAnd - can we extend that and perform this as a generic combine?

Didn't see those earlier.
So optimizeSetCCByHoistingAndByConstFromLogicalShift seems meaningfully different. Just very different node pattern for X & (C shift Y) eq/ne 0 vs (X shift C0) eq/ne (X and C1). Then likewise shouldProduceAndByConstByHoistingConstFromShiftsLHSOfAnd seems built for optimizeSetCCByHoistingAndByConstFromLogicalShift.
I see the similarity and think could probably could implement both in `optimizeSetCCByHoistingAndByConstFromLogicalShift but think it would end up pretty awkward.
WDYT?

[RKSimon]

I suppose I'm asking was whether this patch is x86 specific or not - it's always nice not to have something in X86ISelLowering.cpp if other targets would benefit

[goldstein.w.n]

In D152116#4400494, @RKSimon wrote:

I suppose I'm asking was whether this patch is x86 specific or not - it's always nice not to have something in X86ISelLowering.cpp if other targets would benefit

Ah I see. Happy to move to DAGCombiner and put this behind something like TargetLowing::PreferInvertShiftAndMask.

[goldstein.w.n]

Move to DAG Combiner and make preferences target specific.
Also handle rotates

[goldstein.w.n]

ping.

[RKSimon]

llvm/include/llvm/CodeGen/TargetLowering.h

818 ↗

(On Diff #529198)

This function name seems very general and yet the transform seems very specific.

819 ↗

(On Diff #529198)

Maybe EVT /*VT*/ style would be better than all the (void) ?

[goldstein.w.n]

llvm/include/llvm/CodeGen/TargetLowering.h

818 ↗

(On Diff #529198)

Changed to "PiecesOfOperand" hopefully thats clearer. Otherwise, what would you suggest. "OperandPieces" is mean to refer to the fact that this for comparisons where we are comparing a part of X with another part of X (i.e (X & 255) == (X >> 8)` is comparing that the top 8bits (one piece) equals the low 8bits (another piece).

819 ↗

(On Diff #529198)

What is EVT /*VT*/ style?

[goldstein.w.n]

Rebase + rename helper

[RKSimon]

llvm/include/llvm/CodeGen/TargetLowering.h

827 ↗

(On Diff #535062)

Remove the (void)s

[goldstein.w.n]

Don't void variables

[goldstein.w.n]

Removed (void)s

[goldstein.w.n]

ping.

[RKSimon]

llvm/lib/Target/X86/X86ISelLowering.cpp

6192

AndMask.has_value() ?

6210

(style) remove the else - the if case always returns:

// If the current setup has imm64 mask, then inverse will have
// at least imm32 mask (or be zext i32 -> i64).
if (VT == MVT::i64)
  return AndMask->getSignificantBits() > 32 ? ISD::SRL : ShiftOpc;

// We can only benefit if req at least 7-bit for the mask. We
// don't want to replace shl of 1,2,3 as they can be implemented
// with lea/add.
return ShiftOrRotateAmt.uge(7) ? ISD::SRL : ShiftOpc;

6230

Please can you rephrase this? Its rather confusing.

[goldstein.w.n]

Cleanup

[goldstein.w.n]

Rebase

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12409 ↗

(On Diff #557125)

SimplifySetCC(VT, N0, N1, Cond, ....

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12428 ↗

(On Diff #557125)

Isn't this the same as Cond at line#12405?

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12496 ↗

(On Diff #557125)

(~*AndCMask).popcount() - is this the same as !AndCMask->isAllOnes() ?

[goldstein.w.n]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12496 ↗

(On Diff #557125)

No, we are checking for something like:
(icmp eq (and X, 0x1f), (srl, X, 3). Not all ones.

[goldstein.w.n]

Don't re-declare things

[goldstein.w.n]

Remember to commit...

[RKSimon]

LGTM with one minor

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

12496 ↗

(On Diff #557125)

Test ShiftOpc == ISD::SHL before the popcount as its cheaper?

llvm/lib/Target/X86/X86ISelLowering.cpp

             SDValue SETCC0 = DAG.getSetCC(DL, VT, BaseOp, RHS, CC);
             SDValue SETCC1 = DAG.getSetCC(
                 DL, VT, BaseOp, DAG.getConstant(-CInt, DL, OpVT), CC);
             return DAG.getNode(CC == ISD::SETEQ ? ISD::OR : ISD::AND, DL, VT,
                                SETCC0, SETCC1);
           }
         }
       }
+
+      // Reorder: icmp eq/ne (and X, C0), (shift X, C1)
+      // If X is a mask or shifted mask and the shift isolates the remaining
+      // bits (i.e something like `(x64 & UINT32_MAX) == (x64 >> 32)`) re-order
+      // to get better mask constants. Better mask constants basically means:
+      //    1) Avoid imm64
+      //    2) Try to get shl for low C1 (so we can lower then lea/add)
+      //    3) Try to zero-extend mov masks.
+      if (OpVT == MVT::i64 || OpVT == MVT::i32 || OpVT == MVT::i16 ||
+          OpVT == MVT::i8) {
+        auto IsAndWithShift = [](SDValue A, SDValue B) {
+          return A.getOpcode() == ISD::AND &&
+                 (B.getOpcode() == ISD::SRL || B.getOpcode() == ISD::SHL) &&
+                 A.getOperand(0) == B.getOperand(0);
+        };
+        SDValue And = SDValue(), Shift = SDValue();
+        if (IsAndWithShift(LHS, RHS)) {
+          And = LHS;
+          Shift = RHS;
+        } else if (IsAndWithShift(RHS, LHS)) {
+          And = RHS;
+          Shift = LHS;
+        }
+        if (And && Shift && And.hasOneUse() && Shift.hasOneUse()) {
+          auto *AndCMask = dyn_cast<ConstantSDNode>(And.getOperand(1));
+          auto *ShiftCAmt = dyn_cast<ConstantSDNode>(Shift.getOperand(1));
+          if (AndCMask && ShiftCAmt) {
+            const APInt &Mask = AndCMask->getAPIntValue();
+            const APInt &Amt = ShiftCAmt->getAPIntValue();
+            bool DoTransform = Amt == (~Mask).popcount();
+            unsigned NewShiftOpc;
+            APInt NewMask;
+            if (Shift.getOpcode() == ISD::SHL) {
+              DoTransform &= (~Mask).isMask();
+
+              if (OpVT == MVT::i64)
+                // If the current setup has imm64 mask, then inverse will have
+                // at least imm32 mask (or be zext i32 -> i64).
+                DoTransform &= Mask.getSignificantBits() > 32;
+              else
+                // We can only benefit if req at least 7-bit for the mask. We
+                // don't want to replace shl of 1,2,3 as they can be implemented
+                // with lea/add.
+                DoTransform &= Amt.uge(7);
+              if (DoTransform) {
+                NewShiftOpc = ISD::SRL;
+                NewMask = Mask.lshr(Amt);
+              }
+            } else {
+              DoTransform &= Mask.isMask();
+              if (OpVT == MVT::i64)
+                // Keep exactly 32-bit imm64, this is zext i32 -> i64 which is
+                // extremely efficient.
+                DoTransform &= Mask.getSignificantBits() > 33;
+              else
+                // Keep small shifts as shl so we can generate add/lea.
+                DoTransform &= Amt.ult(7);
+
+              if (DoTransform) {
+                NewShiftOpc = ISD::SHL;
+                NewMask = Mask.shl(Amt);
+              }
+            }
+            if (DoTransform) {
+              SDValue NewShift =
+                  DAG.getNode(NewShiftOpc, DL, OpVT, Shift.getOperand(0),
+                              Shift.getOperand(1));
+              SDValue NewAnd =
+                  DAG.getNode(ISD::AND, DL, OpVT, And.getOperand(0),
+                              DAG.getConstant(NewMask, DL, OpVT));
+              return DAG.getSetCC(DL, VT, NewAnd, NewShift, CC);
+            }
+          }
+        }
+      }
     }
   }
 
   if (VT.isVector() && VT.getVectorElementType() == MVT::i1 &&
       (CC == ISD::SETNE || CC == ISD::SETEQ || ISD::isSignedIntSetCC(CC))) {
     // Using temporaries to avoid messing up operand ordering for later
     // transformations if this doesn't work.
     SDValue Op0 = LHS;

llvm/test/CodeGen/X86/cmp-shiftX-maskX.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
 ; RUN: llc < %s -mtriple=i686-unknown-unknown | FileCheck %s --check-prefixes=X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s --check-prefixes=X64
 
 define i1 @shr_to_shl_eq_i8_s2(i8 %x) {
 ; X86-LABEL: shr_to_shl_eq_i8_s2:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movzbl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    andb $63, %cl
-; X86-NEXT:    shrb $2, %al
-; X86-NEXT:    cmpb %al, %cl
+; X86-NEXT:    shlb $2, %cl
+; X86-NEXT:    andb $-4, %al
+; X86-NEXT:    cmpb %cl, %al
 ; X86-NEXT:    sete %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shr_to_shl_eq_i8_s2:
 ; X64:       # %bb.0:
-; X64-NEXT:    movl %edi, %eax
-; X64-NEXT:    andb $63, %al
-; X64-NEXT:    shrb $2, %dil
-; X64-NEXT:    cmpb %dil, %al
+; X64-NEXT:    # kill: def $edi killed $edi def $rdi
+; X64-NEXT:    leal (,%rdi,4), %eax
+; X64-NEXT:    andb $-4, %dil
+; X64-NEXT:    cmpb %al, %dil
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %and = and i8 %x, 63
   %shr = lshr i8 %x, 2
   %r = icmp eq i8 %and, %shr
   ret i1 %r
 }
 
 define i1 @shl_to_shr_ne_i8_s7(i8 %x) {
 ; X86-LABEL: shl_to_shr_ne_i8_s7:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movzbl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    shlb $7, %cl
-; X86-NEXT:    andb $-128, %al
-; X86-NEXT:    cmpb %al, %cl
+; X86-NEXT:    shrb $7, %cl
+; X86-NEXT:    andb $1, %al
+; X86-NEXT:    cmpb %cl, %al
 ; X86-NEXT:    setne %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shl_to_shr_ne_i8_s7:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movl %edi, %eax
-; X64-NEXT:    shlb $7, %al
-; X64-NEXT:    andb $-128, %dil
-; X64-NEXT:    cmpb %dil, %al
+; X64-NEXT:    shrb $7, %al
+; X64-NEXT:    andb $1, %dil
+; X64-NEXT:    cmpb %al, %dil
 ; X64-NEXT:    setne %al
 ; X64-NEXT:    retq
   %shl = shl i8 %x, 7
   %and = and i8 %x, 128
   %r = icmp ne i8 %shl, %and
   ret i1 %r
 }
 
 define i1 @shr_to_shl_eq_i8_s1(i8 %x) {
 ; X86-LABEL: shr_to_shl_eq_i8_s1:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movzbl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    andb $127, %cl
-; X86-NEXT:    shrb %al
-; X86-NEXT:    cmpb %al, %cl
+; X86-NEXT:    addb %al, %cl
+; X86-NEXT:    andb $-2, %al
+; X86-NEXT:    cmpb %cl, %al
 ; X86-NEXT:    sete %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shr_to_shl_eq_i8_s1:
 ; X64:       # %bb.0:
-; X64-NEXT:    movl %edi, %eax
-; X64-NEXT:    andb $127, %al
-; X64-NEXT:    shrb %dil
-; X64-NEXT:    cmpb %dil, %al
+; X64-NEXT:    # kill: def $edi killed $edi def $rdi
+; X64-NEXT:    leal (%rdi,%rdi), %eax
+; X64-NEXT:    andb $-2, %dil
+; X64-NEXT:    cmpb %al, %dil
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %and = and i8 %x, 127
   %shr = lshr i8 %x, 1
   %r = icmp eq i8 %and, %shr
   ret i1 %r
 }
 
 define i1 @shr_to_shl_eq_i32_s3(i32 %x) {
 ; X86-LABEL: shr_to_shl_eq_i32_s3:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    andl $536870911, %ecx # imm = 0x1FFFFFFF
-; X86-NEXT:    shrl $3, %eax
-; X86-NEXT:    cmpl %eax, %ecx
+; X86-NEXT:    leal (,%eax,8), %ecx
+; X86-NEXT:    andl $-8, %eax
+; X86-NEXT:    cmpl %ecx, %eax
 ; X86-NEXT:    sete %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shr_to_shl_eq_i32_s3:
 ; X64:       # %bb.0:
-; X64-NEXT:    movl %edi, %eax
-; X64-NEXT:    andl $536870911, %eax # imm = 0x1FFFFFFF
-; X64-NEXT:    shrl $3, %edi
-; X64-NEXT:    cmpl %edi, %eax
+; X64-NEXT:    # kill: def $edi killed $edi def $rdi
+; X64-NEXT:    leal (,%rdi,8), %eax
+; X64-NEXT:    andl $-8, %edi
+; X64-NEXT:    cmpl %eax, %edi
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %and = and i32 %x, 536870911
   %shr = lshr i32 %x, 3
   %r = icmp eq i32 %and, %shr
   ret i1 %r
 }
 
; X64-NEXT:    retq
   %r = icmp eq i32 %and, %shr
   ret i1 %r
 }
 
 define i1 @shl_to_shr_ne_i32_s16(i32 %x) {
 ; X86-LABEL: shl_to_shr_ne_i32_s16:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    shll $16, %ecx
-; X86-NEXT:    andl $-65536, %eax # imm = 0xFFFF0000
+; X86-NEXT:    movzwl %ax, %ecx
+; X86-NEXT:    shrl $16, %eax
 ; X86-NEXT:    cmpl %eax, %ecx
 ; X86-NEXT:    setne %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shl_to_shr_ne_i32_s16:
 ; X64:       # %bb.0:
-; X64-NEXT:    movl %edi, %eax
-; X64-NEXT:    shll $16, %eax
-; X64-NEXT:    andl $-65536, %edi # imm = 0xFFFF0000
+; X64-NEXT:    movzwl %di, %eax
+; X64-NEXT:    shrl $16, %edi
 ; X64-NEXT:    cmpl %edi, %eax
 ; X64-NEXT:    setne %al
 ; X64-NEXT:    retq
   %shl = shl i32 %x, 16
   %and = and i32 %x, 4294901760
   %r = icmp ne i32 %shl, %and
   ret i1 %r
 }
; X64-NEXT:    retq
   %r = icmp ne i32 %shl, %and
   ret i1 %r
 }
 
 define i1 @shr_to_shl_eq_i16_s1(i16 %x) {
 ; X86-LABEL: shr_to_shl_eq_i16_s1:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movzwl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    andl $32767, %ecx # imm = 0x7FFF
-; X86-NEXT:    shrl %eax
-; X86-NEXT:    cmpw %ax, %cx
+; X86-NEXT:    leal (%eax,%eax), %ecx
+; X86-NEXT:    andl $-2, %eax
+; X86-NEXT:    cmpw %cx, %ax
 ; X86-NEXT:    sete %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shr_to_shl_eq_i16_s1:
 ; X64:       # %bb.0:
-; X64-NEXT:    movzwl %di, %eax
-; X64-NEXT:    andl $32767, %edi # imm = 0x7FFF
-; X64-NEXT:    shrl %eax
+; X64-NEXT:    # kill: def $edi killed $edi def $rdi
+; X64-NEXT:    leal (%rdi,%rdi), %eax
+; X64-NEXT:    andl $65534, %edi # imm = 0xFFFE
 ; X64-NEXT:    cmpw %ax, %di
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %and = and i16 %x, 32767
   %shr = lshr i16 %x, 1
   %r = icmp eq i16 %and, %shr
   ret i1 %r
 }
; X64-NEXT:    retq
   %r = icmp eq i16 %and, %shr
   ret i1 %r
 }
 
 define i1 @shl_to_shr_eq_i64_s44(i64 %x) {
 ; X86-LABEL: shl_to_shr_eq_i64_s44:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    shll $12, %eax
-; X86-NEXT:    movl $-4096, %ecx # imm = 0xF000
+; X86-NEXT:    shrl $12, %eax
+; X86-NEXT:    movl $1048575, %ecx # imm = 0xFFFFF
 ; X86-NEXT:    andl {{[0-9]+}}(%esp), %ecx
 ; X86-NEXT:    xorl %eax, %ecx
 ; X86-NEXT:    sete %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shl_to_shr_eq_i64_s44:
 ; X64:       # %bb.0:
-; X64-NEXT:    movabsq $-17592186044416, %rax # imm = 0xFFFFF00000000000
-; X64-NEXT:    andq %rdi, %rax
-; X64-NEXT:    shlq $44, %rdi
+; X64-NEXT:    movq %rdi, %rax
+; X64-NEXT:    shrq $44, %rax
+; X64-NEXT:    andl $1048575, %edi # imm = 0xFFFFF
 ; X64-NEXT:    cmpq %rax, %rdi
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %shl = shl i64 %x, 44
   %and = and i64 %x, 18446726481523507200
   %r = icmp eq i64 %shl, %and
   ret i1 %r
 }
; X64-NEXT:    retq
   %r = icmp ne i64 %and, %shr
   ret i1 %r
 }
 
 define i1 @shl_to_shr_eq_i64_s63(i64 %x) {
 ; X86-LABEL: shl_to_shr_eq_i64_s63:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    shll $31, %eax
-; X86-NEXT:    movl $-2147483648, %ecx # imm = 0x80000000
-; X86-NEXT:    andl {{[0-9]+}}(%esp), %ecx
-; X86-NEXT:    xorl %eax, %ecx
+; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:    shrl $31, %ecx
+; X86-NEXT:    andl $1, %eax
+; X86-NEXT:    xorl %ecx, %eax
 ; X86-NEXT:    sete %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shl_to_shr_eq_i64_s63:
 ; X64:       # %bb.0:
-; X64-NEXT:    movabsq $-9223372036854775808, %rax # imm = 0x8000000000000000
-; X64-NEXT:    andq %rdi, %rax
-; X64-NEXT:    shlq $63, %rdi
+; X64-NEXT:    movq %rdi, %rax
+; X64-NEXT:    shrq $63, %rax
+; X64-NEXT:    andl $1, %edi
 ; X64-NEXT:    cmpq %rax, %rdi
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %shl = shl i64 %x, 63
   %and = and i64 %x, 9223372036854775808
   %r = icmp eq i64 %shl, %and
   ret i1 %r
 }
; X64-NEXT:    retq
   %and = and i64 %x, 9223372036854775808
   %r = icmp ugt i64 %shl, %and
   ret i1 %r
 }
 
 define i1 @shr_to_shl_eq_i64_s7(i64 %x) {
 ; X86-LABEL: shr_to_shl_eq_i64_s7:
 ; X86:       # %bb.0:
-; X86-NEXT:    pushl %esi
-; X86-NEXT:    .cfi_def_cfa_offset 8
-; X86-NEXT:    .cfi_offset %esi, -8
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
 ; X86-NEXT:    movl %ecx, %edx
-; X86-NEXT:    andl $33554431, %edx # imm = 0x1FFFFFF
-; X86-NEXT:    movl %ecx, %esi
-; X86-NEXT:    shldl $25, %eax, %esi
-; X86-NEXT:    shrl $7, %ecx
-; X86-NEXT:    xorl %edx, %ecx
-; X86-NEXT:    xorl %eax, %esi
-; X86-NEXT:    orl %ecx, %esi
+; X86-NEXT:    shldl $7, %eax, %edx
+; X86-NEXT:    xorl %ecx, %edx
+; X86-NEXT:    movl %eax, %ecx
+; X86-NEXT:    shll $7, %ecx
+; X86-NEXT:    andl $-128, %eax
+; X86-NEXT:    xorl %ecx, %eax
+; X86-NEXT:    orl %edx, %eax
 ; X86-NEXT:    sete %al
-; X86-NEXT:    popl %esi
-; X86-NEXT:    .cfi_def_cfa_offset 4
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shr_to_shl_eq_i64_s7:
 ; X64:       # %bb.0:
-; X64-NEXT:    movabsq $144115188075855871, %rax # imm = 0x1FFFFFFFFFFFFFF
-; X64-NEXT:    andq %rdi, %rax
-; X64-NEXT:    shrq $7, %rdi
-; X64-NEXT:    cmpq %rdi, %rax
+; X64-NEXT:    movq %rdi, %rax
+; X64-NEXT:    shlq $7, %rax
+; X64-NEXT:    andq $-128, %rdi
+; X64-NEXT:    cmpq %rax, %rdi
 ; X64-NEXT:    sete %al
 ; X64-NEXT:    retq
   %and = and i64 %x, 144115188075855871
   %shr = lshr i64 %x, 7
   %r = icmp eq i64 %and, %shr
   ret i1 %r
 }
 
 define i1 @shl_to_shr_ne_i32_s24(i32 %x) {
 ; X86-LABEL: shl_to_shr_ne_i32_s24:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    movl %eax, %ecx
-; X86-NEXT:    shll $24, %ecx
-; X86-NEXT:    andl $-16777216, %eax # imm = 0xFF000000
+; X86-NEXT:    movzbl %al, %ecx
+; X86-NEXT:    shrl $24, %eax
 ; X86-NEXT:    cmpl %eax, %ecx
 ; X86-NEXT:    setne %al
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: shl_to_shr_ne_i32_s24:
 ; X64:       # %bb.0:
-; X64-NEXT:    movl %edi, %eax
-; X64-NEXT:    shll $24, %eax
-; X64-NEXT:    andl $-16777216, %edi # imm = 0xFF000000
+; X64-NEXT:    movzbl %dil, %eax
+; X64-NEXT:    shrl $24, %edi
 ; X64-NEXT:    cmpl %edi, %eax
 ; X64-NEXT:    setne %al
 ; X64-NEXT:    retq
   %shl = shl i32 %x, 24
   %and = and i32 %x, 4278190080
   %r = icmp ne i32 %shl, %and
   ret i1 %r
 }