[InstCombine] Fix InstCombinerImpl::foldICmpMulConstant for nsw and nuw mul with...

If we have both an nsw and nuw flag, we would see the nsw flag
first and only handle signed comparisons.

This patch ignores the nsw flag if the comparison isn't signed.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D143766

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index c8010b7..20622da 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -2061,35 +2061,37 @@ Instruction *InstCombinerImpl::foldICmpMulConstant(ICmpInst &Cmp,
     }
   }
 
-  if (!Mul->hasNoSignedWrap() && !Mul->hasNoUnsignedWrap())
-    return nullptr;
-
   // With a matching no-overflow guarantee, fold the constants:
   // (X * MulC) < C --> X < (C / MulC)
   // (X * MulC) > C --> X > (C / MulC)
   // TODO: Assert that Pred is not equal to SGE, SLE, UGE, ULE?
   Constant *NewC = nullptr;
-  if (Mul->hasNoSignedWrap()) {
+  if (Mul->hasNoSignedWrap() && ICmpInst::isSigned(Pred)) {
     // MININT / -1 --> overflow.
     if (C.isMinSignedValue() && MulC->isAllOnes())
       return nullptr;
     if (MulC->isNegative())
       Pred = ICmpInst::getSwappedPredicate(Pred);
 
-    if (Pred == ICmpInst::ICMP_SLT || Pred == ICmpInst::ICMP_SGE)
+    if (Pred == ICmpInst::ICMP_SLT || Pred == ICmpInst::ICMP_SGE) {
       NewC = ConstantInt::get(
           MulTy, APIntOps::RoundingSDiv(C, *MulC, APInt::Rounding::UP));
-    if (Pred == ICmpInst::ICMP_SLE || Pred == ICmpInst::ICMP_SGT)
+    } else {
+      assert((Pred == ICmpInst::ICMP_SLE || Pred == ICmpInst::ICMP_SGT) &&
+             "Unexpected predicate");
       NewC = ConstantInt::get(
           MulTy, APIntOps::RoundingSDiv(C, *MulC, APInt::Rounding::DOWN));
-  } else {
-    assert(Mul->hasNoUnsignedWrap() && "Expected mul nuw");
-    if (Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_UGE)
+    }
+  } else if (Mul->hasNoUnsignedWrap() && ICmpInst::isUnsigned(Pred)) {
+    if (Pred == ICmpInst::ICMP_ULT || Pred == ICmpInst::ICMP_UGE) {
       NewC = ConstantInt::get(
           MulTy, APIntOps::RoundingUDiv(C, *MulC, APInt::Rounding::UP));
-    if (Pred == ICmpInst::ICMP_ULE || Pred == ICmpInst::ICMP_UGT)
+    } else {
+      assert((Pred == ICmpInst::ICMP_ULE || Pred == ICmpInst::ICMP_UGT) &&
+             "Unexpected predicate");
       NewC = ConstantInt::get(
           MulTy, APIntOps::RoundingUDiv(C, *MulC, APInt::Rounding::DOWN));
+    }
   }
 
   return NewC ? new ICmpInst(Pred, X, NewC) : nullptr;

diff --git a/llvm/test/Transforms/InstCombine/icmp-mul.ll b/llvm/test/Transforms/InstCombine/icmp-mul.ll
index 13e21b5..4268bbb 100644 (file)
--- a/llvm/test/Transforms/InstCombine/icmp-mul.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-mul.ll
@@ -134,11 +134,10 @@ define i1 @ult_rem_zero(i8 %x) {
 }
 
 ; Same as above, but with nsw flag too.
-; FIXME: This should be optimized like above.
+; This used to not optimize due to nsw being prioritized too much.
 define i1 @ult_rem_zero_nsw(i8 %x) {
 ; CHECK-LABEL: @ult_rem_zero_nsw(
-; CHECK-NEXT:    [[A:%.*]] = mul nuw nsw i8 [[X:%.*]], 7
-; CHECK-NEXT:    [[B:%.*]] = icmp ult i8 [[A]], 21
+; CHECK-NEXT:    [[B:%.*]] = icmp ult i8 [[X:%.*]], 3
 ; CHECK-NEXT:    ret i1 [[B]]
 ;
   %a = mul nuw nsw i8 %x, 7
@@ -157,11 +156,10 @@ define i1 @ult_rem_nz(i8 %x) {
 }
 
 ; Same as above, but with nsw flag too.
-; FIXME: This should be optimized like above.
+; This used to not optimize due to nsw being prioritized too much.
 define i1 @ult_rem_nz_nsw(i8 %x) {
 ; CHECK-LABEL: @ult_rem_nz_nsw(
-; CHECK-NEXT:    [[A:%.*]] = mul nuw nsw i8 [[X:%.*]], 5
-; CHECK-NEXT:    [[B:%.*]] = icmp ult i8 [[A]], 21
+; CHECK-NEXT:    [[B:%.*]] = icmp ult i8 [[X:%.*]], 5
 ; CHECK-NEXT:    ret i1 [[B]]
 ;
   %a = mul nuw nsw i8 %x, 5
@@ -212,11 +210,10 @@ define i1 @ugt_rem_zero(i8 %x) {
 }
 
 ; Same as above, but with nsw flag too.
-; FIXME: This should be optimized like above.
+; This used to not optimize due to nsw being prioritized too much.
 define i1 @ugt_rem_zero_nsw(i8 %x) {
 ; CHECK-LABEL: @ugt_rem_zero_nsw(
-; CHECK-NEXT:    [[A:%.*]] = mul nuw nsw i8 [[X:%.*]], 7
-; CHECK-NEXT:    [[B:%.*]] = icmp ugt i8 [[A]], 21
+; CHECK-NEXT:    [[B:%.*]] = icmp ugt i8 [[X:%.*]], 3
 ; CHECK-NEXT:    ret i1 [[B]]
 ;
   %a = mul nuw nsw i8 %x, 7
@@ -235,11 +232,10 @@ define i1 @ugt_rem_nz(i8 %x) {
 }
 
 ; Same as above, but with nsw flag too.
-; FIXME: This should be optimized like above.
+; This used to not optimize due to nsw being prioritized too much.
 define i1 @ugt_rem_nz_nsw(i8 %x) {
 ; CHECK-LABEL: @ugt_rem_nz_nsw(
-; CHECK-NEXT:    [[A:%.*]] = mul nuw nsw i8 [[X:%.*]], 5
-; CHECK-NEXT:    [[B:%.*]] = icmp ugt i8 [[A]], 21
+; CHECK-NEXT:    [[B:%.*]] = icmp ugt i8 [[X:%.*]], 4
 ; CHECK-NEXT:    ret i1 [[B]]
 ;
   %a = mul nuw nsw i8 %x, 5
@@ -998,9 +994,7 @@ define i1 @splat_mul_known_lz(i32 %x) {
 
 define i1 @splat_mul_unknown_lz(i32 %x) {
 ; CHECK-LABEL: @splat_mul_unknown_lz(
-; CHECK-NEXT:    [[Z:%.*]] = zext i32 [[X:%.*]] to i128
-; CHECK-NEXT:    [[M:%.*]] = mul nuw nsw i128 [[Z]], 18446744078004518913
-; CHECK-NEXT:    [[R:%.*]] = icmp ult i128 [[M]], 39614081257132168796771975168
+; CHECK-NEXT:    [[R:%.*]] = icmp sgt i32 [[X:%.*]], -1
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %z = zext i32 %x to i128