eliminate some malloc traffic, this speeds up mem2reg by 3.4%.
[oota-llvm.git] / lib / Transforms / Utils / PromoteMemoryToRegister.cpp
index d35b457125e658977c1eb2dc01e700920052be2b..a25ea6b54de4d537d24e5f7f5e0e57b01c199a1a 100644 (file)
@@ -1,36 +1,38 @@
 //===- PromoteMemoryToRegister.cpp - Convert allocas to registers ---------===//
-// 
+//
 //                     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 promote memory references to be register references.  It promotes
-// alloca instructions which only have loads and stores as uses (or that have
-// PHI nodes which are only loaded from).  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.  This is just
-// the standard SSA construction algorithm to construct "pruned" SSA form.
+// 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.
+// This is just the standard SSA construction algorithm to construct "pruned"
+// SSA form.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/PromoteMemToReg.h"
-#include "llvm/Analysis/Dominators.h"
-#include "llvm/iMemory.h"
-#include "llvm/iPHINode.h"
-#include "llvm/iOther.h"
+#include "llvm/Constants.h"
+#include "llvm/DerivedTypes.h"
 #include "llvm/Function.h"
-#include "llvm/Constant.h"
+#include "llvm/Instructions.h"
+#include "llvm/Analysis/Dominators.h"
+#include "llvm/Analysis/AliasSetTracker.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CFG.h"
 #include "llvm/Support/StableBasicBlockNumbering.h"
-#include "Support/StringExtras.h"
+#include "llvm/Support/Compiler.h"
+#include <algorithm>
 using namespace llvm;
 
 /// isAllocaPromotable - Return true if this alloca is legal for promotion.
-/// This is true if there are only loads and stores to the alloca... of if there
-/// is a PHI node using the address which can be trivially transformed.
+/// This is true if there are only loads and stores to the alloca.
 ///
 bool llvm::isAllocaPromotable(const AllocaInst *AI, const TargetData &TD) {
   // FIXME: If the memory unit is of pointer or integer type, we can permit
@@ -44,110 +46,77 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI, const TargetData &TD) {
     } 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.
-    } else if (const PHINode *PN = dyn_cast<PHINode>(*UI)) {
-      // We only support PHI nodes in a few simple cases.  The PHI node is only
-      // allowed to have one use, which must be a load instruction, and can only
-      // use alloca instructions (no random pointers).  Also, there cannot be
-      // any accesses to AI between the PHI node and the use of the PHI.
-      if (!PN->hasOneUse()) return false;
-
-      // Our transformation causes the unconditional loading of all pointer
-      // operands to the PHI node.  Because this could cause a fault if there is
-      // a critical edge in the CFG and if one of the pointers is illegal, we
-      // refuse to promote PHI nodes unless they are obviously safe.  For now,
-      // obviously safe means that all of the operands are allocas.
-      //
-      // If we wanted to extend this code to break critical edges, this
-      // restriction could be relaxed, and we could even handle uses of the PHI
-      // node that are volatile loads or stores.
-      //
-      for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
-        if (!isa<AllocaInst>(PN->getIncomingValue(i)))
-          return false;
-      
-      // Now make sure the one user instruction is in the same basic block as
-      // the PHI, and that there are no loads or stores between the PHI node and
-      // the access.
-      BasicBlock::const_iterator UI = cast<Instruction>(PN->use_back());
-      if (!isa<LoadInst>(UI) || cast<LoadInst>(UI)->isVolatile()) return false;
-      
-      // Scan looking for memory accesses.
-      // FIXME: this should REALLY use alias analysis.
-      for (--UI; !isa<PHINode>(UI); --UI)
-        if (isa<LoadInst>(UI) || isa<StoreInst>(UI) || isa<CallInst>(UI))
-          return false;
-
-      // If we got this far, we can promote the PHI use.
-    } else if (const SelectInst *SI = dyn_cast<SelectInst>(*UI)) {
-      // We only support selects in a few simple cases.  The select is only
-      // allowed to have one use, which must be a load instruction, and can only
-      // use alloca instructions (no random pointers).  Also, there cannot be
-      // any accesses to AI between the PHI node and the use of the PHI.
-      if (!SI->hasOneUse()) return false;
-
-      // Our transformation causes the unconditional loading of all pointer
-      // operands of the select.  Because this could cause a fault if there is a
-      // critical edge in the CFG and if one of the pointers is illegal, we
-      // refuse to promote the select unless it is obviously safe.  For now,
-      // obviously safe means that all of the operands are allocas.
-      //
-      if (!isa<AllocaInst>(SI->getOperand(1)) ||
-          !isa<AllocaInst>(SI->getOperand(2)))
-        return false;
-      
-      // Now make sure the one user instruction is in the same basic block as
-      // the PHI, and that there are no loads or stores between the PHI node and
-      // the access.
-      BasicBlock::const_iterator UI = cast<Instruction>(SI->use_back());
-      if (!isa<LoadInst>(UI) || cast<LoadInst>(UI)->isVolatile()) return false;
-      
-      // Scan looking for memory accesses.
-      // FIXME: this should REALLY use alias analysis.
-      for (--UI; &*UI != SI; --UI)
-        if (isa<LoadInst>(UI) || isa<StoreInst>(UI) || isa<CallInst>(UI))
-          return false;
-
-      // If we got this far, we can promote the select use.
     } else {
-      return false;   // Not a load, store, or promotable PHI?
+      return false;   // Not a load or store.
     }
-  
+
   return true;
 }
 
 namespace {
-  struct PromoteMem2Reg {
-    // Allocas - The alloca instructions being promoted
+  struct VISIBILITY_HIDDEN PromoteMem2Reg {
+    /// Allocas - The alloca instructions being promoted.
+    ///
     std::vector<AllocaInst*> Allocas;
+    SmallVector<AllocaInst*, 16> &RetryList;
     DominatorTree &DT;
     DominanceFrontier &DF;
     const TargetData &TD;
 
-    // AllocaLookup - Reverse mapping of Allocas
+    /// AST - An AliasSetTracker object to update.  If null, don't update it.
+    ///
+    AliasSetTracker *AST;
+
+    /// AllocaLookup - Reverse mapping of Allocas.
+    ///
     std::map<AllocaInst*, unsigned>  AllocaLookup;
 
-    // NewPhiNodes - The PhiNodes we're adding.
+    /// NewPhiNodes - The PhiNodes we're adding.
+    ///
     std::map<BasicBlock*, std::vector<PHINode*> > NewPhiNodes;
 
-    // Visited - The set of basic blocks the renamer has already visited.
+    /// PointerAllocaValues - If we are updating an AliasSetTracker, then for
+    /// each alloca that is of pointer type, we keep track of what to copyValue
+    /// to the inserted PHI nodes here.
+    ///
+    std::vector<Value*> PointerAllocaValues;
+
+    /// Visited - The set of basic blocks the renamer has already visited.
+    ///
     std::set<BasicBlock*> Visited;
 
-    // BBNumbers - Contains a stable numbering of basic blocks to avoid
-    // non-determinstic behavior.
+    /// BBNumbers - Contains a stable numbering of basic blocks to avoid
+    /// non-determinstic behavior.
     StableBasicBlockNumbering BBNumbers;
 
   public:
-    PromoteMem2Reg(const std::vector<AllocaInst*> &A, DominatorTree &dt,
-                   DominanceFrontier &df, const TargetData &td)
-      : Allocas(A), DT(dt), DF(df), TD(td) {}
+    PromoteMem2Reg(const std::vector<AllocaInst*> &A,
+                   SmallVector<AllocaInst*, 16> &Retry, DominatorTree &dt,
+                   DominanceFrontier &df, const TargetData &td,
+                   AliasSetTracker *ast)
+      : Allocas(A), RetryList(Retry), DT(dt), DF(df), TD(td), AST(ast) {}
 
     void run();
 
+    /// properlyDominates - Return true if I1 properly dominates I2.
+    ///
+    bool properlyDominates(Instruction *I1, Instruction *I2) const {
+      if (InvokeInst *II = dyn_cast<InvokeInst>(I1))
+        I1 = II->getNormalDest()->begin();
+      return DT[I1->getParent()]->properlyDominates(DT[I2->getParent()]);
+    }
+    
+    /// dominates - Return true if BB1 dominates BB2 using the DominatorTree.
+    ///
+    bool dominates(BasicBlock *BB1, BasicBlock *BB2) const {
+      return DT[BB1]->dominates(DT[BB2]);
+    }
+
   private:
     void MarkDominatingPHILive(BasicBlock *BB, unsigned AllocaNum,
                                std::set<PHINode*> &DeadPHINodes);
-    void PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI);
-    void PromoteLocallyUsedAllocas(BasicBlock *BB, 
+    bool PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI);
+    void PromoteLocallyUsedAllocas(BasicBlock *BB,
                                    const std::vector<AllocaInst*> &AIs);
 
     void RenamePass(BasicBlock *BB, BasicBlock *Pred,
@@ -167,6 +136,7 @@ void PromoteMem2Reg::run() {
   // that are live in a single basic block by the basic block they are live in.
   std::map<BasicBlock*, std::vector<AllocaInst*> > LocallyUsedAllocas;
 
+  if (AST) PointerAllocaValues.resize(Allocas.size());
 
   for (unsigned AllocaNum = 0; AllocaNum != Allocas.size(); ++AllocaNum) {
     AllocaInst *AI = Allocas[AllocaNum];
@@ -178,7 +148,8 @@ void PromoteMem2Reg::run() {
 
     if (AI->use_empty()) {
       // If there are no uses of the alloca, just delete it now.
-      AI->getParent()->getInstList().erase(AI);
+      if (AST) AST->deleteValue(AI);
+      AI->eraseFromParent();
 
       // Remove the alloca from the Allocas list, since it has been processed
       Allocas[AllocaNum] = Allocas.back();
@@ -192,76 +163,26 @@ void PromoteMem2Reg::run() {
     std::vector<BasicBlock*> DefiningBlocks;
     std::vector<BasicBlock*> UsingBlocks;
 
+    StoreInst  *OnlyStore = 0;
     BasicBlock *OnlyBlock = 0;
     bool OnlyUsedInOneBlock = true;
 
     // As we scan the uses of the alloca instruction, keep track of stores, and
     // decide whether all of the loads and stores to the alloca are within the
     // same basic block.
-  RestartUseScan:
+    Value *AllocaPointerVal = 0;
     for (Value::use_iterator U =AI->use_begin(), E = AI->use_end(); 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
         DefiningBlocks.push_back(SI->getParent());
-      } else if (LoadInst *LI = dyn_cast<LoadInst>(User)) {
+        AllocaPointerVal = SI->getOperand(0);
+        OnlyStore = SI;
+      } else {
+        LoadInst *LI = cast<LoadInst>(User);
         // Otherwise it must be a load instruction, keep track of variable reads
         UsingBlocks.push_back(LI->getParent());
-      } else if (SelectInst *SI = dyn_cast<SelectInst>(User)) {
-        // Because of the restrictions we placed on Select instruction uses
-        // above things are very simple.  Transform the PHI of addresses into a
-        // select of loaded values.
-        LoadInst *Load = cast<LoadInst>(SI->use_back());
-        std::string LoadName = Load->getName(); Load->setName("");
-
-        Value *TrueVal = new LoadInst(SI->getOperand(1), 
-                                      SI->getOperand(1)->getName()+".val", SI);
-        Value *FalseVal = new LoadInst(SI->getOperand(2), 
-                                       SI->getOperand(2)->getName()+".val", SI);
-
-        Value *NewSI = new SelectInst(SI->getOperand(0), TrueVal,
-                                      FalseVal, Load->getName(), SI);
-        Load->replaceAllUsesWith(NewSI);
-        Load->getParent()->getInstList().erase(Load);
-        SI->getParent()->getInstList().erase(SI);
-
-        // Restart our scan of uses...
-        DefiningBlocks.clear();
-        UsingBlocks.clear();
-        goto RestartUseScan;
-      } else {
-        // Because of the restrictions we placed on PHI node uses above, the PHI
-        // node reads the block in any using predecessors.  Transform the PHI of
-        // addresses into a PHI of loaded values.
-        PHINode *PN = cast<PHINode>(User);
-        assert(PN->hasOneUse() && "Cannot handle PHI Node with != 1 use!");
-        LoadInst *PNUser = cast<LoadInst>(PN->use_back());
-        std::string PNUserName = PNUser->getName(); PNUser->setName("");
-
-        // Create the new PHI node and insert load instructions as appropriate.
-        PHINode *NewPN = new PHINode(AI->getAllocatedType(), PNUserName, PN);
-        std::map<BasicBlock*, LoadInst*> NewLoads;
-        for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
-          BasicBlock *Pred = PN->getIncomingBlock(i);
-          LoadInst *&NewLoad = NewLoads[Pred];
-          if (NewLoad == 0)  // Insert the new load in the predecessor
-            NewLoad = new LoadInst(PN->getIncomingValue(i),
-                                   PN->getIncomingValue(i)->getName()+".val",
-                                   Pred->getTerminator());
-          NewPN->addIncoming(NewLoad, Pred);
-        }
-
-        // Remove the old load.
-        PNUser->replaceAllUsesWith(NewPN);
-        PNUser->getParent()->getInstList().erase(PNUser);
-
-        // Remove the old PHI node.
-        PN->getParent()->getInstList().erase(PN);
-
-        // Restart our scan of uses...
-        DefiningBlocks.clear();
-        UsingBlocks.clear();
-        goto RestartUseScan;
+        AllocaPointerVal = LI;
       }
 
       if (OnlyUsedInOneBlock) {
@@ -284,6 +205,63 @@ void PromoteMem2Reg::run() {
       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 (DefiningBlocks.size() == 1) {
+      // Be aware of loads before the store.
+      std::set<BasicBlock*> ProcessedBlocks;
+      for (unsigned i = 0, e = UsingBlocks.size(); i != e; ++i)
+        // If the store dominates the block and if we haven't processed it yet,
+        // do so now.
+        if (dominates(OnlyStore->getParent(), UsingBlocks[i]))
+          if (ProcessedBlocks.insert(UsingBlocks[i]).second) {
+            BasicBlock *UseBlock = UsingBlocks[i];
+            
+            // If the use and store are in the same block, do a quick scan to
+            // verify that there are no uses before the store.
+            if (UseBlock == OnlyStore->getParent()) {
+              BasicBlock::iterator I = UseBlock->begin();
+              for (; &*I != OnlyStore; ++I) { // scan block for store.
+                if (isa<LoadInst>(I) && I->getOperand(0) == AI)
+                  break;
+              }
+              if (&*I != OnlyStore) break;  // Do not handle this case.
+            }
+        
+            // Otherwise, if this is a different block or if all uses happen
+            // after the store, do a simple linear scan to replace loads with
+            // the stored value.
+            for (BasicBlock::iterator I = UseBlock->begin(),E = UseBlock->end();
+                 I != E; ) {
+              if (LoadInst *LI = dyn_cast<LoadInst>(I++)) {
+                if (LI->getOperand(0) == AI) {
+                  LI->replaceAllUsesWith(OnlyStore->getOperand(0));
+                  if (AST && isa<PointerType>(LI->getType()))
+                    AST->deleteValue(LI);
+                  LI->eraseFromParent();
+                }
+              }
+            }
+            
+            // Finally, remove this block from the UsingBlock set.
+            UsingBlocks[i] = UsingBlocks.back();
+            --i; --e;
+          }
+
+      // Finally, after the scan, check to see if the store is all that is left.
+      if (UsingBlocks.empty()) {
+        // The alloca has been processed, move on.
+        Allocas[AllocaNum] = Allocas.back();
+        Allocas.pop_back();
+        --AllocaNum;
+        continue;
+      }
+    }
+    
+    
+    if (AST)
+      PointerAllocaValues[AllocaNum] = AllocaPointerVal;
+
     // If we haven't computed a numbering for the BB's in the function, do so
     // now.
     BBNumbers.compute(F);
@@ -308,8 +286,8 @@ void PromoteMem2Reg::run() {
         // (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::iterator P = S.begin(),PE = S.end();
-             P != PE; ++P)
+        for (DominanceFrontier::DomSetType::const_iterator P = S.begin(),
+             PE = S.end(); P != PE; ++P)
           DFBlocks.push_back(BBNumbers.getNumber(*P));
 
         // Sort by which the block ordering in the function.
@@ -353,25 +331,33 @@ void PromoteMem2Reg::run() {
       if (!HasOtherPHIs)
         NewPhiNodes.erase(PN->getParent());
 
-      PN->getParent()->getInstList().erase(PN);      
+      if (AST && isa<PointerType>(PN->getType()))
+        AST->deleteValue(PN);
+      PN->eraseFromParent();
     }
 
-    // Keep the reverse mapping of the 'Allocas' array. 
+    // Keep the reverse mapping of the 'Allocas' array.
     AllocaLookup[Allocas[AllocaNum]] = AllocaNum;
   }
-  
+
   // Process all allocas which are only used in a single basic block.
   for (std::map<BasicBlock*, std::vector<AllocaInst*> >::iterator I =
          LocallyUsedAllocas.begin(), E = LocallyUsedAllocas.end(); I != E; ++I){
-    const std::vector<AllocaInst*> &Allocas = I->second;
-    assert(!Allocas.empty() && "empty alloca list??");
+    const std::vector<AllocaInst*> &LocAllocas = I->second;
+    assert(!LocAllocas.empty() && "empty alloca list??");
 
     // It's common for there to only be one alloca in the list.  Handle it
     // efficiently.
-    if (Allocas.size() == 1)
-      PromoteLocallyUsedAlloca(I->first, Allocas[0]);
-    else
-      PromoteLocallyUsedAllocas(I->first, Allocas);
+    if (LocAllocas.size() == 1) {
+      // If we can do the quick promotion pass, do so now.
+      if (PromoteLocallyUsedAlloca(I->first, LocAllocas[0]))
+        RetryList.push_back(LocAllocas[0]);  // Failed, retry later.
+    } else {
+      // Locally promote anything possible.  Note that if this is unable to
+      // promote a particular alloca, it puts the alloca onto the Allocas vector
+      // for global processing.
+      PromoteLocallyUsedAllocas(I->first, LocAllocas);
+    }
   }
 
   if (Allocas.empty())
@@ -383,7 +369,7 @@ void PromoteMem2Reg::run() {
   //
   std::vector<Value *> Values(Allocas.size());
   for (unsigned i = 0, e = Allocas.size(); i != e; ++i)
-    Values[i] = Constant::getNullValue(Allocas[i]->getAllocatedType());
+    Values[i] = UndefValue::get(Allocas[i]->getAllocatedType());
 
   // Walks all basic blocks in the function performing the SSA rename algorithm
   // and inserting the phi nodes we marked as necessary
@@ -393,7 +379,7 @@ void PromoteMem2Reg::run() {
   // The renamer uses the Visited set to avoid infinite loops.  Clear it now.
   Visited.clear();
 
-  // Remove the allocas themselves from the function...
+  // Remove the allocas themselves from the function.
   for (unsigned i = 0, e = Allocas.size(); i != e; ++i) {
     Instruction *A = Allocas[i];
 
@@ -402,44 +388,79 @@ void PromoteMem2Reg::run() {
     // Just delete the users now.
     //
     if (!A->use_empty())
-      A->replaceAllUsesWith(Constant::getNullValue(A->getType()));
-    A->getParent()->getInstList().erase(A);
+      A->replaceAllUsesWith(UndefValue::get(A->getType()));
+    if (AST) AST->deleteValue(A);
+    A->eraseFromParent();
   }
 
+  
+  // Loop over all of the PHI nodes and see if there are any that we can get
+  // rid of because they merge all of the same incoming values.  This can
+  // happen due to undef values coming into the PHI nodes.  This process is
+  // iterative, because eliminating one PHI node can cause others to be removed.
+  bool EliminatedAPHI = true;
+  while (EliminatedAPHI) {
+    EliminatedAPHI = false;
+    
+    for (std::map<BasicBlock*, std::vector<PHINode *> >::iterator I =
+           NewPhiNodes.begin(), E = NewPhiNodes.end(); I != E; ++I) {
+      std::vector<PHINode*> &PNs = I->second;
+      for (unsigned i = 0, e = PNs.size(); i != e; ++i) {
+        if (!PNs[i]) continue;
+
+        // If this PHI node merges  one value and/or undefs, get the value.
+        if (Value *V = PNs[i]->hasConstantValue(true)) {
+          if (!isa<Instruction>(V) ||
+              properlyDominates(cast<Instruction>(V), PNs[i])) {
+            if (AST && isa<PointerType>(PNs[i]->getType()))
+              AST->deleteValue(PNs[i]);
+            PNs[i]->replaceAllUsesWith(V);
+            PNs[i]->eraseFromParent();
+            PNs[i] = 0;
+            EliminatedAPHI = true;
+            continue;
+          }
+        }
+      }
+    }
+  }
+  
   // At this point, the renamer has added entries to PHI nodes for all reachable
   // code.  Unfortunately, there may be blocks which are not reachable, which
   // the renamer hasn't traversed.  If this is the case, the PHI nodes may not
   // have incoming values for all predecessors.  Loop over all PHI nodes we have
-  // created, inserting null constants if they are missing any incoming values.
+  // created, inserting undef values if they are missing any incoming values.
   //
-  for (std::map<BasicBlock*, std::vector<PHINode *> >::iterator I = 
+  for (std::map<BasicBlock*, std::vector<PHINode *> >::iterator I =
          NewPhiNodes.begin(), E = NewPhiNodes.end(); I != E; ++I) {
 
     std::vector<BasicBlock*> Preds(pred_begin(I->first), pred_end(I->first));
     std::vector<PHINode*> &PNs = I->second;
     assert(!PNs.empty() && "Empty PHI node list??");
+    PHINode *SomePHI = 0;
+    for (unsigned i = 0, e = PNs.size(); i != e; ++i)
+      if (PNs[i]) {
+        SomePHI = PNs[i];
+        break;
+      }
 
     // 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 PHI node.
-    PHINode *FirstPHI;
-    for (unsigned i = 0; (FirstPHI = PNs[i]) == 0; ++i)
-      /*empty*/;
-
-    if (Preds.size() != FirstPHI->getNumIncomingValues()) {
+    if (SomePHI && Preds.size() != SomePHI->getNumIncomingValues()) {
       // 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
       // access.
       std::sort(Preds.begin(), Preds.end());
 
-      // Now we loop through all BB's which have entries in FirstPHI and remove
+      // Now we loop through all BB's which have entries in SomePHI and remove
       // them from the Preds list.
-      for (unsigned i = 0, e = FirstPHI->getNumIncomingValues(); i != e; ++i) {
+      for (unsigned i = 0, e = SomePHI->getNumIncomingValues(); i != e; ++i) {
         // Do a log(n) search of the Preds list for the entry we want.
         std::vector<BasicBlock*>::iterator EntIt =
           std::lower_bound(Preds.begin(), Preds.end(),
-                           FirstPHI->getIncomingBlock(i));
-        assert(EntIt != Preds.end() && *EntIt == FirstPHI->getIncomingBlock(i)&&
+                           SomePHI->getIncomingBlock(i));
+        assert(EntIt != Preds.end() && *EntIt == SomePHI->getIncomingBlock(i)&&
                "PHI node has entry for a block which is not a predecessor!");
 
         // Remove the entry
@@ -450,9 +471,9 @@ void PromoteMem2Reg::run() {
       // entries inserted into every PHI nodes for the block.
       for (unsigned i = 0, e = PNs.size(); i != e; ++i)
         if (PHINode *PN = PNs[i]) {
-          Value *NullVal = Constant::getNullValue(PN->getType());
+          Value *UndefVal = UndefValue::get(PN->getType());
           for (unsigned pred = 0, e = Preds.size(); pred != e; ++pred)
-            PN->addIncoming(NullVal, Preds[pred]);
+            PN->addIncoming(UndefVal, Preds[pred]);
         }
     }
   }
@@ -500,7 +521,16 @@ void PromoteMem2Reg::MarkDominatingPHILive(BasicBlock *BB, unsigned AllocaNum,
 /// potentially useless PHI nodes by just performing a single linear pass over
 /// the basic block using the Alloca.
 ///
-void PromoteMem2Reg::PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI) {
+/// If we cannot promote this alloca (because it is read before it is written),
+/// return true.  This is necessary in cases where, due to control flow, the
+/// alloca is potentially undefined on some control flow paths.  e.g. code like
+/// this is potentially correct:
+///
+///   for (...) { if (c) { A = undef; undef = B; } }
+///
+/// ... so long as A is not used before undef is set.
+///
+bool PromoteMem2Reg::PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI) {
   assert(!AI->use_empty() && "There are no uses of the alloca!");
 
   // Handle degenerate cases quickly.
@@ -508,22 +538,28 @@ void PromoteMem2Reg::PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI) {
     Instruction *U = cast<Instruction>(AI->use_back());
     if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
       // Must be a load of uninitialized value.
-      LI->replaceAllUsesWith(Constant::getNullValue(AI->getAllocatedType()));
+      LI->replaceAllUsesWith(UndefValue::get(AI->getAllocatedType()));
+      if (AST && isa<PointerType>(LI->getType()))
+        AST->deleteValue(LI);
     } else {
       // Otherwise it must be a store which is never read.
       assert(isa<StoreInst>(U));
     }
     BB->getInstList().erase(U);
   } else {
-    // Uses of the uninitialized memory location shall get zero...
-    Value *CurVal = Constant::getNullValue(AI->getAllocatedType());
-  
+    // Uses of the uninitialized memory location shall get undef.
+    Value *CurVal = 0;
+
     for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E; ) {
       Instruction *Inst = I++;
       if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
         if (LI->getOperand(0) == AI) {
+          if (!CurVal) return true;  // Could not locally promote!
+
           // Loads just returns the "current value"...
           LI->replaceAllUsesWith(CurVal);
+          if (AST && isa<PointerType>(LI->getType()))
+            AST->deleteValue(LI);
           BB->getInstList().erase(LI);
         }
       } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
@@ -539,7 +575,9 @@ void PromoteMem2Reg::PromoteLocallyUsedAlloca(BasicBlock *BB, AllocaInst *AI) {
   // After traversing the basic block, there should be no more uses of the
   // alloca, remove it now.
   assert(AI->use_empty() && "Uses of alloca from more than one BB??");
+  if (AST) AST->deleteValue(AI);
   AI->getParent()->getInstList().erase(AI);
+  return false;
 }
 
 /// PromoteLocallyUsedAllocas - This method is just like
@@ -560,11 +598,19 @@ PromoteLocallyUsedAllocas(BasicBlock *BB, const std::vector<AllocaInst*> &AIs) {
       if (AllocaInst *AI = dyn_cast<AllocaInst>(LI->getOperand(0))) {
         std::map<AllocaInst*, Value*>::iterator AIt = CurValues.find(AI);
         if (AIt != CurValues.end()) {
-          // Loads just returns the "current value"...
-          if (AIt->second == 0)   // Uninitialized value??
-            AIt->second =Constant::getNullValue(AIt->first->getAllocatedType());
-          LI->replaceAllUsesWith(AIt->second);
-          BB->getInstList().erase(LI);
+          // If loading an uninitialized value, allow the inter-block case to
+          // handle it.  Due to control flow, this might actually be ok.
+          if (AIt->second == 0) {  // Use of locally uninitialized value??
+            RetryList.push_back(AI);   // Retry elsewhere.
+            CurValues.erase(AIt);   // Stop tracking this here.
+            if (CurValues.empty()) return;
+          } else {
+            // Loads just returns the "current value"...
+            LI->replaceAllUsesWith(AIt->second);
+            if (AST && isa<PointerType>(LI->getType()))
+              AST->deleteValue(LI);
+            BB->getInstList().erase(LI);
+          }
         }
       }
     } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
@@ -588,7 +634,7 @@ PromoteLocallyUsedAllocas(BasicBlock *BB, const std::vector<AllocaInst*> &AIs) {
 bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo,
                                   unsigned &Version,
                                   std::set<PHINode*> &InsertedPHINodes) {
-  // Look up the basic-block in question
+  // Look up the basic-block in question.
   std::vector<PHINode*> &BBPNs = NewPhiNodes[BB];
   if (BBPNs.empty()) BBPNs.resize(Allocas.size());
 
@@ -597,10 +643,15 @@ bool PromoteMem2Reg::QueuePhiNode(BasicBlock *BB, unsigned AllocaNo,
 
   // Create a PhiNode using the dereferenced type... and add the phi-node to the
   // BasicBlock.
-  BBPNs[AllocaNo] = new PHINode(Allocas[AllocaNo]->getAllocatedType(),
-                                Allocas[AllocaNo]->getName() + "." +
+  PHINode *PN = new PHINode(Allocas[AllocaNo]->getAllocatedType(),
+                            Allocas[AllocaNo]->getName() + "." +
                                         utostr(Version++), BB->begin());
-  InsertedPHINodes.insert(BBPNs[AllocaNo]);
+  BBPNs[AllocaNo] = PN;
+  InsertedPHINodes.insert(PN);
+
+  if (AST && isa<PointerType>(PN->getType()))
+    AST->copyValue(PointerAllocaValues[AllocaNo], PN);
+
   return true;
 }
 
@@ -630,7 +681,7 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
 
   // don't revisit nodes
   if (Visited.count(BB)) return;
-  
+
   // mark as visited
   Visited.insert(BB);
 
@@ -645,6 +696,8 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
 
           // 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);
         }
       }
@@ -675,10 +728,35 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
 /// use of DominanceFrontier information.  This function does not modify the CFG
 /// of the function at all.  All allocas must be from the same function.
 ///
+/// If AST is specified, the specified tracker is updated to reflect changes
+/// made to the IR.
+///
 void llvm::PromoteMemToReg(const std::vector<AllocaInst*> &Allocas,
                            DominatorTree &DT, DominanceFrontier &DF,
-                           const TargetData &TD) {
+                           const TargetData &TD, AliasSetTracker *AST) {
   // If there is nothing to do, bail out...
   if (Allocas.empty()) return;
-  PromoteMem2Reg(Allocas, DT, DF, TD).run();
+
+  SmallVector<AllocaInst*, 16> RetryList;
+  PromoteMem2Reg(Allocas, RetryList, DT, DF, TD, AST).run();
+
+  // PromoteMem2Reg may not have been able to promote all of the allocas in one
+  // pass, run it again if needed.
+  std::vector<AllocaInst*> NewAllocas;
+  while (!RetryList.empty()) {
+    // If we need to retry some allocas, this is due to there being no store
+    // before a read in a local block.  To counteract this, insert a store of
+    // undef into the alloca right after the alloca itself.
+    for (unsigned i = 0, e = RetryList.size(); i != e; ++i) {
+      BasicBlock::iterator BBI = RetryList[i];
+
+      new StoreInst(UndefValue::get(RetryList[i]->getAllocatedType()),
+                    RetryList[i], ++BBI);
+    }
+
+    NewAllocas.assign(RetryList.begin(), RetryList.end());
+    RetryList.clear();
+    PromoteMem2Reg(NewAllocas, RetryList, DT, DF, TD, AST).run();
+    NewAllocas.clear();
+  }
 }