[llvm] r220612 - InstCombine: Remove overzealous asserts

[David Majnemer]

Author: majnemer
Date: Sat Oct 25 02:13:13 2014
New Revision: 220612

URL: http://llvm.org/viewvc/llvm-project?rev=220612&view=rev
Log:
InstCombine: Remove overzealous asserts

These asserts can trigger if the worklist iteration order is
sufficiently unlucky.  Instead of adding special case logic to handle
these edge conditions, just bail out on trying to transform them:
InstSimplify will get them when it reaches them on the worklist.

This fixes PR21378.

N.B.  No test case is included because any test would rely on the
fragile worklist iteration order.

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=220612&r1=220611&r2=220612&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Sat Oct 25 02:13:13 2014
@@ -1052,8 +1052,18 @@ Instruction *InstCombiner::FoldICmpCstSh
   APInt AP1 = CI1->getValue();
   APInt AP2 = CI2->getValue();
 
-  assert(AP2 != 0 && "Handled in InstSimplify");
-  assert(!AP2.isAllOnesValue() && "Handled in InstSimplify");
+  // Don't bother doing any work for cases which InstSimplify handles.
+  if (AP2 == 0)
+    return nullptr;
+  bool IsAShr = isa<AShrOperator>(Op);
+  if (IsAShr) {
+    if (AP2.isAllOnesValue())
+      return nullptr;
+    if (AP2.isNegative() != AP1.isNegative())
+      return nullptr;
+    if (AP2.sgt(AP1))
+      return nullptr;
+  }
 
   if (!AP1)
     // 'A' must be large enough to shift out the highest set bit.
@@ -1063,13 +1073,6 @@ Instruction *InstCombiner::FoldICmpCstSh
   if (AP1 == AP2)
     return getICmp(I.ICMP_EQ, A, ConstantInt::getNullValue(A->getType()));
 
-  bool IsAShr = isa<AShrOperator>(Op);
-  // If we are dealing with an arithmetic shift, both constants should agree in
-  // sign.  InstSimplify's SimplifyICmpInst range analysis is supposed to catch
-  // the cases when they disagree.
-  assert((!IsAShr || (AP1.isNegative() == AP2.isNegative() && AP1.sgt(AP2))) &&
-         "Handled in InstSimplify");
-
   // Get the distance between the highest bit that's set.
   int Shift;
   // Both the constants are negative, take their positive to calculate log.
@@ -1109,7 +1112,9 @@ Instruction *InstCombiner::FoldICmpCstSh
   APInt AP1 = CI1->getValue();
   APInt AP2 = CI2->getValue();
 
-  assert(AP2 != 0 && "Handled in InstSimplify");
+  // Don't bother doing any work for cases which InstSimplify handles.
+  if (AP2 == 0)
+    return nullptr;
 
   unsigned AP2TrailingZeros = AP2.countTrailingZeros();
 
@@ -2591,11 +2596,13 @@ Instruction *InstCombiner::visitICmpInst
       if (match(Op0, m_AShr(m_ConstantInt(CI2), m_Value(A))) ||
           match(Op0, m_LShr(m_ConstantInt(CI2), m_Value(A)))) {
         // (icmp eq/ne (ashr/lshr const2, A), const1)
-        return FoldICmpCstShrCst(I, Op0, A, CI, CI2);
+        if (Instruction *Inst = FoldICmpCstShrCst(I, Op0, A, CI, CI2))
+          return Inst;
       }
       if (match(Op0, m_Shl(m_ConstantInt(CI2), m_Value(A)))) {
         // (icmp eq/ne (shl const2, A), const1)
-        return FoldICmpCstShlCst(I, Op0, A, CI, CI2);
+        if (Instruction *Inst = FoldICmpCstShlCst(I, Op0, A, CI, CI2))
+          return Inst;
       }
     }