[PATCH] D51582: [IndVars] Turn isValidRewrite into an assertion

[Max Kazantsev via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341516: [IndVars] Turn isValidRewrite into an assertion (authored by mkazantsev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51582?vs=163963&id=164145#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51582

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


Index: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
===================================================================
--- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -136,7 +136,9 @@
   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 @@
 
 } // 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 @@
   }
   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 @@
                           << '\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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D51582.164145.patch
Type: text/x-patch
Size: 2368 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180906/939b2536/attachment.bin>