[llvm] f31d39c - [InstCombine] remove cast-of-signbit to shift transform

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Tue May 17 08:12:02 PDT 2022


Author: Sanjay Patel
Date: 2022-05-17T11:10:28-04:00
New Revision: f31d39c42c0e17ff9b9f65cae797ba5d66e78ec7

URL: https://github.com/llvm/llvm-project/commit/f31d39c42c0e17ff9b9f65cae797ba5d66e78ec7
DIFF: https://github.com/llvm/llvm-project/commit/f31d39c42c0e17ff9b9f65cae797ba5d66e78ec7.diff

LOG: [InstCombine] remove cast-of-signbit to shift transform

The transform was wrong in 3 ways:

1. It created an extra instruction when the source and dest types don't match.
2. It did not account for an extra use of the icmp, so could create 2 extra insts.
3. It favored bit hacks over icmp (icmp generally has better analysis).

This fixes #54692 (modeled by the PhaseOrdering tests).

This is a minimal step to fix the bug, but we should likely invert
this and the sibling transform for the "is negative" pattern too.

The backend should be able to invert this back to a shift if that
leads to better codegen.

This is a reduced try of 3794cc0e9964 - that was reverted because
it could cause infinite loops by conflicting with the related
transforms in this block that create shifts.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
    llvm/test/Transforms/InstCombine/compare-signs.ll
    llvm/test/Transforms/InstCombine/icmp.ll
    llvm/test/Transforms/InstCombine/zext.ll
    llvm/test/Transforms/PhaseOrdering/cmp-logic.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index 2418ee29f2ee0..764c8df2313d0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -983,13 +983,18 @@ Instruction *InstCombinerImpl::transformZExtICmp(ICmpInst *Cmp, ZExtInst &Zext)
   // If we are just checking for a icmp eq of a single bit and zext'ing it
   // to an integer, then shift the bit to the appropriate place and then
   // cast to integer to avoid the comparison.
+
+  // FIXME: This set of transforms does not check for extra uses and/or creates
+  //        an extra instruction (an optional final cast is not included
+  //        in the transform comments). We may also want to favor icmp over
+  //        shifts in cases of equal instructions because icmp has better
+  //        analysis in general (invert the transform).
+
   const APInt *Op1CV;
   if (match(Cmp->getOperand(1), m_APInt(Op1CV))) {
 
     // zext (x <s  0) to i32 --> x>>u31      true if signbit set.
-    // zext (x >s -1) to i32 --> (x>>u31)^1  true if signbit clear.
-    if ((Cmp->getPredicate() == ICmpInst::ICMP_SLT && Op1CV->isZero()) ||
-        (Cmp->getPredicate() == ICmpInst::ICMP_SGT && Op1CV->isAllOnes())) {
+    if (Cmp->getPredicate() == ICmpInst::ICMP_SLT && Op1CV->isZero()) {
       Value *In = Cmp->getOperand(0);
       Value *Sh = ConstantInt::get(In->getType(),
                                    In->getType()->getScalarSizeInBits() - 1);
@@ -997,11 +1002,6 @@ Instruction *InstCombinerImpl::transformZExtICmp(ICmpInst *Cmp, ZExtInst &Zext)
       if (In->getType() != Zext.getType())
         In = Builder.CreateIntCast(In, Zext.getType(), false /*ZExt*/);
 
-      if (Cmp->getPredicate() == ICmpInst::ICMP_SGT) {
-        Constant *One = ConstantInt::get(In->getType(), 1);
-        In = Builder.CreateXor(In, One, In->getName() + ".not");
-      }
-
       return replaceInstUsesWith(Zext, In);
     }
 

diff  --git a/llvm/test/Transforms/InstCombine/compare-signs.ll b/llvm/test/Transforms/InstCombine/compare-signs.ll
index 7b1e187562417..620a508b2b51e 100644
--- a/llvm/test/Transforms/InstCombine/compare-signs.ll
+++ b/llvm/test/Transforms/InstCombine/compare-signs.ll
@@ -6,9 +6,9 @@
 define i32 @test1(i32 %a, i32 %b) nounwind readnone {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[B:%.*]], [[A:%.*]]
-; CHECK-NEXT:    [[TMP2:%.*]] = xor i32 [[TMP1]], -1
-; CHECK-NEXT:    [[DOTLOBIT_NOT:%.*]] = lshr i32 [[TMP2]], 31
-; CHECK-NEXT:    ret i32 [[DOTLOBIT_NOT]]
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp sgt i32 [[TMP1]], -1
+; CHECK-NEXT:    [[T3:%.*]] = zext i1 [[TMP2]] to i32
+; CHECK-NEXT:    ret i32 [[T3]]
 ;
   %t0 = icmp sgt i32 %a, -1
   %t1 = icmp slt i32 %b, 0
@@ -36,9 +36,9 @@ define i32 @test2(i32 %a, i32 %b) nounwind readnone {
 define i32 @test3(i32 %a, i32 %b) nounwind readnone {
 ; CHECK-LABEL: @test3(
 ; CHECK-NEXT:    [[T2_UNSHIFTED:%.*]] = xor i32 [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[T2_UNSHIFTED]], -1
-; CHECK-NEXT:    [[T2_UNSHIFTED_LOBIT_NOT:%.*]] = lshr i32 [[TMP1]], 31
-; CHECK-NEXT:    ret i32 [[T2_UNSHIFTED_LOBIT_NOT]]
+; CHECK-NEXT:    [[T2:%.*]] = icmp sgt i32 [[T2_UNSHIFTED]], -1
+; CHECK-NEXT:    [[T3:%.*]] = zext i1 [[T2]] to i32
+; CHECK-NEXT:    ret i32 [[T3]]
 ;
   %t0 = lshr i32 %a, 31
   %t1 = lshr i32 %b, 31
@@ -68,9 +68,9 @@ define <2 x i32> @test3vec(<2 x i32> %a, <2 x i32> %b) nounwind readnone {
 define i32 @test3i(i32 %a, i32 %b) nounwind readnone {
 ; CHECK-LABEL: @test3i(
 ; CHECK-NEXT:    [[T01:%.*]] = xor i32 [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[T01]], -1
-; CHECK-NEXT:    [[T01_LOBIT_NOT:%.*]] = lshr i32 [[TMP1]], 31
-; CHECK-NEXT:    ret i32 [[T01_LOBIT_NOT]]
+; CHECK-NEXT:    [[T4:%.*]] = icmp sgt i32 [[T01]], -1
+; CHECK-NEXT:    [[T5:%.*]] = zext i1 [[T4]] to i32
+; CHECK-NEXT:    ret i32 [[T5]]
 ;
   %t0 = lshr i32 %a, 29
   %t1 = lshr i32 %b, 29

diff  --git a/llvm/test/Transforms/InstCombine/icmp.ll b/llvm/test/Transforms/InstCombine/icmp.ll
index b8dc9fba3195f..284e5280cba39 100644
--- a/llvm/test/Transforms/InstCombine/icmp.ll
+++ b/llvm/test/Transforms/InstCombine/icmp.ll
@@ -27,9 +27,9 @@ define <2 x i32> @test1vec(<2 x i32> %X) {
 
 define i32 @test2(i32 %X) {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:    [[TMP1:%.*]] = xor i32 [[X:%.*]], -1
-; CHECK-NEXT:    [[X_LOBIT_NOT:%.*]] = lshr i32 [[TMP1]], 31
-; CHECK-NEXT:    ret i32 [[X_LOBIT_NOT]]
+; CHECK-NEXT:    [[A:%.*]] = icmp sgt i32 [[X:%.*]], -1
+; CHECK-NEXT:    [[B:%.*]] = zext i1 [[A]] to i32
+; CHECK-NEXT:    ret i32 [[B]]
 ;
   %a = icmp ult i32 %X, -2147483648
   %b = zext i1 %a to i32
@@ -38,9 +38,9 @@ define i32 @test2(i32 %X) {
 
 define <2 x i32> @test2vec(<2 x i32> %X) {
 ; CHECK-LABEL: @test2vec(
-; CHECK-NEXT:    [[TMP1:%.*]] = xor <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
-; CHECK-NEXT:    [[X_LOBIT_NOT:%.*]] = lshr <2 x i32> [[TMP1]], <i32 31, i32 31>
-; CHECK-NEXT:    ret <2 x i32> [[X_LOBIT_NOT]]
+; CHECK-NEXT:    [[A:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
+; CHECK-NEXT:    [[B:%.*]] = zext <2 x i1> [[A]] to <2 x i32>
+; CHECK-NEXT:    ret <2 x i32> [[B]]
 ;
   %a = icmp ult <2 x i32> %X, <i32 -2147483648, i32 -2147483648>
   %b = zext <2 x i1> %a to <2 x i32>

diff  --git a/llvm/test/Transforms/InstCombine/zext.ll b/llvm/test/Transforms/InstCombine/zext.ll
index d189799ca8906..32b5fe07d908a 100644
--- a/llvm/test/Transforms/InstCombine/zext.ll
+++ b/llvm/test/Transforms/InstCombine/zext.ll
@@ -452,10 +452,9 @@ define i32 @zext_or_masked_bit_test_uses(i32 %a, i32 %b, i32 %x) {
 
 define i32 @notneg_zext_wider(i8 %x) {
 ; CHECK-LABEL: @notneg_zext_wider(
-; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X:%.*]], -1
-; CHECK-NEXT:    [[TMP2:%.*]] = lshr i8 [[TMP1]], 7
-; CHECK-NEXT:    [[DOTNOT:%.*]] = zext i8 [[TMP2]] to i32
-; CHECK-NEXT:    ret i32 [[DOTNOT]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
+; CHECK-NEXT:    [[R:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT:    ret i32 [[R]]
 ;
   %cmp = icmp sgt i8 %x, -1
   %r = zext i1 %cmp to i32
@@ -464,10 +463,9 @@ define i32 @notneg_zext_wider(i8 %x) {
 
 define <2 x i8> @notneg_zext_narrower(<2 x i32> %x) {
 ; CHECK-LABEL: @notneg_zext_narrower(
-; CHECK-NEXT:    [[X_LOBIT:%.*]] = lshr <2 x i32> [[X:%.*]], <i32 31, i32 31>
-; CHECK-NEXT:    [[TMP1:%.*]] = trunc <2 x i32> [[X_LOBIT]] to <2 x i8>
-; CHECK-NEXT:    [[DOTNOT:%.*]] = xor <2 x i8> [[TMP1]], <i8 1, i8 1>
-; CHECK-NEXT:    ret <2 x i8> [[DOTNOT]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[X:%.*]], <i32 -1, i32 -1>
+; CHECK-NEXT:    [[R:%.*]] = zext <2 x i1> [[CMP]] to <2 x i8>
+; CHECK-NEXT:    ret <2 x i8> [[R]]
 ;
   %cmp = icmp sgt <2 x i32> %x, <i32 -1, i32 -1>
   %r = zext <2 x i1> %cmp to <2 x i8>
@@ -478,10 +476,8 @@ define i32 @notneg_zext_wider_use(i8 %x) {
 ; CHECK-LABEL: @notneg_zext_wider_use(
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i8 [[X:%.*]], -1
 ; CHECK-NEXT:    call void @use1(i1 [[CMP]])
-; CHECK-NEXT:    [[TMP1:%.*]] = xor i8 [[X]], -1
-; CHECK-NEXT:    [[TMP2:%.*]] = lshr i8 [[TMP1]], 7
-; CHECK-NEXT:    [[DOTNOT:%.*]] = zext i8 [[TMP2]] to i32
-; CHECK-NEXT:    ret i32 [[DOTNOT]]
+; CHECK-NEXT:    [[R:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT:    ret i32 [[R]]
 ;
   %cmp = icmp sgt i8 %x, -1
   call void @use1(i1 %cmp)
@@ -493,13 +489,24 @@ define i8 @notneg_zext_narrower_use(i32 %x) {
 ; CHECK-LABEL: @notneg_zext_narrower_use(
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt i32 [[X:%.*]], -1
 ; CHECK-NEXT:    call void @use1(i1 [[CMP]])
-; CHECK-NEXT:    [[X_LOBIT:%.*]] = lshr i32 [[X]], 31
-; CHECK-NEXT:    [[TMP1:%.*]] = trunc i32 [[X_LOBIT]] to i8
-; CHECK-NEXT:    [[DOTNOT:%.*]] = xor i8 [[TMP1]], 1
-; CHECK-NEXT:    ret i8 [[DOTNOT]]
+; CHECK-NEXT:    [[R:%.*]] = zext i1 [[CMP]] to i8
+; CHECK-NEXT:    ret i8 [[R]]
 ;
   %cmp = icmp sgt i32 %x, -1
   call void @use1(i1 %cmp)
   %r = zext i1 %cmp to i8
   ret i8 %r
 }
+
+define i8 @disguised_signbit_clear_test(i64 %x) {
+; CHECK-LABEL: @disguised_signbit_clear_test(
+; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[X:%.*]] to i8
+; CHECK-NEXT:    [[TMP2:%.*]] = xor i8 [[TMP1]], -1
+; CHECK-NEXT:    [[TMP3:%.*]] = lshr i8 [[TMP2]], 7
+; CHECK-NEXT:    ret i8 [[TMP3]]
+;
+  %a1 = and i64 %x, 128
+  %t4 = icmp eq i64 %a1, 0
+  %t6 = zext i1 %t4 to i8
+  ret i8 %t6
+}

diff  --git a/llvm/test/Transforms/PhaseOrdering/cmp-logic.ll b/llvm/test/Transforms/PhaseOrdering/cmp-logic.ll
index 3ffa28ff2a828..c9532408a2f93 100644
--- a/llvm/test/Transforms/PhaseOrdering/cmp-logic.ll
+++ b/llvm/test/Transforms/PhaseOrdering/cmp-logic.ll
@@ -3,9 +3,9 @@
 
 define i32 @PR38781(i32 noundef %a, i32 noundef %b) {
 ; CHECK-LABEL: @PR38781(
-; CHECK-NEXT:    [[A_LOBIT_NOT1_DEMORGAN:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
-; CHECK-NEXT:    [[A_LOBIT_NOT1:%.*]] = xor i32 [[A_LOBIT_NOT1_DEMORGAN]], -1
-; CHECK-NEXT:    [[AND:%.*]] = lshr i32 [[A_LOBIT_NOT1]], 31
+; CHECK-NEXT:    [[TMP1:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
+; CHECK-NEXT:    [[TMP2:%.*]] = icmp sgt i32 [[TMP1]], -1
+; CHECK-NEXT:    [[AND:%.*]] = zext i1 [[TMP2]] to i32
 ; CHECK-NEXT:    ret i32 [[AND]]
 ;
   %cmp = icmp sge i32 %a, 0
@@ -48,17 +48,10 @@ land.end:
 define i1 @PR54692_b(i8 noundef signext %c) {
 ; CHECK-LABEL: @PR54692_b(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = xor i8 [[C:%.*]], -1
-; CHECK-NEXT:    [[TMP1:%.*]] = lshr i8 [[TMP0]], 7
-; CHECK-NEXT:    [[DOTNOT:%.*]] = zext i8 [[TMP1]] to i32
-; CHECK-NEXT:    [[CMP3:%.*]] = icmp slt i8 [[C]], 32
-; CHECK-NEXT:    [[CONV4:%.*]] = zext i1 [[CMP3]] to i32
-; CHECK-NEXT:    [[AND:%.*]] = and i32 [[DOTNOT]], [[CONV4]]
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp ult i8 [[C:%.*]], 32
 ; CHECK-NEXT:    [[CMP6:%.*]] = icmp eq i8 [[C]], 127
-; CHECK-NEXT:    [[CONV7:%.*]] = zext i1 [[CMP6]] to i32
-; CHECK-NEXT:    [[OR:%.*]] = or i32 [[AND]], [[CONV7]]
-; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp ne i32 [[OR]], 0
-; CHECK-NEXT:    ret i1 [[TOBOOL]]
+; CHECK-NEXT:    [[OR2:%.*]] = or i1 [[TMP0]], [[CMP6]]
+; CHECK-NEXT:    ret i1 [[OR2]]
 ;
 entry:
   %conv = sext i8 %c to i32
@@ -79,15 +72,9 @@ entry:
 define i1 @PR54692_c(i8 noundef signext %c) {
 ; CHECK-LABEL: @PR54692_c(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = xor i8 [[C:%.*]], -1
-; CHECK-NEXT:    [[TMP1:%.*]] = lshr i8 [[TMP0]], 7
-; CHECK-NEXT:    [[DOTNOT:%.*]] = zext i8 [[TMP1]] to i32
-; CHECK-NEXT:    [[CMP3:%.*]] = icmp slt i8 [[C]], 32
-; CHECK-NEXT:    [[CONV4:%.*]] = zext i1 [[CMP3]] to i32
-; CHECK-NEXT:    [[AND:%.*]] = and i32 [[DOTNOT]], [[CONV4]]
-; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp ne i32 [[AND]], 0
+; CHECK-NEXT:    [[TMP0:%.*]] = icmp ult i8 [[C:%.*]], 32
 ; CHECK-NEXT:    [[CMP6:%.*]] = icmp eq i8 [[C]], 127
-; CHECK-NEXT:    [[T0:%.*]] = or i1 [[CMP6]], [[TOBOOL]]
+; CHECK-NEXT:    [[T0:%.*]] = or i1 [[TMP0]], [[CMP6]]
 ; CHECK-NEXT:    ret i1 [[T0]]
 ;
 entry:


        


More information about the llvm-commits mailing list