Make RenamePass faster by making the 'is this a new phi node'
[oota-llvm.git] / lib / Transforms / Utils / PromoteMemoryToRegister.cpp
index c5861832a1af4897e345e0c826fe30063dd2bb40..7d08a7cfd1d996b2b179935a07ee6c5a76459547 100644 (file)
@@ -2,12 +2,12 @@
 //
 //                     The LLVM Compiler Infrastructure
 //
-// This file was developed by the LLVM research group and is distributed under
-// the University of Illinois Open Source License. See LICENSE.TXT for details.
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
-// This file promote memory references to be register references.  It promotes
+// This file promotes memory references to be register references.  It promotes
 // alloca instructions which only have loads and stores as uses.  An alloca is
 // transformed by using dominator frontiers to place PHI nodes, then traversing
 // the function in depth-first order to rewrite loads and stores as appropriate.
@@ -37,19 +37,24 @@ using namespace llvm;
 STATISTIC(NumLocalPromoted, "Number of alloca's promoted within one block");
 STATISTIC(NumSingleStore,   "Number of alloca's promoted with a single store");
 STATISTIC(NumDeadAlloca,    "Number of dead alloca's removed");
+STATISTIC(NumPHIInsert,     "Number of PHI nodes inserted");
 
-// Provide DenseMapKeyInfo for all pointers.
+// Provide DenseMapInfo for all pointers.
 namespace llvm {
 template<>
-struct DenseMapKeyInfo<std::pair<BasicBlock*, unsigned> > {
-  static inline std::pair<BasicBlock*, unsigned> getEmptyKey() {
-    return std::make_pair((BasicBlock*)-1, ~0U);
+struct DenseMapInfo<std::pair<BasicBlock*, unsigned> > {
+  typedef std::pair<BasicBlock*, unsigned> EltTy;
+  static inline EltTy getEmptyKey() {
+    return EltTy(reinterpret_cast<BasicBlock*>(-1), ~0U);
   }
-  static inline std::pair<BasicBlock*, unsigned> getTombstoneKey() {
-    return std::make_pair((BasicBlock*)-2, 0U);
+  static inline EltTy getTombstoneKey() {
+    return EltTy(reinterpret_cast<BasicBlock*>(-2), 0U);
   }
   static unsigned getHashValue(const std::pair<BasicBlock*, unsigned> &Val) {
-    return DenseMapKeyInfo<void*>::getHashValue(Val.first) + Val.second*2;
+    return DenseMapInfo<void*>::getHashValue(Val.first) + Val.second*2;
+  }
+  static bool isEqual(const EltTy &LHS, const EltTy &RHS) {
+    return LHS == RHS;
   }
   static bool isPod() { return true; }
 };
@@ -62,14 +67,17 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) {
   // FIXME: If the memory unit is of pointer or integer type, we can permit
   // assignments to subsections of the memory unit.
 
-  // Only allow direct loads and stores...
+  // Only allow direct and non-volatile loads and stores...
   for (Value::use_const_iterator UI = AI->use_begin(), UE = AI->use_end();
        UI != UE; ++UI)     // Loop over all of the uses of the alloca
-    if (isa<LoadInst>(*UI)) {
-      // noop
+    if (const LoadInst *LI = dyn_cast<LoadInst>(*UI)) {
+      if (LI->isVolatile())
+        return false;
     } else if (const StoreInst *SI = dyn_cast<StoreInst>(*UI)) {
       if (SI->getOperand(0) == AI)
         return false;   // Don't allow a store OF the AI, only INTO the AI.
+      if (SI->isVolatile())
+        return false;
     } else {
       return false;   // Not a load or store.
     }
@@ -137,6 +145,8 @@ namespace {
     /// non-determinstic behavior.
     DenseMap<BasicBlock*, unsigned> BBNumbers;
 
+    /// BBNumPreds - Lazily compute the number of predecessors a block has.
+    DenseMap<const BasicBlock*, unsigned> BBNumPreds;
   public:
     PromoteMem2Reg(const std::vector<AllocaInst*> &A,
                    SmallVector<AllocaInst*, 16> &Retry, DominatorTree &dt,
@@ -165,11 +175,22 @@ namespace {
       Allocas.pop_back();
       --AllocaIdx;
     }
+
+    unsigned getNumPreds(const BasicBlock *BB) {
+      unsigned &NP = BBNumPreds[BB];
+      if (NP == 0)
+        NP = std::distance(pred_begin(BB), pred_end(BB))+1;
+      return NP-1;
+    }
+
+    void DetermineInsertionPoint(AllocaInst *AI, unsigned AllocaNum,
+                                 AllocaInfo &Info);
+    void ComputeLiveInBlocks(AllocaInst *AI, AllocaInfo &Info, 
+                             const SmallPtrSet<BasicBlock*, 32> &DefBlocks,
+                             SmallPtrSet<BasicBlock*, 32> &LiveInBlocks);
     
     void RewriteSingleStoreAlloca(AllocaInst *AI, AllocaInfo &Info);
 
-    void MarkDominatingPHILive(BasicBlock *BB, unsigned AllocaNum,
-                               SmallPtrSet<PHINode*, 16> &DeadPHINodes);
     bool PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI);
     void PromoteLocallyUsedAllocas(BasicBlock *BB,
                                    const std::vector<AllocaInst*> &AIs);
@@ -209,7 +230,7 @@ namespace {
       // and decide whether all of the loads and stores to the alloca are within
       // the same basic block.
       for (Value::use_iterator U = AI->use_begin(), E = AI->use_end();
-           U != E; ++U){
+           U != E; ++U) {
         Instruction *User = cast<Instruction>(*U);
         if (StoreInst *SI = dyn_cast<StoreInst>(User)) {
           // Remember the basic blocks which define new values for the alloca
@@ -218,7 +239,8 @@ namespace {
           OnlyStore = SI;
         } else {
           LoadInst *LI = cast<LoadInst>(User);
-          // Otherwise it must be a load instruction, keep track of variable reads
+          // Otherwise it must be a load instruction, keep track of variable
+          // reads.
           UsingBlocks.push_back(LI->getParent());
           AllocaPointerVal = LI;
         }
@@ -273,16 +295,6 @@ void PromoteMem2Reg::run() {
     // analogous to finding the 'uses' and 'definitions' of each variable.
     Info.AnalyzeAlloca(AI);
 
-    // If the alloca is only read and written in one basic block, just perform a
-    // linear sweep over the block to eliminate it.
-    if (Info.OnlyUsedInOneBlock) {
-      LocallyUsedAllocas[Info.OnlyBlock].push_back(AI);
-
-      // Remove the alloca from the Allocas list, since it will be processed.
-      RemoveFromAllocasList(AllocaNum);
-      continue;
-    }
-
     // If there is only a single store to this value, replace any loads of
     // it that are directly dominated by the definition with the value stored.
     if (Info.DefiningBlocks.size() == 1) {
@@ -303,10 +315,16 @@ void PromoteMem2Reg::run() {
       }
     }
     
+    // If the alloca is only read and written in one basic block, just perform a
+    // linear sweep over the block to eliminate it.
+    if (Info.OnlyUsedInOneBlock) {
+      LocallyUsedAllocas[Info.OnlyBlock].push_back(AI);
+      
+      // Remove the alloca from the Allocas list, since it will be processed.
+      RemoveFromAllocasList(AllocaNum);
+      continue;
+    }
     
-    if (AST)
-      PointerAllocaValues[AllocaNum] = Info.AllocaPointerVal;
-
     // If we haven't computed a numbering for the BB's in the function, do so
     // now.
     if (BBNumbers.empty()) {
@@ -315,69 +333,19 @@ void PromoteMem2Reg::run() {
         BBNumbers[I] = ID++;
     }
 
-    // Compute the locations where PhiNodes need to be inserted.  Look at the
-    // dominance frontier of EACH basic-block we have a write in.
-    //
-    unsigned CurrentVersion = 0;
-    SmallPtrSet<PHINode*, 16> InsertedPHINodes;
-    std::vector<std::pair<unsigned, BasicBlock*> > DFBlocks;
-    while (!Info.DefiningBlocks.empty()) {
-      BasicBlock *BB = Info.DefiningBlocks.back();
-      Info.DefiningBlocks.pop_back();
-
-      // Look up the DF for this write, add it to PhiNodes
-      DominanceFrontier::const_iterator it = DF.find(BB);
-      if (it != DF.end()) {
-        const DominanceFrontier::DomSetType &S = it->second;
-
-        // In theory we don't need the indirection through the DFBlocks vector.
-        // In practice, the order of calling QueuePhiNode would depend on the
-        // (unspecified) ordering of basic blocks in the dominance frontier,
-        // which would give PHI nodes non-determinstic subscripts.  Fix this by
-        // processing blocks in order of the occurance in the function.
-        for (DominanceFrontier::DomSetType::const_iterator P = S.begin(),
-             PE = S.end(); P != PE; ++P)
-          DFBlocks.push_back(std::make_pair(BBNumbers[*P], *P));
-
-        // Sort by which the block ordering in the function.
-        std::sort(DFBlocks.begin(), DFBlocks.end());
-
-        for (unsigned i = 0, e = DFBlocks.size(); i != e; ++i) {
-          BasicBlock *BB = DFBlocks[i].second;
-          if (QueuePhiNode(BB, AllocaNum, CurrentVersion, InsertedPHINodes))
-            Info.DefiningBlocks.push_back(BB);
-        }
-        DFBlocks.clear();
-      }
-    }
-
-    // Now that we have inserted PHI nodes along the Iterated Dominance Frontier
-    // of the writes to the variable, scan through the reads of the variable,
-    // marking PHI nodes which are actually necessary as alive (by removing them
-    // from the InsertedPHINodes set).  This is not perfect: there may PHI
-    // marked alive because of loads which are dominated by stores, but there
-    // will be no unmarked PHI nodes which are actually used.
-    //
-    for (unsigned i = 0, e = Info.UsingBlocks.size(); i != e; ++i)
-      MarkDominatingPHILive(Info.UsingBlocks[i], AllocaNum, InsertedPHINodes);
-    Info.UsingBlocks.clear();
-
-    // If there are any PHI nodes which are now known to be dead, remove them!
-    for (SmallPtrSet<PHINode*, 16>::iterator I = InsertedPHINodes.begin(),
-           E = InsertedPHINodes.end(); I != E; ++I) {
-      PHINode *PN = *I;
-      bool Erased=NewPhiNodes.erase(std::make_pair(PN->getParent(), AllocaNum));
-      Erased=Erased;
-      assert(Erased && "PHI already removed?");
-      
-      if (AST && isa<PointerType>(PN->getType()))
-        AST->deleteValue(PN);
-      PN->eraseFromParent();
-      PhiToAllocaMap.erase(PN);
-    }
-
-    // Keep the reverse mapping of the 'Allocas' array.
+    // If we have an AST to keep updated, remember some pointer value that is
+    // stored into the alloca.
+    if (AST)
+      PointerAllocaValues[AllocaNum] = Info.AllocaPointerVal;
+    
+    // Keep the reverse mapping of the 'Allocas' array for the rename pass.
     AllocaLookup[Allocas[AllocaNum]] = AllocaNum;
+
+    // At this point, we're committed to promoting the alloca using IDF's, and
+    // the standard SSA construction algorithm.  Determine which blocks need phi
+    // nodes and see if we can optimize out some work by avoiding insertion of
+    // dead phi nodes.
+    DetermineInsertionPoint(AI, AllocaNum, Info);
   }
 
   // Process all allocas which are only used in a single basic block.
@@ -486,14 +454,14 @@ void PromoteMem2Reg::run() {
     if (&BB->front() != SomePHI)
       continue;
 
-    // Count the number of preds for BB.
-    SmallVector<BasicBlock*, 16> Preds(pred_begin(BB), pred_end(BB));
-
     // Only do work here if there the PHI nodes are missing incoming values.  We
     // know that all PHI nodes that were inserted in a block will have the same
     // number of incoming values, so we can just check any of them.
-    if (SomePHI->getNumIncomingValues() == Preds.size())
+    if (SomePHI->getNumIncomingValues() == getNumPreds(BB))
       continue;
+
+    // Get the preds for BB.
+    SmallVector<BasicBlock*, 16> Preds(pred_begin(BB), pred_end(BB));
     
     // Ok, now we know that all of the PHI nodes are missing entries for some
     // basic blocks.  Start by sorting the incoming predecessors for efficient
@@ -532,6 +500,138 @@ void PromoteMem2Reg::run() {
 }
 
 
+/// ComputeLiveInBlocks - Determine which blocks the value is live in.  These
+/// are blocks which lead to uses.  Knowing this allows us to avoid inserting
+/// PHI nodes into blocks which don't lead to uses (thus, the inserted phi nodes
+/// would be dead).
+void PromoteMem2Reg::
+ComputeLiveInBlocks(AllocaInst *AI, AllocaInfo &Info, 
+                    const SmallPtrSet<BasicBlock*, 32> &DefBlocks,
+                    SmallPtrSet<BasicBlock*, 32> &LiveInBlocks) {
+  
+  // To determine liveness, we must iterate through the predecessors of blocks
+  // where the def is live.  Blocks are added to the worklist if we need to
+  // check their predecessors.  Start with all the using blocks.
+  SmallVector<BasicBlock*, 64> LiveInBlockWorklist;
+  LiveInBlockWorklist.insert(LiveInBlockWorklist.end(), 
+                             Info.UsingBlocks.begin(), Info.UsingBlocks.end());
+  
+  // If any of the using blocks is also a definition block, check to see if the
+  // definition occurs before or after the use.  If it happens before the use,
+  // the value isn't really live-in.
+  for (unsigned i = 0, e = LiveInBlockWorklist.size(); i != e; ++i) {
+    BasicBlock *BB = LiveInBlockWorklist[i];
+    if (!DefBlocks.count(BB)) continue;
+    
+    // Okay, this is a block that both uses and defines the value.  If the first
+    // reference to the alloca is a def (store), then we know it isn't live-in.
+    for (BasicBlock::iterator I = BB->begin(); ; ++I) {
+      if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
+        if (SI->getOperand(1) != AI) continue;
+        
+        // We found a store to the alloca before a load.  The alloca is not
+        // actually live-in here.
+        LiveInBlockWorklist[i] = LiveInBlockWorklist.back();
+        LiveInBlockWorklist.pop_back();
+        --i, --e;
+        break;
+      } else if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
+        if (LI->getOperand(0) != AI) continue;
+        
+        // Okay, we found a load before a store to the alloca.  It is actually
+        // live into this block.
+        break;
+      }
+    }
+  }
+  
+  // Now that we have a set of blocks where the phi is live-in, recursively add
+  // their predecessors until we find the full region the value is live.
+  while (!LiveInBlockWorklist.empty()) {
+    BasicBlock *BB = LiveInBlockWorklist.back();
+    LiveInBlockWorklist.pop_back();
+    
+    // The block really is live in here, insert it into the set.  If already in
+    // the set, then it has already been processed.
+    if (!LiveInBlocks.insert(BB))
+      continue;
+    
+    // Since the value is live into BB, it is either defined in a predecessor or
+    // live into it to.  Add the preds to the worklist unless they are a
+    // defining block.
+    for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {
+      BasicBlock *P = *PI;
+      
+      // The value is not live into a predecessor if it defines the value.
+      if (DefBlocks.count(P))
+        continue;
+      
+      // Otherwise it is, add to the worklist.
+      LiveInBlockWorklist.push_back(P);
+    }
+  }
+}
+
+/// DetermineInsertionPoint - At this point, we're committed to promoting the
+/// alloca using IDF's, and the standard SSA construction algorithm.  Determine
+/// which blocks need phi nodes and see if we can optimize out some work by
+/// avoiding insertion of dead phi nodes.
+void PromoteMem2Reg::DetermineInsertionPoint(AllocaInst *AI, unsigned AllocaNum,
+                                             AllocaInfo &Info) {
+
+  // Unique the set of defining blocks for efficient lookup.
+  SmallPtrSet<BasicBlock*, 32> DefBlocks;
+  DefBlocks.insert(Info.DefiningBlocks.begin(), Info.DefiningBlocks.end());
+
+  // Determine which blocks the value is live in.  These are blocks which lead
+  // to uses.
+  SmallPtrSet<BasicBlock*, 32> LiveInBlocks;
+  ComputeLiveInBlocks(AI, Info, DefBlocks, LiveInBlocks);
+
+  // Compute the locations where PhiNodes need to be inserted.  Look at the
+  // dominance frontier of EACH basic-block we have a write in.
+  unsigned CurrentVersion = 0;
+  SmallPtrSet<PHINode*, 16> InsertedPHINodes;
+  std::vector<std::pair<unsigned, BasicBlock*> > DFBlocks;
+  while (!Info.DefiningBlocks.empty()) {
+    BasicBlock *BB = Info.DefiningBlocks.back();
+    Info.DefiningBlocks.pop_back();
+    
+    // Look up the DF for this write, add it to defining blocks.
+    DominanceFrontier::const_iterator it = DF.find(BB);
+    if (it == DF.end()) continue;
+    
+    const DominanceFrontier::DomSetType &S = it->second;
+    
+    // In theory we don't need the indirection through the DFBlocks vector.
+    // In practice, the order of calling QueuePhiNode would depend on the
+    // (unspecified) ordering of basic blocks in the dominance frontier,
+    // which would give PHI nodes non-determinstic subscripts.  Fix this by
+    // processing blocks in order of the occurance in the function.
+    for (DominanceFrontier::DomSetType::const_iterator P = S.begin(),
+         PE = S.end(); P != PE; ++P) {
+      // If the frontier block is not in the live-in set for the alloca, don't
+      // bother processing it.
+      if (!LiveInBlocks.count(*P))
+        continue;
+      
+      DFBlocks.push_back(std::make_pair(BBNumbers[*P], *P));
+    }
+    
+    // Sort by which the block ordering in the function.
+    if (DFBlocks.size() > 1)
+      std::sort(DFBlocks.begin(), DFBlocks.end());
+    
+    for (unsigned i = 0, e = DFBlocks.size(); i != e; ++i) {
+      BasicBlock *BB = DFBlocks[i].second;
+      if (QueuePhiNode(BB, AllocaNum, CurrentVersion, InsertedPHINodes))
+        Info.DefiningBlocks.push_back(BB);
+    }
+    DFBlocks.clear();
+  }
+}
+  
+
 /// RewriteSingleStoreAlloca - If there is only a single store to this value,
 /// replace any loads of it that are directly dominated by the definition with
 /// the value stored.
@@ -595,40 +695,6 @@ void PromoteMem2Reg::RewriteSingleStoreAlloca(AllocaInst *AI,
 }
 
 
-// MarkDominatingPHILive - Mem2Reg wants to construct "pruned" SSA form, not
-// "minimal" SSA form.  To do this, it inserts all of the PHI nodes on the IDF
-// as usual (inserting the PHI nodes in the DeadPHINodes set), then processes
-// each read of the variable.  For each block that reads the variable, this
-// function is called, which removes used PHI nodes from the DeadPHINodes set.
-// After all of the reads have been processed, any PHI nodes left in the
-// DeadPHINodes set are removed.
-//
-void PromoteMem2Reg::MarkDominatingPHILive(BasicBlock *BB, unsigned AllocaNum,
-                                      SmallPtrSet<PHINode*, 16> &DeadPHINodes) {
-  // Scan the immediate dominators of this block looking for a block which has a
-  // PHI node for Alloca num.  If we find it, mark the PHI node as being alive!
-  DomTreeNode *IDomNode = DT.getNode(BB);
-  for (DomTreeNode *IDom = IDomNode; IDom; IDom = IDom->getIDom()) {
-    BasicBlock *DomBB = IDom->getBlock();
-    DenseMap<std::pair<BasicBlock*, unsigned>, PHINode*>::iterator
-      I = NewPhiNodes.find(std::make_pair(DomBB, AllocaNum));
-    if (I != NewPhiNodes.end()) {
-      // Ok, we found an inserted PHI node which dominates this value.
-      PHINode *DominatingPHI = I->second;
-
-      // Find out if we previously thought it was dead.  If so, mark it as being
-      // live by removing it from the DeadPHINodes set.
-      if (DeadPHINodes.erase(DominatingPHI)) {
-        // Now that we have marked the PHI node alive, also mark any PHI nodes
-        // which it might use as being alive as well.
-        for (pred_iterator PI = pred_begin(DomBB), PE = pred_end(DomBB);
-             PI != PE; ++PI)
-          MarkDominatingPHILive(*PI, AllocaNum, DeadPHINodes);
-      }
-    }
-  }
-}
-
 /// PromoteLocallyUsedAlloca - Many allocas are only used within a single basic
 /// block.  If this is the case, avoid traversing the CFG and inserting a lot of
 /// potentially useless PHI nodes by just performing a single linear pass over
@@ -771,7 +837,9 @@ bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo,
   PN = new PHINode(Allocas[AllocaNo]->getAllocatedType(),
                    Allocas[AllocaNo]->getName() + "." +
                    utostr(Version++), BB->begin());
+  ++NumPHIInsert;
   PhiToAllocaMap[PN] = AllocaNo;
+  PN->reserveOperandSpace(getNumPreds(BB));
   
   InsertedPHINodes.insert(PN);
 
@@ -781,7 +849,6 @@ bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo,
   return true;
 }
 
-
 // RenamePass - Recursively traverse the CFG of the function, renaming loads and
 // stores to the allocas which we are promoting.  IncomingVals indicates what
 // value each Alloca contains on exit from the predecessor block Pred.
@@ -789,6 +856,7 @@ bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo,
 void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
                                 RenamePassData::ValVector &IncomingVals,
                                 std::vector<RenamePassData> &Worklist) {
+NextIteration:
   // If we are inserting any phi nodes into this BB, they will already be in the
   // block.
   if (PHINode *APN = dyn_cast<PHINode>(BB->begin())) {
@@ -808,6 +876,14 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
     // If we have PHI nodes to update, compute the number of edges from Pred to
     // BB.
     if (!HasPredEntries) {
+      // We want to be able to distinguish between PHI nodes being inserted by
+      // this invocation of mem2reg from those phi nodes that already existed in
+      // the IR before mem2reg was run.  We determine that APN is being inserted
+      // because it is missing incoming edges.  All other PHI nodes being
+      // inserted by this pass of mem2reg will have the same number of incoming
+      // operands so far.  Remember this count.
+      unsigned NewPHINumOperands = APN->getNumOperands();
+      
       TerminatorInst *PredTerm = Pred->getTerminator();
       unsigned NumEdges = 0;
       for (unsigned i = 0, e = PredTerm->getNumSuccessors(); i != e; ++i) {
@@ -833,16 +909,9 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
         APN = dyn_cast<PHINode>(PNI);
         if (APN == 0) break;
         
-        // Verify it doesn't already have entries for Pred.  If it does, it is
-        // not being inserted by this mem2reg invocation.
-        HasPredEntries = false;
-        for (unsigned i = 0, e = APN->getNumIncomingValues(); i != e; ++i) {
-          if (APN->getIncomingBlock(i) == Pred) {
-            HasPredEntries = true;
-            break;
-          }
-        }
-      } while (!HasPredEntries);
+        // Verify that it is missing entries.  If not, it is not being inserted
+        // by this mem2reg invocation so we want to ignore it.
+      } while (APN->getNumOperands() == NewPHINumOperands);
     }
   }
   
@@ -853,36 +922,49 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
     Instruction *I = II++; // get the instruction, increment iterator
 
     if (LoadInst *LI = dyn_cast<LoadInst>(I)) {
-      if (AllocaInst *Src = dyn_cast<AllocaInst>(LI->getPointerOperand())) {
-        std::map<AllocaInst*, unsigned>::iterator AI = AllocaLookup.find(Src);
-        if (AI != AllocaLookup.end()) {
-          Value *V = IncomingVals[AI->second];
+      AllocaInst *Src = dyn_cast<AllocaInst>(LI->getPointerOperand());
+      if (!Src) continue;
+  
+      std::map<AllocaInst*, unsigned>::iterator AI = AllocaLookup.find(Src);
+      if (AI == AllocaLookup.end()) continue;
 
-          // walk the use list of this load and replace all uses with r
-          LI->replaceAllUsesWith(V);
-          if (AST && isa<PointerType>(LI->getType()))
-            AST->deleteValue(LI);
-          BB->getInstList().erase(LI);
-        }
-      }
+      Value *V = IncomingVals[AI->second];
+
+      // Anything using the load now uses the current value.
+      LI->replaceAllUsesWith(V);
+      if (AST && isa<PointerType>(LI->getType()))
+        AST->deleteValue(LI);
+      BB->getInstList().erase(LI);
     } else if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
       // Delete this instruction and mark the name as the current holder of the
       // value
-      if (AllocaInst *Dest = dyn_cast<AllocaInst>(SI->getPointerOperand())) {
-        std::map<AllocaInst *, unsigned>::iterator ai = AllocaLookup.find(Dest);
-        if (ai != AllocaLookup.end()) {
-          // what value were we writing?
-          IncomingVals[ai->second] = SI->getOperand(0);
-          BB->getInstList().erase(SI);
-        }
-      }
+      AllocaInst *Dest = dyn_cast<AllocaInst>(SI->getPointerOperand());
+      if (!Dest) continue;
+      
+      std::map<AllocaInst *, unsigned>::iterator ai = AllocaLookup.find(Dest);
+      if (ai == AllocaLookup.end())
+        continue;
+      
+      // what value were we writing?
+      IncomingVals[ai->second] = SI->getOperand(0);
+      BB->getInstList().erase(SI);
     }
   }
 
-  // Recurse to our successors.
+  // 'Recurse' to our successors.
   TerminatorInst *TI = BB->getTerminator();
-  for (unsigned i = 0; i != TI->getNumSuccessors(); i++)
+  unsigned NumSuccs = TI->getNumSuccessors();
+  if (NumSuccs == 0) return;
+  
+  // Add all-but-one successor to the worklist.
+  for (unsigned i = 0; i != NumSuccs-1; i++)
     Worklist.push_back(RenamePassData(TI->getSuccessor(i), BB, IncomingVals));
+  
+  // Handle the last successor without using the worklist.  This allows us to
+  // handle unconditional branches directly, for example.
+  Pred = BB;
+  BB = TI->getSuccessor(NumSuccs-1);
+  goto NextIteration;
 }
 
 /// PromoteMemToReg - Promote the specified list of alloca instructions into