[llvm] r341517 - Revert "[IndVars] Turn isValidRewrite into an assertion" because it seems wrong

[Max Kazantsev via llvm-commits]

Author: mkazantsev
Date: Wed Sep  5 22:52:47 2018
New Revision: 341517

URL: http://llvm.org/viewvc/llvm-project?rev=341517&view=rev
Log:
Revert "[IndVars] Turn isValidRewrite into an assertion" because it seems wrong

Modified:
    llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp?rev=341517&r1=341516&r2=341517&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Wed Sep  5 22:52:47 2018
@@ -136,9 +136,7 @@ class IndVarSimplify {
   SmallVector<WeakTrackingVH, 16> DeadInsts;
   bool Changed = false;
 
-#ifndef NDEBUG
   bool isValidRewrite(Value *FromVal, Value *ToVal);
-#endif
 
   void handleFloatingPointIV(Loop *L, PHINode *PH);
   void rewriteNonIntegerIVs(Loop *L);
@@ -165,17 +163,18 @@ public:
 
 } // end anonymous namespace
 
-#ifndef NDEBUG
 /// Return true if the SCEV expansion generated by the rewriter can replace the
 /// original value. SCEV guarantees that it produces the same value, but the way
-/// it is produced may be illegal IR.
+/// it is produced may be illegal IR.  Ideally, this function will only be
+/// called for verification.
 bool IndVarSimplify::isValidRewrite(Value *FromVal, Value *ToVal) {
   // If an SCEV expression subsumed multiple pointers, its expansion could
   // reassociate the GEP changing the base pointer. This is illegal because the
   // final address produced by a GEP chain must be inbounds relative to its
   // underlying object. Otherwise basic alias analysis, among other things,
-  // could fail in a dangerous way. Specifically, we want to make sure that SCEV
-  // does not convert gep Base, (&p[n] - &p[0]) into gep &p[n], Base - &p[0].
+  // could fail in a dangerous way. Ultimately, SCEV will be improved to avoid
+  // producing an expression involving multiple pointers. Until then, we must
+  // bail out here.
   //
   // Retrieve the pointer operand of the GEP. Don't use GetUnderlyingObject
   // because it understands lcssa phis while SCEV does not.
@@ -215,7 +214,6 @@ bool IndVarSimplify::isValidRewrite(Valu
   }
   return true;
 }
-#endif
 
 /// Determine the insertion point for this user. By default, insert immediately
 /// before the user. SCEVExpander or LICM will hoist loop invariants out of the
@@ -642,7 +640,10 @@ void IndVarSimplify::rewriteLoopExitValu
                           << '\n'
                           << "  LoopVal = " << *Inst << "\n");
 
-        assert(isValidRewrite(Inst, ExitVal) && "Must be!");
+        if (!isValidRewrite(Inst, ExitVal)) {
+          DeadInsts.push_back(ExitVal);
+          continue;
+        }
 
 #ifndef NDEBUG
         // If we reuse an instruction from a loop which is neither L nor one of