enhance isRemovable to refuse to delete volatile mem transfers
[oota-llvm.git] / lib / Transforms / Scalar / DeadStoreElimination.cpp
index 45a553bf2f8494f4d8f09305bf884b0f3220baf1..d05f57f0d029c586eacbc6fa3f60fb3b1fbfa5c7 100644 (file)
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/Dominators.h"
+#include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/MemoryDependenceAnalysis.h"
 #include "llvm/Target/TargetData.h"
 #include "llvm/Transforms/Utils/Local.h"
-#include "llvm/Support/Compiler.h"
 using namespace llvm;
 
 STATISTIC(NumFastStores, "Number of stores deleted");
 STATISTIC(NumFastOther , "Number of other instrs removed");
 
 namespace {
-  struct VISIBILITY_HIDDEN DSE : public FunctionPass {
+  struct DSE : public FunctionPass {
+    TargetData *TD;
+
     static char ID; // Pass identification, replacement for typeid
-    DSE() : FunctionPass(&ID) {}
+    DSE() : FunctionPass(ID) {
+      initializeDSEPass(*PassRegistry::getPassRegistry());
+    }
 
     virtual bool runOnFunction(Function &F) {
       bool Changed = false;
+      
+      DominatorTree &DT = getAnalysis<DominatorTree>();
+      
       for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I)
-        Changed |= runOnBasicBlock(*I);
+        // Only check non-dead blocks.  Dead blocks may have strange pointer
+        // cycles that will confuse alias analysis.
+        if (DT.isReachableFromEntry(I))
+          Changed |= runOnBasicBlock(*I);
       return Changed;
     }
-
+    
     bool runOnBasicBlock(BasicBlock &BB);
-    bool handleFreeWithNonTrivialDependency(FreeInst *F, Instruction *Dep);
+    bool HandleFree(CallInst *F);
     bool handleEndBlock(BasicBlock &BB);
-    bool RemoveUndeadPointers(Value* pointer, uint64_t killPointerSize,
-                              BasicBlock::iteratorBBI,
-                              SmallPtrSet<Value*, 64>deadPointers);
+    bool RemoveUndeadPointers(Value *Ptr, uint64_t killPointerSize,
+                              BasicBlock::iterator &BBI,
+                              SmallPtrSet<Value*, 64> &deadPointers);
     void DeleteDeadInstruction(Instruction *I,
                                SmallPtrSet<Value*, 64> *deadPointers = 0);
     
@@ -62,148 +72,303 @@ namespace {
     virtual void getAnalysisUsage(AnalysisUsage &AU) const {
       AU.setPreservesCFG();
       AU.addRequired<DominatorTree>();
-      AU.addRequired<TargetData>();
       AU.addRequired<AliasAnalysis>();
       AU.addRequired<MemoryDependenceAnalysis>();
       AU.addPreserved<DominatorTree>();
-      AU.addPreserved<AliasAnalysis>();
       AU.addPreserved<MemoryDependenceAnalysis>();
     }
+
+    uint64_t getPointerSize(Value *V) const;
   };
 }
 
 char DSE::ID = 0;
-static RegisterPass<DSE> X("dse", "Dead Store Elimination");
+INITIALIZE_PASS_BEGIN(DSE, "dse", "Dead Store Elimination", false, false)
+INITIALIZE_PASS_DEPENDENCY(DominatorTree)
+INITIALIZE_PASS_DEPENDENCY(MemoryDependenceAnalysis)
+INITIALIZE_AG_DEPENDENCY(AliasAnalysis)
+INITIALIZE_PASS_END(DSE, "dse", "Dead Store Elimination", false, false)
 
 FunctionPass *llvm::createDeadStoreEliminationPass() { return new DSE(); }
 
-bool DSE::runOnBasicBlock(BasicBlock &BB) {
-  MemoryDependenceAnalysis& MD = getAnalysis<MemoryDependenceAnalysis>();
-  TargetData &TD = getAnalysis<TargetData>();  
+/// hasMemoryWrite - Does this instruction write some memory?  This only returns
+/// true for things that we can analyze with other helpers below.
+static bool hasMemoryWrite(Instruction *I) {
+  if (isa<StoreInst>(I))
+    return true;
+  if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
+    switch (II->getIntrinsicID()) {
+    default:
+      return false;
+    case Intrinsic::memset:
+    case Intrinsic::memmove:
+    case Intrinsic::memcpy:
+    case Intrinsic::init_trampoline:
+    case Intrinsic::lifetime_end:
+      return true;
+    }
+  }
+  return false;
+}
+
+/// getLocForWrite - Return a Location stored to by the specified instruction.
+static AliasAnalysis::Location
+getLocForWrite(Instruction *Inst, AliasAnalysis &AA) {
+  if (StoreInst *SI = dyn_cast<StoreInst>(Inst))
+    return AA.getLocation(SI);
+  
+  if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(Inst)) {
+    // memcpy/memmove/memset.
+    AliasAnalysis::Location Loc = AA.getLocationForDest(MI);
+    // If we don't have target data around, an unknown size in Location means
+    // that we should use the size of the pointee type.  This isn't valid for
+    // memset/memcpy, which writes more than an i8.
+    if (Loc.Size == AliasAnalysis::UnknownSize && AA.getTargetData() == 0)
+      return AliasAnalysis::Location();
+    return Loc;
+  }
+  
+  IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst);
+  if (II == 0) return AliasAnalysis::Location();
+  
+  switch (II->getIntrinsicID()) {
+  default: return AliasAnalysis::Location(); // Unhandled intrinsic.
+  case Intrinsic::init_trampoline:
+    // If we don't have target data around, an unknown size in Location means
+    // that we should use the size of the pointee type.  This isn't valid for
+    // init.trampoline, which writes more than an i8.
+    if (AA.getTargetData() == 0) return AliasAnalysis::Location();
+      
+    // FIXME: We don't know the size of the trampoline, so we can't really
+    // handle it here.
+    return AliasAnalysis::Location(II->getArgOperand(0));
+  case Intrinsic::lifetime_end: {
+    uint64_t Len = cast<ConstantInt>(II->getArgOperand(0))->getZExtValue();
+    return AliasAnalysis::Location(II->getArgOperand(1), Len);
+  }
+  }
+}
 
-  // Record the last-seen store to this pointer
-  DenseMap<Value*, StoreInst*> lastStore;
+/// isRemovable - If the value of this instruction and the memory it writes to
+/// is unused, may we delete this instruction?
+static bool isRemovable(Instruction *I) {
+  // Don't remove volatile stores.
+  if (StoreInst *SI = dyn_cast<StoreInst>(I))
+    return !SI->isVolatile();
   
+  IntrinsicInst *II = cast<IntrinsicInst>(I);
+  switch (II->getIntrinsicID()) {
+  default: assert(0 && "doesn't pass 'hasMemoryWrite' predicate");
+  case Intrinsic::lifetime_end:
+    // Never remove dead lifetime_end's, e.g. because it is followed by a
+    // free.
+    return false;
+  case Intrinsic::init_trampoline:
+    // Always safe to remove init_trampoline.
+    return true;
+    
+  case Intrinsic::memset:
+  case Intrinsic::memmove:
+  case Intrinsic::memcpy:
+    // Don't remove volatile memory intrinsics.
+    return !cast<MemIntrinsic>(II)->isVolatile();
+  }
+}
+
+/// getPointerOperand - Return the pointer that is being written to.
+static Value *getPointerOperand(Instruction *I) {
+  assert(hasMemoryWrite(I));
+  if (StoreInst *SI = dyn_cast<StoreInst>(I))
+    return SI->getPointerOperand();
+  if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(I))
+    return MI->getArgOperand(0);
+
+  IntrinsicInst *II = cast<IntrinsicInst>(I);
+  switch (II->getIntrinsicID()) {
+  default: assert(false && "Unexpected intrinsic!");
+  case Intrinsic::init_trampoline:
+    return II->getArgOperand(0);
+  case Intrinsic::lifetime_end:
+    return II->getArgOperand(1);
+  }
+}
+
+/// isCompleteOverwrite - Return true if a store to the 'Later' location
+/// completely overwrites a store to the 'Earlier' location.
+static bool isCompleteOverwrite(const AliasAnalysis::Location &Later,
+                                const AliasAnalysis::Location &Earlier,
+                                AliasAnalysis &AA, const TargetData *TD) {
+  const Value *P1 = Later.Ptr->stripPointerCasts();
+  const Value *P2 = Earlier.Ptr->stripPointerCasts();
+  
+  // Make sure that the start pointers are the same.
+  if (P1 != P2)
+    return false;
+
+  // If we have no TargetData information around, then the size of the store is
+  // inferrable from the pointee type.  If they are the same type, then we know
+  // that the store is safe.
+  if (TD == 0)
+    return Later.Ptr->getType() == Earlier.Ptr->getType();
+  
+  
+  // Make sure that the Later size is >= the Earlier size.
+  if (Later.Size < Earlier.Size)
+    return false;
+  
+  return true;
+}
+
+bool DSE::runOnBasicBlock(BasicBlock &BB) {
+  MemoryDependenceAnalysis &MD = getAnalysis<MemoryDependenceAnalysis>();
+  AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
+  TD = getAnalysisIfAvailable<TargetData>();
+
   bool MadeChange = false;
   
-  // Do a top-down walk on the BB
-  for (BasicBlock::iterator BBI = BB.begin(), BBE = BB.end();
-       BBI != BBE; ++BBI) {
-    // If we find a store or a free...
-    if (!isa<StoreInst>(BBI) && !isa<FreeInst>(BBI))
+  // Do a top-down walk on the BB.
+  for (BasicBlock::iterator BBI = BB.begin(), BBE = BB.end(); BBI != BBE; ) {
+    Instruction *Inst = BBI++;
+    
+    // Handle 'free' calls specially.
+    if (CallInst *F = isFreeCall(Inst)) {
+      MadeChange |= HandleFree(F);
       continue;
-
-    Value* pointer = 0;
-    if (StoreInst* S = dyn_cast<StoreInst>(BBI)) {
-      if (S->isVolatile())
-        continue;
-      pointer = S->getPointerOperand();
-    } else {
-      pointer = cast<FreeInst>(BBI)->getPointerOperand();
     }
+    
+    // If we find something that writes memory, get its memory dependence.
+    if (!hasMemoryWrite(Inst))
+      continue;
 
-    pointer = pointer->stripPointerCasts();
-    StoreInst*& last = lastStore[pointer];
-    bool deletedStore = false;
-
-    // ... to a pointer that has been stored to before...
-    if (last) {
-      Instruction* dep = MD.getDependency(BBI);
-        
-      // ... and no other memory dependencies are between them....
-      while (dep != MemoryDependenceAnalysis::None &&
-             dep != MemoryDependenceAnalysis::NonLocal &&
-             isa<StoreInst>(dep)) {
-        if (dep != last ||
-             TD.getTypeStoreSize(last->getOperand(0)->getType()) >
-             TD.getTypeStoreSize(BBI->getOperand(0)->getType())) {
-          dep = MD.getDependency(BBI, dep);
+    MemDepResult InstDep = MD.getDependency(Inst);
+    
+    // Ignore non-local store liveness.
+    // FIXME: cross-block DSE would be fun. :)
+    if (InstDep.isNonLocal() || 
+        // Ignore self dependence, which happens in the entry block of the
+        // function.
+        InstDep.getInst() == Inst)
+      continue;
+     
+    // If we're storing the same value back to a pointer that we just
+    // loaded from, then the store can be removed.
+    if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
+      if (LoadInst *DepLoad = dyn_cast<LoadInst>(InstDep.getInst())) {
+        if (SI->getPointerOperand() == DepLoad->getPointerOperand() &&
+            SI->getOperand(0) == DepLoad && !SI->isVolatile()) {
+          // DeleteDeadInstruction can delete the current instruction.  Save BBI
+          // in case we need it.
+          WeakVH NextInst(BBI);
+          
+          DeleteDeadInstruction(SI);
+          
+          if (NextInst == 0)  // Next instruction deleted.
+            BBI = BB.begin();
+          else if (BBI != BB.begin())  // Revisit this instruction if possible.
+            --BBI;
+          ++NumFastStores;
+          MadeChange = true;
           continue;
         }
-        
+      }
+    }
+    
+    // Figure out what location is being stored to.
+    AliasAnalysis::Location Loc = getLocForWrite(Inst, AA);
+
+    // If we didn't get a useful location, fail.
+    if (Loc.Ptr == 0)
+      continue;
+    
+    while (!InstDep.isNonLocal()) {
+      // Get the memory clobbered by the instruction we depend on.  MemDep will
+      // skip any instructions that 'Loc' clearly doesn't interact with.  If we
+      // end up depending on a may- or must-aliased load, then we can't optimize
+      // away the store and we bail out.  However, if we depend on on something
+      // that overwrites the memory location we *can* potentially optimize it.
+      //
+      // Find out what memory location the dependant instruction stores.
+      Instruction *DepWrite = InstDep.getInst();
+      AliasAnalysis::Location DepLoc = getLocForWrite(DepWrite, AA);
+      // If we didn't get a useful location, or if it isn't a size, bail out.
+      if (DepLoc.Ptr == 0)
+        break;
+
+      // If we find a removable write that is completely obliterated by the
+      // store to 'Loc' then we can remove it.
+      if (isRemovable(DepWrite) && isCompleteOverwrite(Loc, DepLoc, AA, TD)) {
         // Delete the store and now-dead instructions that feed it.
-        DeleteDeadInstruction(last);
-        NumFastStores++;
-        deletedStore = true;
+        DeleteDeadInstruction(DepWrite);
+        ++NumFastStores;
         MadeChange = true;
+        
+        // DeleteDeadInstruction can delete the current instruction in loop
+        // cases, reset BBI.
+        BBI = Inst;
+        if (BBI != BB.begin())
+          --BBI;
         break;
       }
-    }
-    
-    // Handle frees whose dependencies are non-trivial.
-    if (FreeInst* F = dyn_cast<FreeInst>(BBI)) {
-      if (!deletedStore)
-        MadeChange |= handleFreeWithNonTrivialDependency(F,MD.getDependency(F));
       
-      // No known stores after the free
-      last = 0;
-    } else {
-      StoreInst* S = cast<StoreInst>(BBI);
+      // If this is a may-aliased store that is clobbering the store value, we
+      // can keep searching past it for another must-aliased pointer that stores
+      // to the same location.  For example, in:
+      //   store -> P
+      //   store -> Q
+      //   store -> P
+      // we can remove the first store to P even though we don't know if P and Q
+      // alias.
+      if (DepWrite == &BB.front()) break;
       
-      // If we're storing the same value back to a pointer that we just
-      // loaded from, then the store can be removed;
-      if (LoadInst* L = dyn_cast<LoadInst>(S->getOperand(0))) {
-        Instruction* dep = MD.getDependency(S);
-        DominatorTree& DT = getAnalysis<DominatorTree>();
+      // Can't look past this instruction if it might read 'Loc'.
+      if (AA.getModRefInfo(DepWrite, Loc) & AliasAnalysis::Ref)
+        break;
         
-        if (!S->isVolatile() && S->getParent() == L->getParent() &&
-            S->getPointerOperand() == L->getPointerOperand() &&
-            (dep == MemoryDependenceAnalysis::None ||
-             dep == MemoryDependenceAnalysis::NonLocal ||
-             DT.dominates(dep, L))) {
-          
-          // Avoid iterator invalidation.
-          BBI++;
-          DeleteDeadInstruction(S);
-          NumFastStores++;
-          MadeChange = true;
-        } else
-          // Update our most-recent-store map.
-          last = S;
-      } else
-        // Update our most-recent-store map.
-        last = S;
+      InstDep = MD.getPointerDependencyFrom(Loc, false, DepWrite, &BB);
     }
   }
   
-  // If this block ends in a return, unwind, unreachable, and eventually
-  // tailcall, then all allocas are dead at its end.
+  // If this block ends in a return, unwind, or unreachable, all allocas are
+  // dead at its end, which means stores to them are also dead.
   if (BB.getTerminator()->getNumSuccessors() == 0)
     MadeChange |= handleEndBlock(BB);
   
   return MadeChange;
 }
 
-/// handleFreeWithNonTrivialDependency - Handle frees of entire structures whose
-/// dependency is a store to a field of that structure.
-bool DSE::handleFreeWithNonTrivialDependency(FreeInst* F, Instruction* dep) {
-  TargetData &TD = getAnalysis<TargetData>();
+/// HandleFree - Handle frees of entire structures whose dependency is a store
+/// to a field of that structure.
+bool DSE::HandleFree(CallInst *F) {
   AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
-  
-  if (dep == MemoryDependenceAnalysis::None ||
-      dep == MemoryDependenceAnalysis::NonLocal)
-    return false;
-  
-  StoreInst* dependency = dyn_cast<StoreInst>(dep);
-  if (!dependency)
-    return false;
-  else if (dependency->isVolatile())
-    return false;
-  
-  Value* depPointer = dependency->getPointerOperand();
-  const Type* depType = dependency->getOperand(0)->getType();
-  unsigned depPointerSize = TD.getTypeStoreSize(depType);
+  MemoryDependenceAnalysis &MD = getAnalysis<MemoryDependenceAnalysis>();
 
-  // Check for aliasing
-  AliasAnalysis::AliasResult A = AA.alias(F->getPointerOperand(), ~0U,
-                                          depPointer, depPointerSize);
+  MemDepResult Dep = MD.getDependency(F);
+  do {
+    if (Dep.isNonLocal()) return false;
+    
+    Instruction *Dependency = Dep.getInst();
+    if (!hasMemoryWrite(Dependency) || !isRemovable(Dependency))
+      return false;
+  
+    Value *DepPointer = getPointerOperand(Dependency)->getUnderlyingObject();
 
-  if (A != AliasAnalysis::MustAlias)
-    return false;
+    // Check for aliasing.
+    if (AA.alias(F->getArgOperand(0), 1, DepPointer, 1) !=
+          AliasAnalysis::MustAlias)
+      return false;
+  
+    // DCE instructions only used to calculate that store
+    DeleteDeadInstruction(Dependency);
+    ++NumFastStores;
+
+    // Inst's old Dependency is now deleted. Compute the next dependency,
+    // which may also be dead, as in
+    //    s[0] = 0;
+    //    s[1] = 0; // This has just been deleted.
+    //    free(s);
+    Dep = MD.getDependency(F);
+  } while (!Dep.isNonLocal());
   
-  // DCE instructions only used to calculate that store
-  DeleteDeadInstruction(dependency);
-  NumFastStores++;
   return true;
 }
 
@@ -214,7 +379,6 @@ bool DSE::handleFreeWithNonTrivialDependency(FreeInst* F, Instruction* dep) {
 /// store i32 1, i32* %A
 /// ret void
 bool DSE::handleEndBlock(BasicBlock &BB) {
-  TargetData &TD = getAnalysis<TargetData>();
   AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
   
   bool MadeChange = false;
@@ -240,80 +404,68 @@ bool DSE::handleEndBlock(BasicBlock &BB) {
     --BBI;
     
     // If we find a store whose pointer is dead.
-    if (StoreInst* S = dyn_cast<StoreInst>(BBI)) {
-      if (!S->isVolatile()) {
+    if (hasMemoryWrite(BBI)) {
+      if (isRemovable(BBI)) {
         // See through pointer-to-pointer bitcasts
-        Value* pointerOperand = S->getPointerOperand()->getUnderlyingObject();
+        Value *pointerOperand = getPointerOperand(BBI)->getUnderlyingObject();
 
         // Alloca'd pointers or byval arguments (which are functionally like
         // alloca's) are valid candidates for removal.
         if (deadPointers.count(pointerOperand)) {
           // DCE instructions only used to calculate that store.
-          BBI++;
-          DeleteDeadInstruction(S, &deadPointers);
-          NumFastStores++;
+          Instruction *Dead = BBI;
+          ++BBI;
+          DeleteDeadInstruction(Dead, &deadPointers);
+          ++NumFastStores;
           MadeChange = true;
+          continue;
         }
       }
       
-      continue;
-    }
-    
-    // We can also remove memcpy's to local variables at the end of a function.
-    if (MemCpyInst *M = dyn_cast<MemCpyInst>(BBI)) {
-      Value *dest = M->getDest()->getUnderlyingObject();
-
-      if (deadPointers.count(dest)) {
-        BBI++;
-        DeleteDeadInstruction(M, &deadPointers);
-        NumFastOther++;
-        MadeChange = true;
+      // Because a memcpy or memmove is also a load, we can't skip it if we
+      // didn't remove it.
+      if (!isa<MemTransferInst>(BBI))
         continue;
-      }
-      
-      // Because a memcpy is also a load, we can't skip it if we didn't remove
-      // it.
     }
     
-    ValuekillPointer = 0;
-    uint64_t killPointerSize = ~0UL;
+    Value *killPointer = 0;
+    uint64_t killPointerSize = AliasAnalysis::UnknownSize;
     
     // If we encounter a use of the pointer, it is no longer considered dead
     if (LoadInst *L = dyn_cast<LoadInst>(BBI)) {
       // However, if this load is unused and not volatile, we can go ahead and
       // remove it, and not have to worry about it making our pointer undead!
       if (L->use_empty() && !L->isVolatile()) {
-        BBI++;
+        ++BBI;
         DeleteDeadInstruction(L, &deadPointers);
-        NumFastOther++;
+        ++NumFastOther;
         MadeChange = true;
         continue;
       }
       
       killPointer = L->getPointerOperand();
-    } else if (VAArgInstV = dyn_cast<VAArgInst>(BBI)) {
+    } else if (VAArgInst *V = dyn_cast<VAArgInst>(BBI)) {
       killPointer = V->getOperand(0);
-    } else if (isa<MemCpyInst>(BBI) &&
-               isa<ConstantInt>(cast<MemCpyInst>(BBI)->getLength())) {
-      killPointer = cast<MemCpyInst>(BBI)->getSource();
+    } else if (isa<MemTransferInst>(BBI) &&
+               isa<ConstantInt>(cast<MemTransferInst>(BBI)->getLength())) {
+      killPointer = cast<MemTransferInst>(BBI)->getSource();
       killPointerSize = cast<ConstantInt>(
-                            cast<MemCpyInst>(BBI)->getLength())->getZExtValue();
-    } else if (AllocaInstA = dyn_cast<AllocaInst>(BBI)) {
+                       cast<MemTransferInst>(BBI)->getLength())->getZExtValue();
+    } else if (AllocaInst *A = dyn_cast<AllocaInst>(BBI)) {
       deadPointers.erase(A);
       
       // Dead alloca's can be DCE'd when we reach them
       if (A->use_empty()) {
-        BBI++;
+        ++BBI;
         DeleteDeadInstruction(A, &deadPointers);
-        NumFastOther++;
+        ++NumFastOther;
         MadeChange = true;
       }
       
       continue;
-    } else if (CallSite::get(BBI).getInstruction() != 0) {
+    } else if (CallSite CS = cast<Value>(BBI)) {
       // If this call does not access memory, it can't
       // be undeadifying any of our pointers.
-      CallSite CS = CallSite::get(BBI);
       if (AA.doesNotAccessMemory(CS))
         continue;
       
@@ -332,26 +484,15 @@ bool DSE::handleEndBlock(BasicBlock &BB) {
           deadPointers.clear();
           return MadeChange;
         }
-
-        // Get size information for the alloca
-        unsigned pointerSize = ~0U;
-        if (AllocaInst* A = dyn_cast<AllocaInst>(*I)) {
-          if (ConstantInt* C = dyn_cast<ConstantInt>(A->getArraySize()))
-            pointerSize = C->getZExtValue() * \
-                          TD.getABITypeSize(A->getAllocatedType());
-        } else {
-          const PointerType* PT = cast<PointerType>(
-                                                 cast<Argument>(*I)->getType());
-          pointerSize = TD.getABITypeSize(PT->getElementType());
-        }
-
+        
         // See if the call site touches it
-        AliasAnalysis::ModRefResult A = AA.getModRefInfo(CS, *I, pointerSize);
+        AliasAnalysis::ModRefResult A = AA.getModRefInfo(CS, *I,
+                                                         getPointerSize(*I));
         
         if (A == AliasAnalysis::ModRef)
-          modRef++;
+          ++modRef;
         else
-          other++;
+          ++other;
         
         if (A == AliasAnalysis::ModRef || A == AliasAnalysis::Ref)
           dead.push_back(*I);
@@ -365,9 +506,9 @@ bool DSE::handleEndBlock(BasicBlock &BB) {
     } else if (isInstructionTriviallyDead(BBI)) {
       // For any non-memory-affecting non-terminators, DCE them as we reach them
       Instruction *Inst = BBI;
-      BBI++;
+      ++BBI;
       DeleteDeadInstruction(Inst, &deadPointers);
-      NumFastOther++;
+      ++NumFastOther;
       MadeChange = true;
       continue;
     }
@@ -387,12 +528,11 @@ bool DSE::handleEndBlock(BasicBlock &BB) {
 
 /// RemoveUndeadPointers - check for uses of a pointer that make it
 /// undead when scanning for dead stores to alloca's.
-bool DSE::RemoveUndeadPointers(ValuekillPointer, uint64_t killPointerSize,
+bool DSE::RemoveUndeadPointers(Value *killPointer, uint64_t killPointerSize,
                                BasicBlock::iterator &BBI,
-                               SmallPtrSet<Value*, 64>& deadPointers) {
-  TargetData &TD = getAnalysis<TargetData>();
+                               SmallPtrSet<Value*, 64> &deadPointers) {
   AliasAnalysis &AA = getAnalysis<AliasAnalysis>();
-                                  
+
   // If the kill pointer can be easily reduced to an alloca,
   // don't bother doing extraneous AA queries.
   if (deadPointers.count(killPointer)) {
@@ -407,32 +547,21 @@ bool DSE::RemoveUndeadPointers(Value* killPointer, uint64_t killPointerSize,
   bool MadeChange = false;
   
   SmallVector<Value*, 16> undead;
-    
+  
   for (SmallPtrSet<Value*, 64>::iterator I = deadPointers.begin(),
-      E = deadPointers.end(); I != E; ++I) {
-    // Get size information for the alloca.
-    unsigned pointerSize = ~0U;
-    if (AllocaInst* A = dyn_cast<AllocaInst>(*I)) {
-      if (ConstantInt* C = dyn_cast<ConstantInt>(A->getArraySize()))
-        pointerSize = C->getZExtValue() *
-                      TD.getABITypeSize(A->getAllocatedType());
-    } else {
-      const PointerType* PT = cast<PointerType>(cast<Argument>(*I)->getType());
-      pointerSize = TD.getABITypeSize(PT->getElementType());
-    }
-
+       E = deadPointers.end(); I != E; ++I) {
     // See if this pointer could alias it
-    AliasAnalysis::AliasResult A = AA.alias(*I, pointerSize,
+    AliasAnalysis::AliasResult A = AA.alias(*I, getPointerSize(*I),
                                             killPointer, killPointerSize);
 
     // If it must-alias and a store, we can delete it
     if (isa<StoreInst>(BBI) && A == AliasAnalysis::MustAlias) {
-      StoreInstS = cast<StoreInst>(BBI);
+      StoreInst *S = cast<StoreInst>(BBI);
 
       // Remove it!
-      BBI++;
+      ++BBI;
       DeleteDeadInstruction(S, &deadPointers);
-      NumFastStores++;
+      ++NumFastStores;
       MadeChange = true;
 
       continue;
@@ -464,9 +593,8 @@ void DSE::DeleteDeadInstruction(Instruction *I,
 
   // Before we touch this instruction, remove it from memdep!
   MemoryDependenceAnalysis &MDA = getAnalysis<MemoryDependenceAnalysis>();
-  while (!NowDeadInsts.empty()) {
-    Instruction *DeadInst = NowDeadInsts.back();
-    NowDeadInsts.pop_back();
+  do {
+    Instruction *DeadInst = NowDeadInsts.pop_back_val();
     
     ++NumFastOther;
     
@@ -490,5 +618,20 @@ void DSE::DeleteDeadInstruction(Instruction *I,
     DeadInst->eraseFromParent();
     
     if (ValueSet) ValueSet->erase(DeadInst);
+  } while (!NowDeadInsts.empty());
+}
+
+uint64_t DSE::getPointerSize(Value *V) const {
+  if (TD) {
+    if (AllocaInst *A = dyn_cast<AllocaInst>(V)) {
+      // Get size information for the alloca
+      if (ConstantInt *C = dyn_cast<ConstantInt>(A->getArraySize()))
+        return C->getZExtValue() * TD->getTypeAllocSize(A->getAllocatedType());
+    } else {
+      assert(isa<Argument>(V) && "Expected AllocaInst or Argument!");
+      const PointerType *PT = cast<PointerType>(V->getType());
+      return TD->getTypeAllocSize(PT->getElementType());
+    }
   }
+  return AliasAnalysis::UnknownSize;
 }