It turns out that with the current scev organization ReuseOrCreateCast cannot

know where users will be added. Because of this, it cannot use
Builder.GetInsertPoint at all.

This patch
* removes the FIXME about adding the assert.
* adds a comment explaining hy we don't have one.
* removes a broken logic that only works for some callers and is not needed
  since r150884.
* adds an assert to caller that would have caught the bug fixed by r150884.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@151015 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Analysis/ScalarEvolutionExpander.cpp b/lib/Analysis/ScalarEvolutionExpander.cpp
index 852c7a08246d0d17a2ae8901f6b8e208bd4b29ab..345c8a700b599765886ced511dbec59ee3d82a5a 100644 (file)
--- a/lib/Analysis/ScalarEvolutionExpander.cpp
+++ b/lib/Analysis/ScalarEvolutionExpander.cpp
@@ -31,13 +31,11 @@ using namespace llvm;
 Value *SCEVExpander::ReuseOrCreateCast(Value *V, Type *Ty,
                                        Instruction::CastOps Op,
                                        BasicBlock::iterator IP) {
-  // All new or reused instructions must strictly dominate the Builder's
-  // InsertPt to ensure that the expression's expansion dominates its uses.
-  // Assert that the requested insertion point works at least for new
-  // instructions.
-
-  // FIXME: disabled to make the bots happy.
-  //assert(SE.DT->dominates(IP, Builder.GetInsertPoint()));
+  // All new or reused instructions must strictly dominate their uses.
+  // It would be nice to assert this here, but we don't always know where
+  // the next instructions will be added as the the caller can move the
+  // Builder's InsertPt before creating them and we might be called with
+  // an invalid InsertPt.
 
   // Check to see if there is already a cast!
   for (Value::use_iterator UI = V->use_begin(), E = V->use_end();
@@ -47,8 +45,7 @@ Value *SCEVExpander::ReuseOrCreateCast(Value *V, Type *Ty,
       if (CastInst *CI = dyn_cast<CastInst>(U))
         if (CI->getOpcode() == Op) {
           // If the cast isn't where we want it, fix it.
-          if (BasicBlock::iterator(CI) != IP
-              || IP == Builder.GetInsertPoint()) {
+          if (BasicBlock::iterator(CI) != IP) {
             // Create a new cast, and leave the old cast in place in case
             // it is being used as an insert point. Clear its operand
             // so that it doesn't hold anything live.
@@ -506,6 +503,9 @@ Value *SCEVExpander::expandAddToGEP(const SCEV *const *op_begin,
     V = InsertNoopCastOfTo(V,
        Type::getInt8PtrTy(Ty->getContext(), PTy->getAddressSpace()));
 
+    Instruction *Inst = dyn_cast<Instruction>(V);
+    assert(!Inst || SE.DT->properlyDominates(Inst, Builder.GetInsertPoint()));
+
     // Expand the operands for a plain byte offset.
     Value *Idx = expandCodeFor(SE.getAddExpr(Ops), Ty);