[InstCombine] invert canonicalization for cast of signbit test

rotateright · rotateright · commit 3794cc0e9964 · 2022-05-16T12:55:52.000-04:00
The existing 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 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.
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -987,21 +987,14 @@ Instruction *InstCombinerImpl::transformZExtICmp(ICmpInst *Cmp, ZExtInst &Zext)
   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);
       In = Builder.CreateLShr(In, Sh, In->getName() + ".lobit");
       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/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -1158,6 +1158,10 @@ Instruction *InstCombinerImpl::visitLShr(BinaryOperator &I) {
     }
 
     if (ShAmtC == BitWidth - 1) {
+      // lshr i32 (~X), 31 --> zext (X > -1)
+      if (match(Op0, m_OneUse(m_Not(m_Value(X)))))
+        return new ZExtInst(Builder.CreateIsNotNeg(X), Ty);
+
       // lshr i32 or(X,-X), 31 --> zext (X != 0)
       if (match(Op0, m_OneUse(m_c_Or(m_Neg(m_Value(X)), m_Deferred(X)))))
         return new ZExtInst(Builder.CreateIsNotNull(X), Ty);
diff --git a/llvm/test/Transforms/InstCombine/and-xor-merge.ll b/llvm/test/Transforms/InstCombine/and-xor-merge.ll
@@ -28,9 +28,9 @@ define i32 @test2(i32 %x, i32 %y, i32 %z) {
 
 define i32 @PR38781(i32 %a, i32 %b) {
 ; CHECK-LABEL: @PR38781(
-; CHECK-NEXT:    [[B_LOBIT_NOT1_DEMORGAN:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
-; CHECK-NEXT:    [[B_LOBIT_NOT1:%.*]] = xor i32 [[B_LOBIT_NOT1_DEMORGAN]], -1
-; CHECK-NEXT:    [[AND:%.*]] = lshr i32 [[B_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]]
 ;
   %a.lobit = lshr i32 %a, 31
diff --git 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
@@ -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/xor.ll b/llvm/test/Transforms/InstCombine/xor.ll
@@ -1059,8 +1059,8 @@ define i8 @not_lshr(i8 %x) {
 
 define <2 x i8> @not_lshr_vec(<2 x i8> %x) {
 ; CHECK-LABEL: @not_lshr_vec(
-; CHECK-NEXT:    [[TMP1:%.*]] = xor <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
-; CHECK-NEXT:    [[R:%.*]] = lshr <2 x i8> [[TMP1]], <i8 7, i8 7>
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp sgt <2 x i8> [[X:%.*]], <i8 -1, i8 -1>
+; CHECK-NEXT:    [[R:%.*]] = zext <2 x i1> [[TMP1]] to <2 x i8>
 ; CHECK-NEXT:    ret <2 x i8> [[R]]
 ;
   %a = lshr <2 x i8> %x, <i8 7, i8 7>
diff --git 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,10 +489,8 @@ 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)
diff --git 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:
