[llvm-commits] [llvm] r151015 - /llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp

[Rafael Espindola]

Author: rafael
Date: Mon Feb 20 19:19:51 2012
New Revision: 151015

URL: http://llvm.org/viewvc/llvm-project?rev=151015&view=rev
Log:
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.

Modified:
    llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp

Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=151015&r1=151014&r2=151015&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
+++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Mon Feb 20 19:19:51 2012
@@ -31,13 +31,11 @@
 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 @@
       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 @@
     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);