[llvm] r341516 - [IndVars] Turn isValidRewrite into an assertion

[Max Kazantsev via llvm-commits]

Author: mkazantsev
Date: Wed Sep  5 22:21:25 2018
New Revision: 341516

URL: http://llvm.org/viewvc/llvm-project?rev=341516&view=rev
Log:
[IndVars] Turn isValidRewrite into an assertion

Function rewriteLoopExitValues contains a check on isValidRewrite which
is needed to make sure that SCEV does not convert the pattern
`gep Base, (&p[n] - &p[0])` into `gep &p[n], Base - &p[0]`. This problem
has been fixed in SCEV long ago, so this check is just obsolete.

This patch converts it into an assertion to make sure that the SCEV will
not mess up this case in the future.

Differential Revision: https://reviews.llvm.org/D51582
Reviewed By: atrick

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=341516&r1=341515&r2=341516&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Wed Sep  5 22:21:25 2018
@@ -136,7 +136,9 @@ 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);
@@ -163,18 +165,17 @@ 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.  Ideally, this function will only be
-/// called for verification.
+/// it is produced may be illegal IR.
 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. Ultimately, SCEV will be improved to avoid
-  // producing an expression involving multiple pointers. Until then, we must
-  // bail out here.
+  // 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].
   //
   // Retrieve the pointer operand of the GEP. Don't use GetUnderlyingObject
   // because it understands lcssa phis while SCEV does not.
@@ -214,6 +215,7 @@ 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
@@ -640,10 +642,7 @@ void IndVarSimplify::rewriteLoopExitValu
                           << '\n'
                           << "  LoopVal = " << *Inst << "\n");
 
-        if (!isValidRewrite(Inst, ExitVal)) {
-          DeadInsts.push_back(ExitVal);
-          continue;
-        }
+        assert(isValidRewrite(Inst, ExitVal) && "Must be!");
 
 #ifndef NDEBUG
         // If we reuse an instruction from a loop which is neither L nor one of