[InstCombine] Don't assume CmpInst has been visited in getFlippedStrictnessPredicateA...

Summary:
Removing an assumption (assert) that the CmpInst already has been
simplified in getFlippedStrictnessPredicateAndConstant. Solution is
to simply bail out instead of hitting the assertion. Instead we
assume that any profitable rewrite will happen in the next iteration
of InstCombine.

The reason why we can't assume that the CmpInst already has been
simplified is that the worklist does not guarantee such an ordering.

Solves https://bugs.llvm.org/show_bug.cgi?id=43376

Reviewers: spatel, lebedev.ri

Reviewed By: lebedev.ri

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@372972 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp
index b68266705e50822f3e0e89cf469d492f25bca576..ddc7de39d8d2aaae9f781685d06e42110ef065dd 100644 (file)
--- a/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -5141,16 +5141,11 @@ llvm::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate Pred,
     return WillIncrement ? !C->isMaxValue(IsSigned) : !C->isMinValue(IsSigned);
   };
 
-  // For scalars, SimplifyICmpInst should have already handled
-  // the edge cases for us, so we just assert on them.
-  // For vectors, we must handle the edge cases.
-  if (isa<ConstantInt>(C)) {
-    // A <= MAX -> TRUE ; A >= MIN -> TRUE
-    assert(ConstantIsOk(cast<ConstantInt>(C)));
+  if (auto *CI = dyn_cast<ConstantInt>(C)) {
+    // Bail out if the constant can't be safely incremented/decremented.
+    if (!ConstantIsOk(CI))
+      return llvm::None;
   } else if (Type->isVectorTy()) {
-    // TODO? If the edge cases for vectors were guaranteed to be handled as they
-    // are for scalar, we could remove the min/max checks. However, to do that,
-    // we would have to use insertelement/shufflevector to replace edge values.
     unsigned NumElts = Type->getVectorNumElements();
     for (unsigned i = 0; i != NumElts; ++i) {
       Constant *Elt = C->getAggregateElement(i);

diff --git a/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll b/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll
new file mode 100644 (file)
index 0000000..1e5bf51
--- /dev/null
+++ b/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -instcombine -S | FileCheck %s
+
+; We used to hit an assertion in getFlippedStrictnessPredicateAndConstant due
+; to assuming that edge cases such as %cmp (ult x, 0) already has been
+; simplified. But that depends on the worklist order, so that is not always
+; guaranteed.
+
+define i16 @d(i16* %d.a, i16* %d.b) {
+; CHECK-LABEL: @d(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[T0:%.*]] = load i16, i16* [[D_A:%.*]], align 1
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp eq i16 [[T0]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL]], label [[LAND_END:%.*]], label [[LAND_RHS:%.*]]
+; CHECK:       land.rhs:
+; CHECK-NEXT:    br label [[LAND_END]]
+; CHECK:       land.end:
+; CHECK-NEXT:    ret i16 -1
+;
+entry:
+  %t0 = load i16, i16* %d.a, align 1
+  %tobool = icmp ne i16 %t0, 0
+  br i1 %tobool, label %land.rhs, label %land.end
+
+land.rhs:
+  %t1 = load i16, i16* %d.b, align 1
+  %cmp = icmp ult i16 %t1, 0
+  br label %land.end
+
+land.end:
+  %t2 = phi i1 [ false, %entry ], [ %cmp, %land.rhs ]
+  %land.ext = zext i1 %t2 to i16
+  %mul = mul nsw i16 %land.ext, 3
+  %neg = xor i16 %mul, -1
+  ret i16 %neg
+}