[llvm] r372972 - [InstCombine] Don't assume CmpInst has been visited in getFlippedStrictnessPredicateAndConstant

[Bjorn Pettersson via llvm-commits]

Author: bjope
Date: Thu Sep 26 05:16:01 2019
New Revision: 372972

URL: http://llvm.org/viewvc/llvm-project?rev=372972&view=rev
Log:
[InstCombine] Don't assume CmpInst has been visited in getFlippedStrictnessPredicateAndConstant

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

Added:
    llvm/trunk/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll
Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=372972&r1=372971&r2=372972&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Thu Sep 26 05:16:01 2019
@@ -5141,16 +5141,11 @@ llvm::getFlippedStrictnessPredicateAndCo
     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);

Added: llvm/trunk/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll?rev=372972&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/pr43376-getFlippedStrictnessPredicateAndConstant-assert.ll Thu Sep 26 05:16:01 2019
@@ -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
+}