LICM shouldn't sink/delete debug information. Fix this and add a testcase.
[oota-llvm.git] / lib / Transforms / Scalar / LICM.cpp
index 695a4fb0159fe4ba5a2be0d36ec14e1df70e509a..40af0a811d103650c5417abfd6874636472d4f62 100644 (file)
@@ -35,6 +35,7 @@
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Constants.h"
 #include "llvm/DerivedTypes.h"
+#include "llvm/IntrinsicInst.h"
 #include "llvm/Instructions.h"
 #include "llvm/Target/TargetData.h"
 #include "llvm/Analysis/LoopInfo.h"
@@ -45,8 +46,8 @@
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Transforms/Utils/PromoteMemToReg.h"
 #include "llvm/Support/CFG.h"
-#include "llvm/Support/Compiler.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/ADT/Statistic.h"
 #include <algorithm>
@@ -62,10 +63,19 @@ static cl::opt<bool>
 DisablePromotion("disable-licm-promotion", cl::Hidden,
                  cl::desc("Disable memory promotion in LICM pass"));
 
+// This feature is currently disabled by default because CodeGen is not yet
+// capable of rematerializing these constants in PIC mode, so it can lead to
+// degraded performance. Compile test/CodeGen/X86/remat-constant.ll with
+// -relocation-model=pic to see an example of this.
+static cl::opt<bool>
+EnableLICMConstantMotion("enable-licm-constant-variables", cl::Hidden,
+                         cl::desc("Enable hoisting/sinking of constant "
+                                  "global variables"));
+
 namespace {
-  struct VISIBILITY_HIDDEN LICM : public LoopPass {
+  struct LICM : public LoopPass {
     static char ID; // Pass identification, replacement for typeid
-    LICM() : LoopPass((intptr_t)&ID) {}
+    LICM() : LoopPass(&ID) {}
 
     virtual bool runOnLoop(Loop *L, LPPassManager &LPM);
 
@@ -81,6 +91,7 @@ namespace {
       AU.addRequired<AliasAnalysis>();
       AU.addPreserved<ScalarEvolution>();
       AU.addPreserved<DominanceFrontier>();
+      AU.addPreservedID(LoopSimplifyID);
     }
 
     bool doFinalization() {
@@ -130,6 +141,10 @@ namespace {
     ///
     void HoistRegion(DomTreeNode *N);
 
+    // Cleanup debug information (remove stoppoints with no coressponding
+    // instructions).
+    void CleanupDbgInfoRegion(DomTreeNode *N);
+
     /// inSubLoop - Little predicate that returns true if the specified basic
     /// block is in a subloop of the current one, not the current one itself.
     ///
@@ -221,7 +236,7 @@ namespace {
 char LICM::ID = 0;
 static RegisterPass<LICM> X("licm", "Loop Invariant Code Motion");
 
-LoopPass *llvm::createLICMPass() { return new LICM(); }
+Pass *llvm::createLICMPass() { return new LICM(); }
 
 /// Hoist expressions out of the specified loop. Note, alias info for inner
 /// loop is not preserved so it is not a good idea to run LICM multiple 
@@ -258,10 +273,12 @@ bool LICM::runOnLoop(Loop *L, LPPassManager &LPM) {
   // Because subloops have already been incorporated into AST, we skip blocks in
   // subloops.
   //
-  for (std::vector<BasicBlock*>::const_iterator I = L->getBlocks().begin(),
-         E = L->getBlocks().end(); I != E; ++I)
-    if (LI->getLoopFor(*I) == L)        // Ignore blocks in subloops...
-      CurAST->add(**I);                 // Incorporate the specified basic block
+  for (Loop::block_iterator I = L->block_begin(), E = L->block_end();
+       I != E; ++I) {
+    BasicBlock *BB = *I;
+    if (LI->getLoopFor(BB) == L)        // Ignore blocks in subloops...
+      CurAST->add(*BB);                 // Incorporate the specified basic block
+  }
 
   // We want to visit all of the instructions in this loop... that are not parts
   // of our subloops (they have already had their invariants hoisted out of
@@ -275,6 +292,7 @@ bool LICM::runOnLoop(Loop *L, LPPassManager &LPM) {
   //
   SinkRegion(DT->getNode(L->getHeader()));
   HoistRegion(DT->getNode(L->getHeader()));
+  CleanupDbgInfoRegion(DT->getNode(L->getHeader()));
 
   // Now that all loop invariants have been removed from the loop, promote any
   // memory references to scalars that we can...
@@ -326,6 +344,35 @@ void LICM::SinkRegion(DomTreeNode *N) {
   }
 }
 
+void LICM::CleanupDbgInfoRegion(DomTreeNode *N) {
+  BasicBlock *BB = N->getBlock();
+
+  // If this subregion is not in the top level loop at all, exit.
+  if (!CurLoop->contains(BB)) return;
+
+  // We are processing blocks in reverse dfo, so process children first...
+  const std::vector<DomTreeNode*> &Children = N->getChildren();
+  for (unsigned i = 0, e = Children.size(); i != e; ++i)
+    CleanupDbgInfoRegion(Children[i]);
+
+  // Only need to process the contents of this block if it is not part of a
+  // subloop (which would already have been processed).
+  if (inSubLoop(BB)) return;
+
+  // We modify the basicblock, so don't cache end()
+  for (BasicBlock::iterator I=BB->begin(); I != BB->end();) {
+    Instruction *Last = 0;
+    // Remove consecutive dbgstoppoints, leave only last
+    do {
+      if (Last) {
+        Last->eraseFromParent();
+        Changed = true;
+      }
+      Last = I;
+      ++I;
+    } while (isa<DbgStopPointInst>(Last) && isa<DbgStopPointInst>(I));
+  }
+}
 
 /// HoistRegion - Walk the specified region of the CFG (defined by all blocks
 /// dominated by the specified block, and that are in the current loop) in depth
@@ -368,12 +415,22 @@ bool LICM::canSinkOrHoistInst(Instruction &I) {
     if (LI->isVolatile())
       return false;        // Don't hoist volatile loads!
 
+    // Loads from constant memory are always safe to move, even if they end up
+    // in the same alias set as something that ends up being modified.
+    if (EnableLICMConstantMotion &&
+        AA->pointsToConstantMemory(LI->getOperand(0)))
+      return true;
+    
     // Don't hoist loads which have may-aliased stores in loop.
     unsigned Size = 0;
     if (LI->getType()->isSized())
-      Size = AA->getTargetData().getTypeStoreSize(LI->getType());
+      Size = AA->getTypeStoreSize(LI->getType());
     return !pointerInvalidatedByLoop(LI->getOperand(0), Size);
   } else if (CallInst *CI = dyn_cast<CallInst>(&I)) {
+    if (isa<DbgStopPointInst>(CI)) {
+      // Don't hoist/sink dbgstoppoints, we handle them separately
+      return false;
+    }
     // Handle obvious cases efficiently.
     AliasAnalysis::ModRefBehavior Behavior = AA->getModRefBehavior(CI);
     if (Behavior == AliasAnalysis::DoesNotAccessMemory)
@@ -447,7 +504,7 @@ bool LICM::isLoopInvariantInst(Instruction &I) {
 /// position, and may either delete it or move it to outside of the loop.
 ///
 void LICM::sink(Instruction &I) {
-  DOUT << "LICM sinking instruction: " << I;
+  DEBUG(errs() << "LICM sinking instruction: " << I);
 
   SmallVector<BasicBlock*, 8> ExitBlocks;
   CurLoop->getExitBlocks(ExitBlocks);
@@ -471,9 +528,7 @@ void LICM::sink(Instruction &I) {
       // Move the instruction to the start of the exit block, after any PHI
       // nodes in it.
       I.removeFromParent();
-
-      BasicBlock::iterator InsertPt = ExitBlocks[0]->begin();
-      while (isa<PHINode>(InsertPt)) ++InsertPt;
+      BasicBlock::iterator InsertPt = ExitBlocks[0]->getFirstNonPHI();
       ExitBlocks[0]->getInstList().insert(InsertPt, &I);
     }
   } else if (ExitBlocks.empty()) {
@@ -490,7 +545,7 @@ void LICM::sink(Instruction &I) {
     // Firstly, we create a stack object to hold the value...
     AllocaInst *AI = 0;
 
-    if (I.getType() != Type::VoidTy) {
+    if (I.getType() != Type::getVoidTy(I.getContext())) {
       AI = new AllocaInst(I.getType(), 0, I.getName(),
                           I.getParent()->getParent()->getEntryBlock().begin());
       CurAST->add(AI);
@@ -542,8 +597,7 @@ void LICM::sink(Instruction &I) {
         // If we haven't already processed this exit block, do so now.
         if (InsertedBlocks.insert(ExitBlock).second) {
           // Insert the code after the last PHI node...
-          BasicBlock::iterator InsertPt = ExitBlock->begin();
-          while (isa<PHINode>(InsertPt)) ++InsertPt;
+          BasicBlock::iterator InsertPt = ExitBlock->getFirstNonPHI();
 
           // If this is the first exit block processed, just move the original
           // instruction, otherwise clone the original instruction and insert
@@ -577,7 +631,7 @@ void LICM::sink(Instruction &I) {
     if (AI) {
       std::vector<AllocaInst*> Allocas;
       Allocas.push_back(AI);
-      PromoteMemToReg(Allocas, *DT, *DF, CurAST);
+      PromoteMemToReg(Allocas, *DT, *DF, AI->getContext(), CurAST);
     }
   }
 }
@@ -586,7 +640,7 @@ void LICM::sink(Instruction &I) {
 /// that is safe to hoist, this instruction is called to do the dirty work.
 ///
 void LICM::hoist(Instruction &I) {
-  DOUT << "LICM hoisting to " << Preheader->getName() << ": " << I;
+  DEBUG(errs() << "LICM hoisting to " << Preheader->getName() << ": " << I);
 
   // Remove the instruction from its current basic block... but don't delete the
   // instruction.
@@ -607,7 +661,8 @@ void LICM::hoist(Instruction &I) {
 ///
 bool LICM::isSafeToExecuteUnconditionally(Instruction &Inst) {
   // If it is not a trapping instruction, it is always safe to hoist.
-  if (!Inst.isTrapping()) return true;
+  if (Inst.isSafeToSpeculativelyExecute())
+    return true;
 
   // Otherwise we have to check to make sure that the instruction dominates all
   // of the exit blocks.  If it doesn't, then there is a path out of the loop
@@ -619,12 +674,6 @@ bool LICM::isSafeToExecuteUnconditionally(Instruction &Inst) {
   if (Inst.getParent() == CurLoop->getHeader())
     return true;
 
-  // It's always safe to load from a global or alloca.
-  if (isa<LoadInst>(Inst))
-    if (isa<AllocationInst>(Inst.getOperand(0)) ||
-        isa<GlobalVariable>(Inst.getOperand(0)))
-      return true;
-
   // Get the exit blocks for the current loop.
   SmallVector<BasicBlock*, 8> ExitBlocks;
   CurLoop->getExitBlocks(ExitBlocks);
@@ -700,12 +749,11 @@ void LICM::PromoteValuesInLoop() {
   // Scan the basic blocks in the loop, replacing uses of our pointers with
   // uses of the allocas in question.
   //
-  const std::vector<BasicBlock*> &LoopBBs = CurLoop->getBlocks();
-  for (std::vector<BasicBlock*>::const_iterator I = LoopBBs.begin(),
-         E = LoopBBs.end(); I != E; ++I) {
+  for (Loop::block_iterator I = CurLoop->block_begin(),
+         E = CurLoop->block_end(); I != E; ++I) {
+    BasicBlock *BB = *I;
     // Rewrite all loads and stores in the block of the pointer...
-    for (BasicBlock::iterator II = (*I)->begin(), E = (*I)->end();
-         II != E; ++II) {
+    for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E; ++II) {
       if (LoadInst *L = dyn_cast<LoadInst>(II)) {
         std::map<Value*, AllocaInst*>::iterator
           I = ValueToAllocaMap.find(L->getOperand(0));
@@ -726,30 +774,30 @@ void LICM::PromoteValuesInLoop() {
   // want to insert one copy of the code in each exit block, though the loop may
   // exit to the same block more than once.
   //
-  std::set<BasicBlock*> ProcessedBlocks;
+  SmallPtrSet<BasicBlock*, 16> ProcessedBlocks;
 
   SmallVector<BasicBlock*, 8> ExitBlocks;
   CurLoop->getExitBlocks(ExitBlocks);
-  for (unsigned i = 0, e = ExitBlocks.size(); i != e; ++i)
-    if (ProcessedBlocks.insert(ExitBlocks[i]).second) {
-      // Copy all of the allocas into their memory locations.
-      BasicBlock::iterator BI = ExitBlocks[i]->begin();
-      while (isa<PHINode>(*BI))
-        ++BI;             // Skip over all of the phi nodes in the block.
-      Instruction *InsertPos = BI;
-      unsigned PVN = 0;
-      for (unsigned i = 0, e = PromotedValues.size(); i != e; ++i) {
-        // Load from the alloca.
-        LoadInst *LI = new LoadInst(PromotedValues[i].first, "", InsertPos);
-
-        // If this is a pointer type, update alias info appropriately.
-        if (isa<PointerType>(LI->getType()))
-          CurAST->copyValue(PointerValueNumbers[PVN++], LI);
-
-        // Store into the memory we promoted.
-        new StoreInst(LI, PromotedValues[i].second, InsertPos);
-      }
+  for (unsigned i = 0, e = ExitBlocks.size(); i != e; ++i) {
+    if (!ProcessedBlocks.insert(ExitBlocks[i]))
+      continue;
+  
+    // Copy all of the allocas into their memory locations.
+    BasicBlock::iterator BI = ExitBlocks[i]->getFirstNonPHI();
+    Instruction *InsertPos = BI;
+    unsigned PVN = 0;
+    for (unsigned i = 0, e = PromotedValues.size(); i != e; ++i) {
+      // Load from the alloca.
+      LoadInst *LI = new LoadInst(PromotedValues[i].first, "", InsertPos);
+
+      // If this is a pointer type, update alias info appropriately.
+      if (isa<PointerType>(LI->getType()))
+        CurAST->copyValue(PointerValueNumbers[PVN++], LI);
+
+      // Store into the memory we promoted.
+      new StoreInst(LI, PromotedValues[i].second, InsertPos);
     }
+  }
 
   // Now that we have done the deed, use the mem2reg functionality to promote
   // all of the new allocas we just created into real SSA registers.
@@ -758,7 +806,7 @@ void LICM::PromoteValuesInLoop() {
   PromotedAllocas.reserve(PromotedValues.size());
   for (unsigned i = 0, e = PromotedValues.size(); i != e; ++i)
     PromotedAllocas.push_back(PromotedValues[i].first);
-  PromoteMemToReg(PromotedAllocas, *DT, *DF, CurAST);
+  PromoteMemToReg(PromotedAllocas, *DT, *DF, Preheader->getContext(), CurAST);
 }
 
 /// FindPromotableValuesInLoop - Check the current loop for stores to definite
@@ -771,15 +819,6 @@ void LICM::FindPromotableValuesInLoop(
                              std::map<Value*, AllocaInst*> &ValueToAllocaMap) {
   Instruction *FnStart = CurLoop->getHeader()->getParent()->begin()->begin();
 
-  SmallVector<Instruction *, 4> LoopExits;
-  SmallVector<BasicBlock *, 4> Blocks;
-  CurLoop->getExitingBlocks(Blocks);
-  for (SmallVector<BasicBlock *, 4>::iterator BI = Blocks.begin(),
-         BE = Blocks.end(); BI != BE; ++BI) {
-    BasicBlock *BB = *BI;
-    LoopExits.push_back(BB->getTerminator());
-  }
-
   // Loop over all of the alias sets in the tracker object.
   for (AliasSetTracker::iterator I = CurAST->begin(), E = CurAST->end();
        I != E; ++I) {
@@ -787,72 +826,76 @@ void LICM::FindPromotableValuesInLoop(
     // We can promote this alias set if it has a store, if it is a "Must" alias
     // set, if the pointer is loop invariant, and if we are not eliminating any
     // volatile loads or stores.
-    if (!AS.isForwardingAliasSet() && AS.isMod() && AS.isMustAlias() &&
-        !AS.isVolatile() && CurLoop->isLoopInvariant(AS.begin()->first)) {
-      assert(!AS.empty() &&
-             "Must alias set should have at least one pointer element in it!");
-      Value *V = AS.begin()->first;
-
-      // Check that all of the pointers in the alias set have the same type.  We
-      // cannot (yet) promote a memory location that is loaded and stored in
-      // different sizes.
+    if (AS.isForwardingAliasSet() || !AS.isMod() || !AS.isMustAlias() ||
+        AS.isVolatile() || !CurLoop->isLoopInvariant(AS.begin()->getValue()))
+      continue;
+    
+    assert(!AS.empty() &&
+           "Must alias set should have at least one pointer element in it!");
+    Value *V = AS.begin()->getValue();
+
+    // Check that all of the pointers in the alias set have the same type.  We
+    // cannot (yet) promote a memory location that is loaded and stored in
+    // different sizes.
+    {
       bool PointerOk = true;
       for (AliasSet::iterator I = AS.begin(), E = AS.end(); I != E; ++I)
-        if (V->getType() != I->first->getType()) {
+        if (V->getType() != I->getValue()->getType()) {
           PointerOk = false;
           break;
         }
+      if (!PointerOk)
+        continue;
+    }
 
-      // If one use of value V inside the loop is safe then it is OK to promote 
-      // this value. On the otherside if there is not any unsafe use inside the
-      // loop then also it is OK to promote this value. Otherwise it is
-      // unsafe to promote this value.
-      if (PointerOk) {
-        bool oneSafeUse = false;
-        bool oneUnsafeUse = false;
-        for(Value::use_iterator UI = V->use_begin(), UE = V->use_end();
-            UI != UE; ++UI) {
-          Instruction *Use = dyn_cast<Instruction>(*UI);
-          if (!Use || !CurLoop->contains(Use->getParent()))
-            continue;
-          for (SmallVector<Instruction *, 4>::iterator 
-                 ExitI = LoopExits.begin(), ExitE = LoopExits.end();
-               ExitI != ExitE; ++ExitI) {
-            Instruction *Ex = *ExitI;
-            if (!isa<PHINode>(Use) && DT->dominates(Use, Ex)) {
-              oneSafeUse = true;
-              break;
-            }
-            else 
-              oneUnsafeUse = true;
-          }
-
-          if (oneSafeUse)
-            break;
-        }
-
-        if (oneSafeUse)
-          PointerOk = true;
-        else if (!oneUnsafeUse)
-          PointerOk = true;
-        else
-          PointerOk = false;
+    // It isn't safe to promote a load/store from the loop if the load/store is
+    // conditional.  For example, turning:
+    //
+    //    for () { if (c) *P += 1; }
+    //
+    // into:
+    //
+    //    tmp = *P;  for () { if (c) tmp +=1; } *P = tmp;
+    //
+    // is not safe, because *P may only be valid to access if 'c' is true.
+    // 
+    // It is safe to promote P if all uses are direct load/stores and if at
+    // least one is guaranteed to be executed.
+    bool GuaranteedToExecute = false;
+    bool InvalidInst = false;
+    for (Value::use_iterator UI = V->use_begin(), UE = V->use_end();
+         UI != UE; ++UI) {
+      // Ignore instructions not in this loop.
+      Instruction *Use = dyn_cast<Instruction>(*UI);
+      if (!Use || !CurLoop->contains(Use->getParent()))
+        continue;
+
+      if (!isa<LoadInst>(Use) && !isa<StoreInst>(Use)) {
+        InvalidInst = true;
+        break;
       }
       
-      if (PointerOk) {
-        const Type *Ty = cast<PointerType>(V->getType())->getElementType();
-        AllocaInst *AI = new AllocaInst(Ty, 0, V->getName()+".tmp", FnStart);
-        PromotedValues.push_back(std::make_pair(AI, V));
+      if (!GuaranteedToExecute)
+        GuaranteedToExecute = isSafeToExecuteUnconditionally(*Use);
+    }
 
-        // Update the AST and alias analysis.
-        CurAST->copyValue(V, AI);
+    // If there is an non-load/store instruction in the loop, we can't promote
+    // it.  If there isn't a guaranteed-to-execute instruction, we can't
+    // promote.
+    if (InvalidInst || !GuaranteedToExecute)
+      continue;
+    
+    const Type *Ty = cast<PointerType>(V->getType())->getElementType();
+    AllocaInst *AI = new AllocaInst(Ty, 0, V->getName()+".tmp", FnStart);
+    PromotedValues.push_back(std::make_pair(AI, V));
 
-        for (AliasSet::iterator I = AS.begin(), E = AS.end(); I != E; ++I)
-          ValueToAllocaMap.insert(std::make_pair(I->first, AI));
+    // Update the AST and alias analysis.
+    CurAST->copyValue(V, AI);
 
-        DOUT << "LICM: Promoting value: " << *V << "\n";
-      }
-    }
+    for (AliasSet::iterator I = AS.begin(), E = AS.end(); I != E; ++I)
+      ValueToAllocaMap.insert(std::make_pair(I->getValue(), AI));
+
+    DEBUG(errs() << "LICM: Promoting value: " << *V << "\n");
   }
 }