llvm_unreachable->llvm_unreachable(0), LLVM_UNREACHABLE->llvm_unreachable.
[oota-llvm.git] / lib / Transforms / Scalar / GVN.cpp
index 385de02322f1881ce8af3c5612111219ef0b9bfb..e7be98506b99f53e9a2092487a39536db7400615 100644 (file)
@@ -10,7 +10,7 @@
 // This pass performs global value numbering to eliminate fully redundant
 // instructions.  It also performs simple dead load elimination.
 //
-// Note that this pass does the value numbering itself, it does not use the
+// Note that this pass does the value numbering itself; it does not use the
 // ValueNumbering analysis passes.
 //
 //===----------------------------------------------------------------------===//
 #include "llvm/Constants.h"
 #include "llvm/DerivedTypes.h"
 #include "llvm/Function.h"
-#include "llvm/Instructions.h"
+#include "llvm/IntrinsicInst.h"
+#include "llvm/LLVMContext.h"
 #include "llvm/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/ADT/PostOrderIterator.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "llvm/Transforms/Utils/Local.h"
 #include <cstdio>
 using namespace llvm;
 
-STATISTIC(NumGVNInstr, "Number of instructions deleted");
-STATISTIC(NumGVNLoad, "Number of loads deleted");
-STATISTIC(NumGVNPRE, "Number of instructions PRE'd");
+STATISTIC(NumGVNInstr,  "Number of instructions deleted");
+STATISTIC(NumGVNLoad,   "Number of loads deleted");
+STATISTIC(NumGVNPRE,    "Number of instructions PRE'd");
 STATISTIC(NumGVNBlocks, "Number of blocks merged");
-STATISTIC(NumPRELoad, "Number of loads PRE'd");
+STATISTIC(NumPRELoad,   "Number of loads PRE'd");
 
 static cl::opt<bool> EnablePRE("enable-pre",
                                cl::init(true), cl::Hidden);
-cl::opt<bool> EnableLoadPRE("enable-load-pre"/*, cl::init(true)*/);
+static cl::opt<bool> EnableLoadPRE("enable-load-pre", cl::init(true));
 
 //===----------------------------------------------------------------------===//
 //                         ValueTable Class
@@ -58,7 +62,8 @@ cl::opt<bool> EnableLoadPRE("enable-load-pre"/*, cl::init(true)*/);
 /// two values.
 namespace {
   struct VISIBILITY_HIDDEN Expression {
-    enum ExpressionOpcode { ADD, SUB, MUL, UDIV, SDIV, FDIV, UREM, SREM, 
+    enum ExpressionOpcode { ADD, FADD, SUB, FSUB, MUL, FMUL,
+                            UDIV, SDIV, FDIV, UREM, SREM,
                             FREM, SHL, LSHR, ASHR, AND, OR, XOR, ICMPEQ, 
                             ICMPNE, ICMPUGT, ICMPUGE, ICMPULT, ICMPULE, 
                             ICMPSGT, ICMPSGE, ICMPSLT, ICMPSLE, FCMPOEQ, 
@@ -109,30 +114,7 @@ namespace {
     }
   
     bool operator!=(const Expression &other) const {
-      if (opcode != other.opcode)
-        return true;
-      else if (opcode == EMPTY || opcode == TOMBSTONE)
-        return false;
-      else if (type != other.type)
-        return true;
-      else if (function != other.function)
-        return true;
-      else if (firstVN != other.firstVN)
-        return true;
-      else if (secondVN != other.secondVN)
-        return true;
-      else if (thirdVN != other.thirdVN)
-        return true;
-      else {
-        if (varargs.size() != other.varargs.size())
-          return true;
-      
-        for (size_t i = 0; i < varargs.size(); ++i)
-          if (varargs[i] != other.varargs[i])
-            return true;
-    
-          return false;
-      }
+      return !(*this == other);
     }
   };
   
@@ -172,6 +154,7 @@ namespace {
       void setMemDep(MemoryDependenceAnalysis* M) { MD = M; }
       void setDomTree(DominatorTree* D) { DT = D; }
       uint32_t getNextUnusedValueNumber() { return nextValueNumber; }
+      void verifyRemoved(const Value *) const;
   };
 }
 
@@ -219,10 +202,13 @@ template <> struct DenseMapInfo<Expression> {
 Expression::ExpressionOpcode ValueTable::getOpcode(BinaryOperator* BO) {
   switch(BO->getOpcode()) {
   default: // THIS SHOULD NEVER HAPPEN
-    assert(0 && "Binary operator with unknown opcode?");
+    llvm_unreachable("Binary operator with unknown opcode?");
   case Instruction::Add:  return Expression::ADD;
+  case Instruction::FAdd: return Expression::FADD;
   case Instruction::Sub:  return Expression::SUB;
+  case Instruction::FSub: return Expression::FSUB;
   case Instruction::Mul:  return Expression::MUL;
+  case Instruction::FMul: return Expression::FMUL;
   case Instruction::UDiv: return Expression::UDIV;
   case Instruction::SDiv: return Expression::SDIV;
   case Instruction::FDiv: return Expression::FDIV;
@@ -239,10 +225,10 @@ Expression::ExpressionOpcode ValueTable::getOpcode(BinaryOperator* BO) {
 }
 
 Expression::ExpressionOpcode ValueTable::getOpcode(CmpInst* C) {
-  if (isa<ICmpInst>(C) || isa<VICmpInst>(C)) {
+  if (isa<ICmpInst>(C)) {
     switch (C->getPredicate()) {
     default:  // THIS SHOULD NEVER HAPPEN
-      assert(0 && "Comparison with unknown predicate?");
+      llvm_unreachable("Comparison with unknown predicate?");
     case ICmpInst::ICMP_EQ:  return Expression::ICMPEQ;
     case ICmpInst::ICMP_NE:  return Expression::ICMPNE;
     case ICmpInst::ICMP_UGT: return Expression::ICMPUGT;
@@ -254,32 +240,32 @@ Expression::ExpressionOpcode ValueTable::getOpcode(CmpInst* C) {
     case ICmpInst::ICMP_SLT: return Expression::ICMPSLT;
     case ICmpInst::ICMP_SLE: return Expression::ICMPSLE;
     }
-  }
-  assert((isa<FCmpInst>(C) || isa<VFCmpInst>(C)) && "Unknown compare");
-  switch (C->getPredicate()) {
-  default: // THIS SHOULD NEVER HAPPEN
-    assert(0 && "Comparison with unknown predicate?");
-  case FCmpInst::FCMP_OEQ: return Expression::FCMPOEQ;
-  case FCmpInst::FCMP_OGT: return Expression::FCMPOGT;
-  case FCmpInst::FCMP_OGE: return Expression::FCMPOGE;
-  case FCmpInst::FCMP_OLT: return Expression::FCMPOLT;
-  case FCmpInst::FCMP_OLE: return Expression::FCMPOLE;
-  case FCmpInst::FCMP_ONE: return Expression::FCMPONE;
-  case FCmpInst::FCMP_ORD: return Expression::FCMPORD;
-  case FCmpInst::FCMP_UNO: return Expression::FCMPUNO;
-  case FCmpInst::FCMP_UEQ: return Expression::FCMPUEQ;
-  case FCmpInst::FCMP_UGT: return Expression::FCMPUGT;
-  case FCmpInst::FCMP_UGE: return Expression::FCMPUGE;
-  case FCmpInst::FCMP_ULT: return Expression::FCMPULT;
-  case FCmpInst::FCMP_ULE: return Expression::FCMPULE;
-  case FCmpInst::FCMP_UNE: return Expression::FCMPUNE;
+  } else {
+    switch (C->getPredicate()) {
+    default: // THIS SHOULD NEVER HAPPEN
+      llvm_unreachable("Comparison with unknown predicate?");
+    case FCmpInst::FCMP_OEQ: return Expression::FCMPOEQ;
+    case FCmpInst::FCMP_OGT: return Expression::FCMPOGT;
+    case FCmpInst::FCMP_OGE: return Expression::FCMPOGE;
+    case FCmpInst::FCMP_OLT: return Expression::FCMPOLT;
+    case FCmpInst::FCMP_OLE: return Expression::FCMPOLE;
+    case FCmpInst::FCMP_ONE: return Expression::FCMPONE;
+    case FCmpInst::FCMP_ORD: return Expression::FCMPORD;
+    case FCmpInst::FCMP_UNO: return Expression::FCMPUNO;
+    case FCmpInst::FCMP_UEQ: return Expression::FCMPUEQ;
+    case FCmpInst::FCMP_UGT: return Expression::FCMPUGT;
+    case FCmpInst::FCMP_UGE: return Expression::FCMPUGE;
+    case FCmpInst::FCMP_ULT: return Expression::FCMPULT;
+    case FCmpInst::FCMP_ULE: return Expression::FCMPULE;
+    case FCmpInst::FCMP_UNE: return Expression::FCMPUNE;
+    }
   }
 }
 
 Expression::ExpressionOpcode ValueTable::getOpcode(CastInst* C) {
   switch(C->getOpcode()) {
   default: // THIS SHOULD NEVER HAPPEN
-    assert(0 && "Cast operator with unknown opcode?");
+    llvm_unreachable("Cast operator with unknown opcode?");
   case Instruction::Trunc:    return Expression::TRUNC;
   case Instruction::ZExt:     return Expression::ZEXT;
   case Instruction::SExt:     return Expression::SEXT;
@@ -461,30 +447,19 @@ uint32_t ValueTable::lookup_or_add(Value* V) {
       
       MemDepResult local_dep = MD->getDependency(C);
       
-      if (local_dep.isNone()) {
+      if (!local_dep.isDef() && !local_dep.isNonLocal()) {
         valueNumbering.insert(std::make_pair(V, nextValueNumber));
         return nextValueNumber++;
       }
-      
-      if (Instruction *LocalDepInst = local_dep.getInst()) {
-        if (!isa<CallInst>(LocalDepInst)) {
-          valueNumbering.insert(std::make_pair(V, nextValueNumber));
-          return nextValueNumber++;
-        }
-        
-        CallInst* local_cdep = cast<CallInst>(LocalDepInst);
-        
-        if (local_cdep->getCalledFunction() != C->getCalledFunction() ||
-            local_cdep->getNumOperands() != C->getNumOperands()) {
-          valueNumbering.insert(std::make_pair(V, nextValueNumber));
-          return nextValueNumber++;
-        }
+
+      if (local_dep.isDef()) {
+        CallInst* local_cdep = cast<CallInst>(local_dep.getInst());
         
-        if (!C->getCalledFunction()) { 
+        if (local_cdep->getNumOperands() != C->getNumOperands()) {
           valueNumbering.insert(std::make_pair(V, nextValueNumber));
           return nextValueNumber++;
         }
-        
+          
         for (unsigned i = 1; i < C->getNumOperands(); ++i) {
           uint32_t c_vn = lookup_or_add(C->getOperand(i));
           uint32_t cd_vn = lookup_or_add(local_cdep->getOperand(i));
@@ -498,10 +473,12 @@ uint32_t ValueTable::lookup_or_add(Value* V) {
         valueNumbering.insert(std::make_pair(V, v));
         return v;
       }
-      
 
+      // Non-local case.
       const MemoryDependenceAnalysis::NonLocalDepInfo &deps = 
-        MD->getNonLocalDependency(C);
+        MD->getNonLocalCallDependency(CallSite(C));
+      // FIXME: call/call dependencies for readonly calls should return def, not
+      // clobber!  Move the checking logic to MemDep!
       CallInst* cdep = 0;
       
       // Check to see if we have a single dominating call instruction that is
@@ -514,7 +491,7 @@ uint32_t ValueTable::lookup_or_add(Value* V) {
 
         // We don't handle non-depedencies.  If we already have a call, reject
         // instruction dependencies.
-        if (I->second.isNone() || cdep != 0) {
+        if (I->second.isClobber() || cdep != 0) {
           cdep = 0;
           break;
         }
@@ -535,12 +512,7 @@ uint32_t ValueTable::lookup_or_add(Value* V) {
         return nextValueNumber++;
       }
       
-      if (cdep->getCalledFunction() != C->getCalledFunction() ||
-          cdep->getNumOperands() != C->getNumOperands()) {
-        valueNumbering.insert(std::make_pair(V, nextValueNumber));
-        return nextValueNumber++;
-      }
-      if (!C->getCalledFunction()) { 
+      if (cdep->getNumOperands() != C->getNumOperands()) {
         valueNumbering.insert(std::make_pair(V, nextValueNumber));
         return nextValueNumber++;
       }
@@ -691,8 +663,17 @@ void ValueTable::erase(Value* V) {
   valueNumbering.erase(V);
 }
 
+/// verifyRemoved - Verify that the value is removed from all internal data
+/// structures.
+void ValueTable::verifyRemoved(const Value *V) const {
+  for (DenseMap<Value*, uint32_t>::iterator
+         I = valueNumbering.begin(), E = valueNumbering.end(); I != E; ++I) {
+    assert(I->first != V && "Inst still occurs in value numbering map!");
+  }
+}
+
 //===----------------------------------------------------------------------===//
-//                         GVN Pass
+//                                GVN Pass
 //===----------------------------------------------------------------------===//
 
 namespace {
@@ -736,15 +717,13 @@ namespace {
     // Helper fuctions
     // FIXME: eliminate or document these better
     bool processLoad(LoadInst* L,
-                     DenseMap<Value*, LoadInst*> &lastLoad,
                      SmallVectorImpl<Instruction*> &toErase);
     bool processInstruction(Instruction* I,
-                            DenseMap<Value*, LoadInst*>& lastSeenLoad,
                             SmallVectorImpl<Instruction*> &toErase);
     bool processNonLocalLoad(LoadInst* L,
                              SmallVectorImpl<Instruction*> &toErase);
-    bool processBlock(DomTreeNode* DTN);
-    Value *GetValueForBlock(BasicBlock *BB, LoadInst* orig,
+    bool processBlock(BasicBlock* BB);
+    Value *GetValueForBlock(BasicBlock *BB, Instruction* orig,
                             DenseMap<BasicBlock*, Value*> &Phis,
                             bool top_level = false);
     void dump(DenseMap<uint32_t, Value*>& d);
@@ -754,7 +733,9 @@ namespace {
     bool performPRE(Function& F);
     Value* lookupNumber(BasicBlock* BB, uint32_t num);
     bool mergeBlockIntoPredecessor(BasicBlock* BB);
+    Value* AttemptRedundancyElimination(Instruction* orig, unsigned valno);
     void cleanupGlobalSets();
+    void verifyRemoved(const Instruction *I) const;
   };
   
   char GVN::ID = 0;
@@ -805,7 +786,7 @@ bool GVN::isSafeReplacement(PHINode* p, Instruction* inst) {
 
 /// GetValueForBlock - Get the value to use within the specified basic block.
 /// available values are in Phis.
-Value *GVN::GetValueForBlock(BasicBlock *BB, LoadInst* orig,
+Value *GVN::GetValueForBlock(BasicBlock *BB, Instruction* orig,
                              DenseMap<BasicBlock*, Value*> &Phis,
                              bool top_level) { 
                                  
@@ -816,23 +797,30 @@ Value *GVN::GetValueForBlock(BasicBlock *BB, LoadInst* orig,
   // If the block is unreachable, just return undef, since this path
   // can't actually occur at runtime.
   if (!DT->isReachableFromEntry(BB))
-    return Phis[BB] = UndefValue::get(orig->getType());
+    return Phis[BB] = Context->getUndef(orig->getType());
   
-  BasicBlock* singlePred = BB->getSinglePredecessor();
-  if (singlePred) {
-    Value *ret = GetValueForBlock(singlePred, orig, Phis);
+  if (BasicBlock *Pred = BB->getSinglePredecessor()) {
+    Value *ret = GetValueForBlock(Pred, orig, Phis);
     Phis[BB] = ret;
     return ret;
   }
+
+  // Get the number of predecessors of this block so we can reserve space later.
+  // If there is already a PHI in it, use the #preds from it, otherwise count.
+  // Getting it from the PHI is constant time.
+  unsigned NumPreds;
+  if (PHINode *ExistingPN = dyn_cast<PHINode>(BB->begin()))
+    NumPreds = ExistingPN->getNumIncomingValues();
+  else
+    NumPreds = std::distance(pred_begin(BB), pred_end(BB));
   
   // Otherwise, the idom is the loop, so we need to insert a PHI node.  Do so
   // now, then get values to fill in the incoming values for the PHI.
   PHINode *PN = PHINode::Create(orig->getType(), orig->getName()+".rle",
                                 BB->begin());
-  PN->reserveOperandSpace(std::distance(pred_begin(BB), pred_end(BB)));
+  PN->reserveOperandSpace(NumPreds);
   
-  if (Phis.count(BB) == 0)
-    Phis.insert(std::make_pair(BB, PN));
+  Phis.insert(std::make_pair(BB, PN));
   
   // Fill in the incoming values for the block.
   for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {
@@ -846,11 +834,17 @@ Value *GVN::GetValueForBlock(BasicBlock *BB, LoadInst* orig,
   Value* v = CollapsePhi(PN);
   if (!v) {
     // Cache our phi construction results
-    phiMap[orig->getPointerOperand()].insert(PN);
+    if (LoadInst* L = dyn_cast<LoadInst>(orig))
+      phiMap[L->getPointerOperand()].insert(PN);
+    else
+      phiMap[orig].insert(PN);
+    
     return PN;
   }
     
   PN->replaceAllUsesWith(v);
+  if (isa<PointerType>(v->getType()))
+    MD->invalidateCachedPointerInfo(v);
 
   for (DenseMap<BasicBlock*, Value*>::iterator I = Phis.begin(),
        E = Phis.end(); I != E; ++I)
@@ -860,6 +854,7 @@ Value *GVN::GetValueForBlock(BasicBlock *BB, LoadInst* orig,
   DEBUG(cerr << "GVN removed: " << *PN);
   MD->removeInstruction(PN);
   PN->eraseFromParent();
+  DEBUG(verifyRemoved(PN));
 
   Phis[BB] = v;
   return v;
@@ -947,17 +942,27 @@ SpeculationFailure:
 bool GVN::processNonLocalLoad(LoadInst *LI,
                               SmallVectorImpl<Instruction*> &toErase) {
   // Find the non-local dependencies of the load.
-  const MemoryDependenceAnalysis::NonLocalDepInfo &deps = 
-    MD->getNonLocalDependency(LI);
-  //DEBUG(cerr << "INVESTIGATING NONLOCAL LOAD: " << deps.size() << *LI);
+  SmallVector<MemoryDependenceAnalysis::NonLocalDepEntry, 64> Deps; 
+  MD->getNonLocalPointerDependency(LI->getOperand(0), true, LI->getParent(),
+                                   Deps);
+  //DEBUG(cerr << "INVESTIGATING NONLOCAL LOAD: " << Deps.size() << *LI);
   
   // If we had to process more than one hundred blocks to find the
   // dependencies, this load isn't worth worrying about.  Optimizing
   // it will be too expensive.
-  if (deps.size() > 100)
+  if (Deps.size() > 100)
     return false;
-  
-  BasicBlock *EntryBlock = &LI->getParent()->getParent()->getEntryBlock();
+
+  // If we had a phi translation failure, we'll have a single entry which is a
+  // clobber in the current block.  Reject this early.
+  if (Deps.size() == 1 && Deps[0].second.isClobber()) {
+    DEBUG(
+      DOUT << "GVN: non-local load ";
+      WriteAsOperand(*DOUT.stream(), LI);
+      DOUT << " is clobbered by " << *Deps[0].second.getInst();
+    );
+    return false;
+  }
   
   // Filter out useless results (non-locals, etc).  Keep track of the blocks
   // where we have a value available in repl, also keep track of whether we see
@@ -966,26 +971,25 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
   SmallVector<std::pair<BasicBlock*, Value*>, 16> ValuesPerBlock;
   SmallVector<BasicBlock*, 16> UnavailableBlocks;
   
-  for (unsigned i = 0, e = deps.size(); i != e; ++i) {
-    BasicBlock *DepBB = deps[i].first;
-    MemDepResult DepInfo = deps[i].second;
+  for (unsigned i = 0, e = Deps.size(); i != e; ++i) {
+    BasicBlock *DepBB = Deps[i].first;
+    MemDepResult DepInfo = Deps[i].second;
     
-    if (DepInfo.isNonLocal()) {
-      // If this is a non-local dependency in the entry block, then we depend on
-      // the value live-in at the start of the function.  We could insert a load
-      // in the entry block to get this, but for now we'll just bail out.
-      if (DepBB == EntryBlock)
-        UnavailableBlocks.push_back(DepBB);
+    if (DepInfo.isClobber()) {
+      UnavailableBlocks.push_back(DepBB);
       continue;
     }
     
-    if (DepInfo.isNone()) {
+    Instruction *DepInst = DepInfo.getInst();
+    
+    // Loading the allocation -> undef.
+    if (isa<AllocationInst>(DepInst)) {
       ValuesPerBlock.push_back(std::make_pair(DepBB, 
-                                              UndefValue::get(LI->getType())));
+                                            Context->getUndef(LI->getType())));
       continue;
     }
   
-    if (StoreInst* S = dyn_cast<StoreInst>(DepInfo.getInst())) {
+    if (StoreInst* S = dyn_cast<StoreInst>(DepInst)) {
       // Reject loads and stores that are to the same address but are of 
       // different types.
       // NOTE: 403.gcc does have this case (e.g. in readonly_fields_p) because
@@ -996,28 +1000,13 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
         continue;
       }
       
-      if (S->getPointerOperand() != LI->getPointerOperand() &&
-          VN.getAliasAnalysis()->alias(S->getPointerOperand(), 1,
-                                       LI->getPointerOperand(), 1)
-            != AliasAnalysis::MustAlias) {
-        UnavailableBlocks.push_back(DepBB);
-        continue;
-      }
       ValuesPerBlock.push_back(std::make_pair(DepBB, S->getOperand(0)));
       
-    } else if (LoadInst* LD = dyn_cast<LoadInst>(DepInfo.getInst())) {
+    } else if (LoadInst* LD = dyn_cast<LoadInst>(DepInst)) {
       if (LD->getType() != LI->getType()) {
         UnavailableBlocks.push_back(DepBB);
         continue;
       }
-      
-      if (LD->getPointerOperand() != LI->getPointerOperand() &&
-          VN.getAliasAnalysis()->alias(LD->getPointerOperand(), 1,
-                                       LI->getPointerOperand(), 1)
-          != AliasAnalysis::MustAlias) {
-        UnavailableBlocks.push_back(DepBB);
-        continue;
-      }
       ValuesPerBlock.push_back(std::make_pair(DepBB, LD));
     } else {
       UnavailableBlocks.push_back(DepBB);
@@ -1035,11 +1024,14 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
   if (UnavailableBlocks.empty()) {
     // Use cached PHI construction information from previous runs
     SmallPtrSet<Instruction*, 4> &p = phiMap[LI->getPointerOperand()];
+    // FIXME: What does phiMap do? Are we positive it isn't getting invalidated?
     for (SmallPtrSet<Instruction*, 4>::iterator I = p.begin(), E = p.end();
          I != E; ++I) {
       if ((*I)->getParent() == LI->getParent()) {
         DEBUG(cerr << "GVN REMOVING NONLOCAL LOAD #1: " << *LI);
         LI->replaceAllUsesWith(*I);
+        if (isa<PointerType>((*I)->getType()))
+          MD->invalidateCachedPointerInfo(*I);
         toErase.push_back(LI);
         NumGVNLoad++;
         return true;
@@ -1055,6 +1047,11 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
     // Perform PHI construction.
     Value* v = GetValueForBlock(LI->getParent(), LI, BlockReplValues, true);
     LI->replaceAllUsesWith(v);
+    
+    if (isa<PHINode>(v))
+      v->takeName(LI);
+    if (isa<PointerType>(v->getType()))
+      MD->invalidateCachedPointerInfo(v);
     toErase.push_back(LI);
     NumGVNLoad++;
     return true;
@@ -1071,11 +1068,32 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
   // that we only have to insert *one* load (which means we're basically moving
   // the load, not inserting a new one).
   
-  // Everything we do here is based on local predecessors of LI's block.  If it
-  // only has one predecessor, bail now.
+  SmallPtrSet<BasicBlock *, 4> Blockers;
+  for (unsigned i = 0, e = UnavailableBlocks.size(); i != e; ++i)
+    Blockers.insert(UnavailableBlocks[i]);
+
+  // Lets find first basic block with more than one predecessor.  Walk backwards
+  // through predecessors if needed.
   BasicBlock *LoadBB = LI->getParent();
-  if (LoadBB->getSinglePredecessor())
-    return false;
+  BasicBlock *TmpBB = LoadBB;
+
+  bool isSinglePred = false;
+  bool allSingleSucc = true;
+  while (TmpBB->getSinglePredecessor()) {
+    isSinglePred = true;
+    TmpBB = TmpBB->getSinglePredecessor();
+    if (!TmpBB) // If haven't found any, bail now.
+      return false;
+    if (TmpBB == LoadBB) // Infinite (unreachable) loop.
+      return false;
+    if (Blockers.count(TmpBB))
+      return false;
+    if (TmpBB->getTerminator()->getNumSuccessors() != 1)
+      allSingleSucc = false;
+  }
+  
+  assert(TmpBB);
+  LoadBB = TmpBB;
   
   // If we have a repl set with LI itself in it, this means we have a loop where
   // at least one of the values is LI.  Since this means that we won't be able
@@ -1085,6 +1103,23 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
     if (ValuesPerBlock[i].second == LI)
       return false;
   
+  if (isSinglePred) {
+    bool isHot = false;
+    for (unsigned i = 0, e = ValuesPerBlock.size(); i != e; ++i)
+      if (Instruction *I = dyn_cast<Instruction>(ValuesPerBlock[i].second))
+       // "Hot" Instruction is in some loop (because it dominates its dep. 
+       // instruction).
+       if (DT->dominates(LI, I)) { 
+         isHot = true;
+         break;
+       }
+
+    // We are interested only in "hot" instructions. We don't want to do any
+    // mis-optimizations here.
+    if (!isHot)
+      return false;
+  }
+
   // Okay, we have some hope :).  Check to see if the loaded value is fully
   // available in all but one predecessor.
   // FIXME: If we could restructure the CFG, we could make a common pred with
@@ -1131,7 +1166,20 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
                 << UnavailablePred->getName() << "': " << *LI);
     return false;
   }
-  
+
+  // Make sure it is valid to move this load here.  We have to watch out for:
+  //  @1 = getelementptr (i8* p, ...
+  //  test p and branch if == 0
+  //  load @1
+  // It is valid to have the getelementptr before the test, even if p can be 0,
+  // as getelementptr only does address arithmetic.
+  // If we are not pushing the value through any multiple-successor blocks
+  // we do not have this case.  Otherwise, check that the load is safe to
+  // put anywhere; this can be improved, but should be conservatively safe.
+  if (!allSingleSucc &&
+      !isSafeToLoadUnconditionally(LoadPtr, UnavailablePred->getTerminator()))
+    return false;
+
   // Okay, we can eliminate this load by inserting a reload in the predecessor
   // and using PHI construction to get the value in the other predecessors, do
   // it.
@@ -1141,6 +1189,11 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
                                 LI->getAlignment(),
                                 UnavailablePred->getTerminator());
   
+  SmallPtrSet<Instruction*, 4> &p = phiMap[LI->getPointerOperand()];
+  for (SmallPtrSet<Instruction*, 4>::iterator I = p.begin(), E = p.end();
+       I != E; ++I)
+    ValuesPerBlock.push_back(std::make_pair((*I)->getParent(), *I));
+  
   DenseMap<BasicBlock*, Value*> BlockReplValues;
   BlockReplValues.insert(ValuesPerBlock.begin(), ValuesPerBlock.end());
   BlockReplValues[UnavailablePred] = NewLoad;
@@ -1148,7 +1201,10 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
   // Perform PHI construction.
   Value* v = GetValueForBlock(LI->getParent(), LI, BlockReplValues, true);
   LI->replaceAllUsesWith(v);
-  v->takeName(LI);
+  if (isa<PHINode>(v))
+    v->takeName(LI);
+  if (isa<PointerType>(v->getType()))
+    MD->invalidateCachedPointerInfo(v);
   toErase.push_back(LI);
   NumPRELoad++;
   return true;
@@ -1156,84 +1212,73 @@ bool GVN::processNonLocalLoad(LoadInst *LI,
 
 /// processLoad - Attempt to eliminate a load, first by eliminating it
 /// locally, and then attempting non-local elimination if that fails.
-bool GVN::processLoad(LoadInst *L, DenseMap<Value*, LoadInst*> &lastLoad,
-                      SmallVectorImpl<Instruction*> &toErase) {
-  if (L->isVolatile()) {
-    lastLoad[L->getPointerOperand()] = L;
+bool GVN::processLoad(LoadInst *L, SmallVectorImpl<Instruction*> &toErase) {
+  if (L->isVolatile())
     return false;
-  }
   
   Value* pointer = L->getPointerOperand();
-  LoadInst*& last = lastLoad[pointer];
-  
+
   // ... to a pointer that has been loaded from before...
-  bool removedNonLocal = false;
   MemDepResult dep = MD->getDependency(L);
-  if (dep.isNonLocal() &&
-      L->getParent() != &L->getParent()->getParent()->getEntryBlock()) {
-    removedNonLocal = processNonLocalLoad(L, toErase);
+  
+  // If the value isn't available, don't do anything!
+  if (dep.isClobber()) {
+    DEBUG(
+      // fast print dep, using operator<< on instruction would be too slow
+      DOUT << "GVN: load ";
+      WriteAsOperand(*DOUT.stream(), L);
+      Instruction *I = dep.getInst();
+      DOUT << " is clobbered by " << *I;
+    );
+    return false;
+  }
+
+  // If it is defined in another block, try harder.
+  if (dep.isNonLocal())
+    return processNonLocalLoad(L, toErase);
+
+  Instruction *DepInst = dep.getInst();
+  if (StoreInst *DepSI = dyn_cast<StoreInst>(DepInst)) {
+    // Only forward substitute stores to loads of the same type.
+    // FIXME: Could do better!
+    if (DepSI->getPointerOperand()->getType() != pointer->getType())
+      return false;
     
-    if (!removedNonLocal)
-      last = L;
+    // Remove it!
+    L->replaceAllUsesWith(DepSI->getOperand(0));
+    if (isa<PointerType>(DepSI->getOperand(0)->getType()))
+      MD->invalidateCachedPointerInfo(DepSI->getOperand(0));
+    toErase.push_back(L);
+    NumGVNLoad++;
+    return true;
+  }
+
+  if (LoadInst *DepLI = dyn_cast<LoadInst>(DepInst)) {
+    // Only forward substitute stores to loads of the same type.
+    // FIXME: Could do better! load i32 -> load i8 -> truncate on little endian.
+    if (DepLI->getType() != L->getType())
+      return false;
     
-    return removedNonLocal;
+    // Remove it!
+    L->replaceAllUsesWith(DepLI);
+    if (isa<PointerType>(DepLI->getType()))
+      MD->invalidateCachedPointerInfo(DepLI);
+    toErase.push_back(L);
+    NumGVNLoad++;
+    return true;
   }
   
-  
-  bool deletedLoad = false;
-  
-  // Walk up the dependency chain until we either find
-  // a dependency we can use, or we can't walk any further
-  while (Instruction *DepInst = dep.getInst()) {
-    // ... that depends on a store ...
-    if (StoreInst* S = dyn_cast<StoreInst>(DepInst)) {
-      if (S->getPointerOperand() == pointer) {
-        // Remove it!
-        L->replaceAllUsesWith(S->getOperand(0));
-        toErase.push_back(L);
-        deletedLoad = true;
-        NumGVNLoad++;
-      }
-      
-      // Whether we removed it or not, we can't
-      // go any further
-      break;
-    } else if (!isa<LoadInst>(DepInst)) {
-      // Only want to handle loads below.
-      break;
-    } else if (!last) {
-      // If we don't depend on a store, and we haven't
-      // been loaded before, bail.
-      break;
-    } else if (DepInst == last) {
-      // Remove it!
-      L->replaceAllUsesWith(last);
-      toErase.push_back(L);
-      deletedLoad = true;
-      NumGVNLoad++;
-      break;
-    } else {
-      dep = MD->getDependencyFrom(L, DepInst, DepInst->getParent());
-    }
-  }
-
   // If this load really doesn't depend on anything, then we must be loading an
   // undef value.  This can happen when loading for a fresh allocation with no
   // intervening stores, for example.
-  if (dep.isNone()) {
-    // If this load depends directly on an allocation, there isn't
-    // anything stored there; therefore, we can optimize this load
-    // to undef.
-    L->replaceAllUsesWith(UndefValue::get(L->getType()));
+  if (isa<AllocationInst>(DepInst)) {
+    L->replaceAllUsesWith(Context->getUndef(L->getType()));
     toErase.push_back(L);
-    deletedLoad = true;
     NumGVNLoad++;
+    return true;
   }
 
-  if (!deletedLoad)
-    last = L;
-  
-  return deletedLoad;
+  return false;
 }
 
 Value* GVN::lookupNumber(BasicBlock* BB, uint32_t num) {
@@ -1254,13 +1299,66 @@ Value* GVN::lookupNumber(BasicBlock* BB, uint32_t num) {
   return 0;
 }
 
+/// AttemptRedundancyElimination - If the "fast path" of redundancy elimination
+/// by inheritance from the dominator fails, see if we can perform phi 
+/// construction to eliminate the redundancy.
+Value* GVN::AttemptRedundancyElimination(Instruction* orig, unsigned valno) {
+  BasicBlock* BaseBlock = orig->getParent();
+  
+  SmallPtrSet<BasicBlock*, 4> Visited;
+  SmallVector<BasicBlock*, 8> Stack;
+  Stack.push_back(BaseBlock);
+  
+  DenseMap<BasicBlock*, Value*> Results;
+  
+  // Walk backwards through our predecessors, looking for instances of the
+  // value number we're looking for.  Instances are recorded in the Results
+  // map, which is then used to perform phi construction.
+  while (!Stack.empty()) {
+    BasicBlock* Current = Stack.back();
+    Stack.pop_back();
+    
+    // If we've walked all the way to a proper dominator, then give up. Cases
+    // where the instance is in the dominator will have been caught by the fast
+    // path, and any cases that require phi construction further than this are
+    // probably not worth it anyways.  Note that this is a SIGNIFICANT compile
+    // time improvement.
+    if (DT->properlyDominates(Current, orig->getParent())) return 0;
+    
+    DenseMap<BasicBlock*, ValueNumberScope*>::iterator LA =
+                                                       localAvail.find(Current);
+    if (LA == localAvail.end()) return 0;
+    DenseMap<uint32_t, Value*>::iterator V = LA->second->table.find(valno);
+    
+    if (V != LA->second->table.end()) {
+      // Found an instance, record it.
+      Results.insert(std::make_pair(Current, V->second));
+      continue;
+    }
+    
+    // If we reach the beginning of the function, then give up.
+    if (pred_begin(Current) == pred_end(Current))
+      return 0;
+    
+    for (pred_iterator PI = pred_begin(Current), PE = pred_end(Current);
+         PI != PE; ++PI)
+      if (Visited.insert(*PI))
+        Stack.push_back(*PI);
+  }
+  
+  // If we didn't find instances, give up.  Otherwise, perform phi construction.
+  if (Results.size() == 0)
+    return 0;
+  else
+    return GetValueForBlock(BaseBlock, orig, Results, true);
+}
+
 /// processInstruction - When calculating availability, handle an instruction
 /// by inserting it into the appropriate sets
 bool GVN::processInstruction(Instruction *I,
-                             DenseMap<Value*, LoadInst*> &lastSeenLoad,
                              SmallVectorImpl<Instruction*> &toErase) {
   if (LoadInst* L = dyn_cast<LoadInst>(I)) {
-    bool changed = processLoad(L, lastSeenLoad, toErase);
+    bool changed = processLoad(L, toErase);
     
     if (!changed) {
       unsigned num = VN.lookup_or_add(L);
@@ -1273,9 +1371,28 @@ bool GVN::processInstruction(Instruction *I,
   uint32_t nextNum = VN.getNextUnusedValueNumber();
   unsigned num = VN.lookup_or_add(I);
   
+  if (BranchInst* BI = dyn_cast<BranchInst>(I)) {
+    localAvail[I->getParent()]->table.insert(std::make_pair(num, I));
+    
+    if (!BI->isConditional() || isa<Constant>(BI->getCondition()))
+      return false;
+    
+    Value* branchCond = BI->getCondition();
+    uint32_t condVN = VN.lookup_or_add(branchCond);
+    
+    BasicBlock* trueSucc = BI->getSuccessor(0);
+    BasicBlock* falseSucc = BI->getSuccessor(1);
+    
+    if (trueSucc->getSinglePredecessor())
+      localAvail[trueSucc]->table[condVN] = Context->getConstantIntTrue();
+    if (falseSucc->getSinglePredecessor())
+      localAvail[falseSucc]->table[condVN] = Context->getConstantIntFalse();
+
+    return false;
+    
   // Allocations are always uniquely numbered, so we can save time and memory
-  // by fast failing them.
-  if (isa<AllocationInst>(I) || isa<TerminatorInst>(I)) {
+  // by fast failing them.  
+  } else if (isa<AllocationInst>(I) || isa<TerminatorInst>(I)) {
     localAvail[I->getParent()]->table.insert(std::make_pair(num, I));
     return false;
   }
@@ -1287,10 +1404,13 @@ bool GVN::processInstruction(Instruction *I,
     if (constVal) {
       for (PhiMapType::iterator PI = phiMap.begin(), PE = phiMap.end();
            PI != PE; ++PI)
-        if (PI->second.count(p))
-          PI->second.erase(p);
+        PI->second.erase(p);
         
       p->replaceAllUsesWith(constVal);
+      if (isa<PointerType>(constVal->getType()))
+        MD->invalidateCachedPointerInfo(constVal);
+      VN.erase(p);
+      
       toErase.push_back(p);
     } else {
       localAvail[I->getParent()]->table.insert(std::make_pair(num, I));
@@ -1302,13 +1422,28 @@ bool GVN::processInstruction(Instruction *I,
   } else if (num == nextNum) {
     localAvail[I->getParent()]->table.insert(std::make_pair(num, I));
     
-  // Perform value-number based elimination
+  // Perform fast-path value-number based elimination of values inherited from
+  // dominators.
   } else if (Value* repl = lookupNumber(I->getParent(), num)) {
     // Remove it!
     VN.erase(I);
     I->replaceAllUsesWith(repl);
+    if (isa<PointerType>(repl->getType()))
+      MD->invalidateCachedPointerInfo(repl);
+    toErase.push_back(I);
+    return true;
+
+#if 0
+  // Perform slow-pathvalue-number based elimination with phi construction.
+  } else if (Value* repl = AttemptRedundancyElimination(I, num)) {
+    // Remove it!
+    VN.erase(I);
+    I->replaceAllUsesWith(repl);
+    if (isa<PointerType>(repl->getType()))
+      MD->invalidateCachedPointerInfo(repl);
     toErase.push_back(I);
     return true;
+#endif
   } else {
     localAvail[I->getParent()]->table.insert(std::make_pair(num, I));
   }
@@ -1316,9 +1451,7 @@ bool GVN::processInstruction(Instruction *I,
   return false;
 }
 
-// GVN::runOnFunction - This is the main transformation entry point for a
-// function.
-//
+/// runOnFunction - This is the main transformation entry point for a function.
 bool GVN::runOnFunction(Function& F) {
   MD = &getAnalysis<MemoryDependenceAnalysis>();
   DT = &getAnalysis<DominatorTree>();
@@ -1340,9 +1473,13 @@ bool GVN::runOnFunction(Function& F) {
     changed |= removedBlock;
   }
   
+  unsigned Iteration = 0;
+  
   while (shouldContinue) {
+    DEBUG(cerr << "GVN iteration: " << Iteration << "\n");
     shouldContinue = iterateOnFunction(F);
     changed |= shouldContinue;
+    ++Iteration;
   }
   
   if (EnablePRE) {
@@ -1352,6 +1489,10 @@ bool GVN::runOnFunction(Function& F) {
       changed |= PREChanged;
     }
   }
+  // FIXME: Should perform GVN again after PRE does something.  PRE can move
+  // computations into blocks where they become fully redundant.  Note that
+  // we can't do this until PRE's critical edge splitting updates memdep.
+  // Actually, when this happens, we should just fully integrate PRE into GVN.
 
   cleanupGlobalSets();
 
@@ -1359,21 +1500,15 @@ bool GVN::runOnFunction(Function& F) {
 }
 
 
-bool GVN::processBlock(DomTreeNode* DTN) {
-  BasicBlock* BB = DTN->getBlock();
+bool GVN::processBlock(BasicBlock* BB) {
+  // FIXME: Kill off toErase by doing erasing eagerly in a helper function (and
+  // incrementing BI before processing an instruction).
   SmallVector<Instruction*, 8> toErase;
-  DenseMap<Value*, LoadInst*> lastSeenLoad;
   bool changed_function = false;
   
-  if (DTN->getIDom())
-    localAvail[BB] =
-                  new ValueNumberScope(localAvail[DTN->getIDom()->getBlock()]);
-  else
-    localAvail[BB] = new ValueNumberScope(0);
-  
   for (BasicBlock::iterator BI = BB->begin(), BE = BB->end();
        BI != BE;) {
-    changed_function |= processInstruction(BI, lastSeenLoad, toErase);
+    changed_function |= processInstruction(BI, toErase);
     if (toErase.empty()) {
       ++BI;
       continue;
@@ -1392,14 +1527,14 @@ bool GVN::processBlock(DomTreeNode* DTN) {
       DEBUG(cerr << "GVN removed: " << **I);
       MD->removeInstruction(*I);
       (*I)->eraseFromParent();
+      DEBUG(verifyRemoved(*I));
     }
+    toErase.clear();
 
     if (AtStart)
       BI = BB->begin();
     else
       ++BI;
-    
-    toErase.clear();
   }
   
   return changed_function;
@@ -1421,12 +1556,13 @@ bool GVN::performPRE(Function& F) {
     for (BasicBlock::iterator BI = CurrentBlock->begin(),
          BE = CurrentBlock->end(); BI != BE; ) {
       Instruction *CurInst = BI++;
-      
+
       if (isa<AllocationInst>(CurInst) || isa<TerminatorInst>(CurInst) ||
-          isa<PHINode>(CurInst) || CurInst->mayReadFromMemory() ||
-          CurInst->mayWriteToMemory())
+          isa<PHINode>(CurInst) || (CurInst->getType() == Type::VoidTy) ||
+          CurInst->mayReadFromMemory() || CurInst->mayHaveSideEffects() ||
+          isa<DbgInfoIntrinsic>(CurInst))
         continue;
-      
+
       uint32_t valno = VN.lookup(CurInst);
       
       // Look for the predecessors for PRE opportunities.  We're
@@ -1484,7 +1620,6 @@ bool GVN::performPRE(Function& F) {
         
       if (isCriticalEdge(PREPred->getTerminator(), succNum)) {
         toSplit.push_back(std::make_pair(PREPred->getTerminator(), succNum));
-        Changed = true;
         continue;
       }
       
@@ -1493,7 +1628,7 @@ bool GVN::performPRE(Function& F) {
       // will be available in the predecessor by the time we need them.  Any
       // that weren't original present will have been instantiated earlier
       // in this loop.
-      Instruction* PREInstr = CurInst->clone();
+      Instruction* PREInstr = CurInst->clone(*Context);
       bool success = true;
       for (unsigned i = 0, e = CurInst->getNumOperands(); i != e; ++i) {
         Value *Op = PREInstr->getOperand(i);
@@ -1513,6 +1648,7 @@ bool GVN::performPRE(Function& F) {
       // are not value numbered precisely.
       if (!success) {
         delete PREInstr;
+        DEBUG(verifyRemoved(PREInstr));
         continue;
       }
       
@@ -1537,36 +1673,52 @@ bool GVN::performPRE(Function& F) {
       localAvail[CurrentBlock]->table[valno] = Phi;
       
       CurInst->replaceAllUsesWith(Phi);
+      if (isa<PointerType>(Phi->getType()))
+        MD->invalidateCachedPointerInfo(Phi);
       VN.erase(CurInst);
       
       DEBUG(cerr << "GVN PRE removed: " << *CurInst);
       MD->removeInstruction(CurInst);
       CurInst->eraseFromParent();
+      DEBUG(verifyRemoved(CurInst));
       Changed = true;
     }
   }
   
   for (SmallVector<std::pair<TerminatorInst*, unsigned>, 4>::iterator
-       I = toSplit.begin(), E = toSplit.end(); I != E; ++I) {
+       I = toSplit.begin(), E = toSplit.end(); I != E; ++I)
     SplitCriticalEdge(I->first, I->second, this);
-    BasicBlock* NewBlock = I->first->getSuccessor(I->second);
-    localAvail[NewBlock] =
-             new ValueNumberScope(localAvail[I->first->getParent()]);
-  }
   
-  return Changed;
+  return Changed || toSplit.size();
 }
 
-// iterateOnFunction - Executes one iteration of GVN
+/// iterateOnFunction - Executes one iteration of GVN
 bool GVN::iterateOnFunction(Function &F) {
   cleanupGlobalSets();
 
+  for (df_iterator<DomTreeNode*> DI = df_begin(DT->getRootNode()),
+       DE = df_end(DT->getRootNode()); DI != DE; ++DI) {
+    if (DI->getIDom())
+      localAvail[DI->getBlock()] =
+                   new ValueNumberScope(localAvail[DI->getIDom()->getBlock()]);
+    else
+      localAvail[DI->getBlock()] = new ValueNumberScope(0);
+  }
+
   // Top-down walk of the dominator tree
   bool changed = false;
+#if 0
+  // Needed for value numbering with phi construction to work.
+  ReversePostOrderTraversal<Function*> RPOT(&F);
+  for (ReversePostOrderTraversal<Function*>::rpo_iterator RI = RPOT.begin(),
+       RE = RPOT.end(); RI != RE; ++RI)
+    changed |= processBlock(*RI);
+#else
   for (df_iterator<DomTreeNode*> DI = df_begin(DT->getRootNode()),
        DE = df_end(DT->getRootNode()); DI != DE; ++DI)
-    changed |= processBlock(*DI);
-  
+    changed |= processBlock(DI->getBlock());
+#endif
+
   return changed;
 }
 
@@ -1579,3 +1731,37 @@ void GVN::cleanupGlobalSets() {
     delete I->second;
   localAvail.clear();
 }
+
+/// verifyRemoved - Verify that the specified instruction does not occur in our
+/// internal data structures.
+void GVN::verifyRemoved(const Instruction *Inst) const {
+  VN.verifyRemoved(Inst);
+
+  // Walk through the PHI map to make sure the instruction isn't hiding in there
+  // somewhere.
+  for (PhiMapType::iterator
+         I = phiMap.begin(), E = phiMap.end(); I != E; ++I) {
+    assert(I->first != Inst && "Inst is still a key in PHI map!");
+
+    for (SmallPtrSet<Instruction*, 4>::iterator
+           II = I->second.begin(), IE = I->second.end(); II != IE; ++II) {
+      assert(*II != Inst && "Inst is still a value in PHI map!");
+    }
+  }
+
+  // Walk through the value number scope to make sure the instruction isn't
+  // ferreted away in it.
+  for (DenseMap<BasicBlock*, ValueNumberScope*>::iterator
+         I = localAvail.begin(), E = localAvail.end(); I != E; ++I) {
+    const ValueNumberScope *VNS = I->second;
+
+    while (VNS) {
+      for (DenseMap<uint32_t, Value*>::iterator
+             II = VNS->table.begin(), IE = VNS->table.end(); II != IE; ++II) {
+        assert(II->second != Inst && "Inst still in value numbering scope!");
+      }
+
+      VNS = VNS->parent;
+    }
+  }
+}