From 51690c4f52a35e1008eaeb4e90a121fa3a6dc647 Mon Sep 17 00:00:00 2001 From: Max Kazantsev Date: Thu, 6 Sep 2018 05:21:25 +0000 Subject: [PATCH] [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 llvm-svn: 341516 --- llvm/lib/Transforms/Scalar/IndVarSimplify.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp index 0683636..e7578c5 100644 --- a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp +++ b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp @@ -136,7 +136,9 @@ class IndVarSimplify { SmallVector 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(Value *FromVal, Value *ToVal) { } 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::rewriteLoopExitValues(Loop *L, SCEVExpander &Rewriter) { << '\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 -- 2.7.4