use splice instead of remove/insert to avoid some symtab operations
[oota-llvm.git] / lib / Transforms / Utils / SimplifyCFG.cpp
index 419c8a7542156fc623592c900d06d2cf1a36bba6..3eaae8eacc530b26675e24cc61cd35e8ae029db9 100644 (file)
@@ -1,10 +1,10 @@
 //===- SimplifyCFG.cpp - Code to perform CFG simplification ---------------===//
-// 
+//
 //                     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.
-// 
+//
 //===----------------------------------------------------------------------===//
 //
 // Peephole optimize the CFG.
 #include <algorithm>
 #include <functional>
 #include <set>
-
+#include <map>
 using namespace llvm;
 
+/// SafeToMergeTerminators - Return true if it is safe to merge these two
+/// terminator instructions together.
+///
+static bool SafeToMergeTerminators(TerminatorInst *SI1, TerminatorInst *SI2) {
+  if (SI1 == SI2) return false;  // Can't merge with self!
+  
+  // It is not safe to merge these two switch instructions if they have a common
+  // successor, and if that successor has a PHI node, and if *that* PHI node has
+  // conflicting incoming values from the two switch blocks.
+  BasicBlock *SI1BB = SI1->getParent();
+  BasicBlock *SI2BB = SI2->getParent();
+  std::set<BasicBlock*> SI1Succs(succ_begin(SI1BB), succ_end(SI1BB));
+  
+  for (succ_iterator I = succ_begin(SI2BB), E = succ_end(SI2BB); I != E; ++I)
+    if (SI1Succs.count(*I))
+      for (BasicBlock::iterator BBI = (*I)->begin();
+           isa<PHINode>(BBI); ++BBI) {
+        PHINode *PN = cast<PHINode>(BBI);
+        if (PN->getIncomingValueForBlock(SI1BB) !=
+            PN->getIncomingValueForBlock(SI2BB))
+          return false;
+      }
+        
+  return true;
+}
+
+/// AddPredecessorToBlock - Update PHI nodes in Succ to indicate that there will
+/// now be entries in it from the 'NewPred' block.  The values that will be
+/// flowing into the PHI nodes will be the same as those coming in from
+/// ExistPred, an existing predecessor of Succ.
+static void AddPredecessorToBlock(BasicBlock *Succ, BasicBlock *NewPred,
+                                  BasicBlock *ExistPred) {
+  assert(std::find(succ_begin(ExistPred), succ_end(ExistPred), Succ) !=
+         succ_end(ExistPred) && "ExistPred is not a predecessor of Succ!");
+  if (!isa<PHINode>(Succ->begin())) return; // Quick exit if nothing to do
+  
+  for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
+    PHINode *PN = cast<PHINode>(I);
+    Value *V = PN->getIncomingValueForBlock(ExistPred);
+    PN->addIncoming(V, NewPred);
+  }
+}
+
 // PropagatePredecessorsForPHIs - This gets "Succ" ready to have the
 // predecessors from "BB".  This is a little tricky because "Succ" has PHI
 // nodes, which need to have extra slots added to them to hold the merge edges
@@ -48,28 +91,12 @@ static bool PropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
   // Succ.  If so, we cannot do the transformation if there are any PHI nodes
   // with incompatible values coming in from the two edges!
   //
-  for (pred_iterator PI = pred_begin(Succ), PE = pred_end(Succ); PI != PE; ++PI)
-    if (std::find(BBPreds.begin(), BBPreds.end(), *PI) != BBPreds.end()) {
-      // Loop over all of the PHI nodes checking to see if there are
-      // incompatible values coming in.
-      for (BasicBlock::iterator I = Succ->begin();
-           PHINode *PN = dyn_cast<PHINode>(I); ++I) {
-        // Loop up the entries in the PHI node for BB and for *PI if the values
-        // coming in are non-equal, we cannot merge these two blocks (instead we
-        // should insert a conditional move or something, then merge the
-        // blocks).
-        int Idx1 = PN->getBasicBlockIndex(BB);
-        int Idx2 = PN->getBasicBlockIndex(*PI);
-        assert(Idx1 != -1 && Idx2 != -1 &&
-               "Didn't have entries for my predecessors??");
-        if (PN->getIncomingValue(Idx1) != PN->getIncomingValue(Idx2))
-          return true;  // Values are not equal...
-      }
-    }
+  if (!SafeToMergeTerminators(BB->getTerminator(), Succ->getTerminator()))
+    return true;   // Cannot merge.
 
   // Loop over all of the PHI nodes in the successor BB.
-  for (BasicBlock::iterator I = Succ->begin();
-       PHINode *PN = dyn_cast<PHINode>(I); ++I) {
+  for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
+    PHINode *PN = cast<PHINode>(I);
     Value *OldVal = PN->removeIncomingValue(BB, false);
     assert(OldVal && "No entry in PHI for Pred BB!");
 
@@ -81,7 +108,7 @@ static bool PropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
         PN->addIncoming(OldValPN->getIncomingValue(i),
                         OldValPN->getIncomingBlock(i));
     } else {
-      for (std::vector<BasicBlock*>::const_iterator PredI = BBPreds.begin(), 
+      for (std::vector<BasicBlock*>::const_iterator PredI = BBPreds.begin(),
              End = BBPreds.end(); PredI != End; ++PredI) {
         // Add an incoming value for each of the new incoming values...
         PN->addIncoming(OldVal, *PredI);
@@ -91,13 +118,73 @@ static bool PropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
   return false;
 }
 
+/// TryToSimplifyUncondBranchFromEmptyBlock - BB contains an unconditional
+/// branch to Succ, and contains no instructions other than PHI nodes and the
+/// branch.  If possible, eliminate BB.
+static bool TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
+                                                    BasicBlock *Succ) {
+  // If our successor has PHI nodes, then we need to update them to include
+  // entries for BB's predecessors, not for BB itself.  Be careful though,
+  // if this transformation fails (returns true) then we cannot do this
+  // transformation!
+  //
+  if (PropagatePredecessorsForPHIs(BB, Succ)) return false;
+  
+  DEBUG(std::cerr << "Killing Trivial BB: \n" << *BB);
+  
+  if (isa<PHINode>(&BB->front())) {
+    std::vector<BasicBlock*>
+    OldSuccPreds(pred_begin(Succ), pred_end(Succ));
+    
+    // Move all PHI nodes in BB to Succ if they are alive, otherwise
+    // delete them.
+    while (PHINode *PN = dyn_cast<PHINode>(&BB->front()))
+      if (PN->use_empty() /*|| Succ->getSinglePredecessor() == 0*/) {
+        // We can only move the PHI node into Succ if BB dominates Succ.
+        // Since BB only has a single successor (Succ), the PHI nodes
+        // will dominate Succ, unless Succ has multiple predecessors.  In
+        // this case, the PHIs are either dead, or have references in dead
+        // blocks.  In either case, we can just remove them.
+        if (!PN->use_empty())   // Uses in dead block?
+          PN->replaceAllUsesWith(UndefValue::get(PN->getType()));
+        PN->eraseFromParent();  // Nuke instruction.
+      } else {
+        // The instruction is alive, so this means that Succ must have
+        // *ONLY* had BB as a predecessor, and the PHI node is still valid
+        // now.  Simply move it into Succ, because we know that BB
+        // strictly dominated Succ.
+        Succ->getInstList().splice(Succ->begin(),
+                                   BB->getInstList(), BB->begin());
+        
+        // We need to add new entries for the PHI node to account for
+        // predecessors of Succ that the PHI node does not take into
+        // account.  At this point, since we know that BB dominated succ,
+        // this means that we should any newly added incoming edges should
+        // use the PHI node as the value for these edges, because they are
+        // loop back edges.
+        for (unsigned i = 0, e = OldSuccPreds.size(); i != e; ++i)
+          if (OldSuccPreds[i] != BB)
+            PN->addIncoming(PN, OldSuccPreds[i]);
+      }
+  }
+    
+  // Everything that jumped to BB now goes to Succ.
+  std::string OldName = BB->getName();
+  BB->replaceAllUsesWith(Succ);
+  BB->eraseFromParent();              // Delete the old basic block.
+  
+  if (!OldName.empty() && !Succ->hasName())  // Transfer name if we can
+    Succ->setName(OldName);
+  return true;
+}
+
 /// GetIfCondition - Given a basic block (BB) with two predecessors (and
 /// presumably PHI nodes in it), check to see if the merge at this block is due
 /// to an "if condition".  If so, return the boolean condition that determines
 /// which entry into BB will be taken.  Also, return by references the block
 /// that will be entered from if the condition is true, and the block that will
 /// be entered if the condition is false.
-/// 
+///
 ///
 static Value *GetIfCondition(BasicBlock *BB,
                              BasicBlock *&IfTrue, BasicBlock *&IfFalse) {
@@ -185,12 +272,18 @@ static Value *GetIfCondition(BasicBlock *BB,
 // if the specified value dominates the block.  We don't handle the true
 // generality of domination here, just a special case which works well enough
 // for us.
-static bool DominatesMergePoint(Value *V, BasicBlock *BB, bool AllowAggressive){
+//
+// If AggressiveInsts is non-null, and if V does not dominate BB, we check to
+// see if V (which must be an instruction) is cheap to compute and is
+// non-trapping.  If both are true, the instruction is inserted into the set and
+// true is returned.
+static bool DominatesMergePoint(Value *V, BasicBlock *BB,
+                                std::set<Instruction*> *AggressiveInsts) {
   Instruction *I = dyn_cast<Instruction>(V);
   if (!I) return true;    // Non-instructions all dominate instructions.
   BasicBlock *PBB = I->getParent();
 
-  // We don't want to allow wierd loops that might have the "if condition" in
+  // We don't want to allow weird loops that might have the "if condition" in
   // the bottom of this block.
   if (PBB == BB) return false;
 
@@ -199,7 +292,7 @@ static bool DominatesMergePoint(Value *V, BasicBlock *BB, bool AllowAggressive){
   // statement".
   if (BranchInst *BI = dyn_cast<BranchInst>(PBB->getTerminator()))
     if (BI->isUnconditional() && BI->getSuccessor(0) == BB) {
-      if (!AllowAggressive) return false;
+      if (!AggressiveInsts) return false;
       // Okay, it looks like the instruction IS in the "condition".  Check to
       // see if its a cheap instruction to unconditionally compute, and if it
       // only uses stuff defined outside of the condition.  If so, hoist it out.
@@ -226,15 +319,22 @@ static bool DominatesMergePoint(Value *V, BasicBlock *BB, bool AllowAggressive){
       case Instruction::Xor:
       case Instruction::Shl:
       case Instruction::Shr:
+      case Instruction::SetEQ:
+      case Instruction::SetNE:
+      case Instruction::SetLT:
+      case Instruction::SetGT:
+      case Instruction::SetLE:
+      case Instruction::SetGE:
         break;   // These are all cheap and non-trapping instructions.
       }
-      
+
       // Okay, we can only really hoist these out if their operands are not
       // defined in the conditional region.
       for (unsigned i = 0, e = I->getNumOperands(); i != e; ++i)
-        if (!DominatesMergePoint(I->getOperand(i), BB, false))
+        if (!DominatesMergePoint(I->getOperand(i), BB, 0))
           return false;
-      // Okay, it's safe to do this!
+      // Okay, it's safe to do this!  Remember this instruction.
+      AggressiveInsts->insert(I);
     }
 
   return true;
@@ -304,7 +404,7 @@ static bool GatherValueComparisons(Instruction *Cond, Value *&CompVal,
     return true;
   } else if (Cond->getOpcode() == Instruction::And) {
     CompVal = GatherConstantSetNEs(Cond, Values);
-        
+
     // Return false to indicate that the condition is false if the CompVal is
     // equal to one of the constants.
     return false;
@@ -325,47 +425,6 @@ static void ErasePossiblyDeadInstructionTree(Instruction *I) {
   }
 }
 
-/// SafeToMergeTerminators - Return true if it is safe to merge these two
-/// terminator instructions together.
-///
-static bool SafeToMergeTerminators(TerminatorInst *SI1, TerminatorInst *SI2) {
-  if (SI1 == SI2) return false;  // Can't merge with self!
-
-  // It is not safe to merge these two switch instructions if they have a common
-  // successor, and if that successor has a PHI node, and if *that* PHI node has
-  // conflicting incoming values from the two switch blocks.
-  BasicBlock *SI1BB = SI1->getParent();
-  BasicBlock *SI2BB = SI2->getParent();
-  std::set<BasicBlock*> SI1Succs(succ_begin(SI1BB), succ_end(SI1BB));
-
-  for (succ_iterator I = succ_begin(SI2BB), E = succ_end(SI2BB); I != E; ++I)
-    if (SI1Succs.count(*I))
-      for (BasicBlock::iterator BBI = (*I)->begin();
-           PHINode *PN = dyn_cast<PHINode>(BBI); ++BBI)
-        if (PN->getIncomingValueForBlock(SI1BB) !=
-            PN->getIncomingValueForBlock(SI2BB))
-          return false;
-        
-  return true;
-}
-
-/// AddPredecessorToBlock - Update PHI nodes in Succ to indicate that there will
-/// now be entries in it from the 'NewPred' block.  The values that will be
-/// flowing into the PHI nodes will be the same as those coming in from
-/// ExistPred, an existing predecessor of Succ.
-static void AddPredecessorToBlock(BasicBlock *Succ, BasicBlock *NewPred,
-                                  BasicBlock *ExistPred) {
-  assert(std::find(succ_begin(ExistPred), succ_end(ExistPred), Succ) !=
-         succ_end(ExistPred) && "ExistPred is not a predecessor of Succ!");
-  if (!isa<PHINode>(Succ->begin())) return; // Quick exit if nothing to do
-
-  for (BasicBlock::iterator I = Succ->begin();
-       PHINode *PN = dyn_cast<PHINode>(I); ++I) {
-    Value *V = PN->getIncomingValueForBlock(ExistPred);
-    PN->addIncoming(V, NewPred);
-  }
-}
-
 // isValueEqualityComparison - Return true if the specified terminator checks to
 // see if a value is equal to constant integer value.
 static Value *isValueEqualityComparison(TerminatorInst *TI) {
@@ -382,7 +441,7 @@ static Value *isValueEqualityComparison(TerminatorInst *TI) {
     if (BI->isConditional() && BI->getCondition()->hasOneUse())
       if (SetCondInst *SCI = dyn_cast<SetCondInst>(BI->getCondition()))
         if ((SCI->getOpcode() == Instruction::SetEQ ||
-             SCI->getOpcode() == Instruction::SetNE) && 
+             SCI->getOpcode() == Instruction::SetNE) &&
             isa<ConstantInt>(SCI->getOperand(1)))
           return SCI->getOperand(0);
   return 0;
@@ -391,14 +450,13 @@ static Value *isValueEqualityComparison(TerminatorInst *TI) {
 // Given a value comparison instruction, decode all of the 'cases' that it
 // represents and return the 'default' block.
 static BasicBlock *
-GetValueEqualityComparisonCases(TerminatorInst *TI, 
+GetValueEqualityComparisonCases(TerminatorInst *TI,
                                 std::vector<std::pair<ConstantInt*,
                                                       BasicBlock*> > &Cases) {
   if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
     Cases.reserve(SI->getNumCases());
     for (unsigned i = 1, e = SI->getNumCases(); i != e; ++i)
-      Cases.push_back(std::make_pair(cast<ConstantInt>(SI->getCaseValue(i)),
-                                     SI->getSuccessor(i)));
+      Cases.push_back(std::make_pair(SI->getCaseValue(i), SI->getSuccessor(i)));
     return SI->getDefaultDest();
   }
 
@@ -411,6 +469,175 @@ GetValueEqualityComparisonCases(TerminatorInst *TI,
 }
 
 
+// EliminateBlockCases - Given an vector of bb/value pairs, remove any entries
+// in the list that match the specified block.
+static void EliminateBlockCases(BasicBlock *BB,
+               std::vector<std::pair<ConstantInt*, BasicBlock*> > &Cases) {
+  for (unsigned i = 0, e = Cases.size(); i != e; ++i)
+    if (Cases[i].second == BB) {
+      Cases.erase(Cases.begin()+i);
+      --i; --e;
+    }
+}
+
+// ValuesOverlap - Return true if there are any keys in C1 that exist in C2 as
+// well.
+static bool
+ValuesOverlap(std::vector<std::pair<ConstantInt*, BasicBlock*> > &C1,
+              std::vector<std::pair<ConstantInt*, BasicBlock*> > &C2) {
+  std::vector<std::pair<ConstantInt*, BasicBlock*> > *V1 = &C1, *V2 = &C2;
+
+  // Make V1 be smaller than V2.
+  if (V1->size() > V2->size())
+    std::swap(V1, V2);
+
+  if (V1->size() == 0) return false;
+  if (V1->size() == 1) {
+    // Just scan V2.
+    ConstantInt *TheVal = (*V1)[0].first;
+    for (unsigned i = 0, e = V2->size(); i != e; ++i)
+      if (TheVal == (*V2)[i].first)
+        return true;
+  }
+
+  // Otherwise, just sort both lists and compare element by element.
+  std::sort(V1->begin(), V1->end());
+  std::sort(V2->begin(), V2->end());
+  unsigned i1 = 0, i2 = 0, e1 = V1->size(), e2 = V2->size();
+  while (i1 != e1 && i2 != e2) {
+    if ((*V1)[i1].first == (*V2)[i2].first)
+      return true;
+    if ((*V1)[i1].first < (*V2)[i2].first)
+      ++i1;
+    else
+      ++i2;
+  }
+  return false;
+}
+
+// SimplifyEqualityComparisonWithOnlyPredecessor - If TI is known to be a
+// terminator instruction and its block is known to only have a single
+// predecessor block, check to see if that predecessor is also a value
+// comparison with the same value, and if that comparison determines the outcome
+// of this comparison.  If so, simplify TI.  This does a very limited form of
+// jump threading.
+static bool SimplifyEqualityComparisonWithOnlyPredecessor(TerminatorInst *TI,
+                                                          BasicBlock *Pred) {
+  Value *PredVal = isValueEqualityComparison(Pred->getTerminator());
+  if (!PredVal) return false;  // Not a value comparison in predecessor.
+
+  Value *ThisVal = isValueEqualityComparison(TI);
+  assert(ThisVal && "This isn't a value comparison!!");
+  if (ThisVal != PredVal) return false;  // Different predicates.
+
+  // Find out information about when control will move from Pred to TI's block.
+  std::vector<std::pair<ConstantInt*, BasicBlock*> > PredCases;
+  BasicBlock *PredDef = GetValueEqualityComparisonCases(Pred->getTerminator(),
+                                                        PredCases);
+  EliminateBlockCases(PredDef, PredCases);  // Remove default from cases.
+
+  // Find information about how control leaves this block.
+  std::vector<std::pair<ConstantInt*, BasicBlock*> > ThisCases;
+  BasicBlock *ThisDef = GetValueEqualityComparisonCases(TI, ThisCases);
+  EliminateBlockCases(ThisDef, ThisCases);  // Remove default from cases.
+
+  // If TI's block is the default block from Pred's comparison, potentially
+  // simplify TI based on this knowledge.
+  if (PredDef == TI->getParent()) {
+    // If we are here, we know that the value is none of those cases listed in
+    // PredCases.  If there are any cases in ThisCases that are in PredCases, we
+    // can simplify TI.
+    if (ValuesOverlap(PredCases, ThisCases)) {
+      if (BranchInst *BTI = dyn_cast<BranchInst>(TI)) {
+        // Okay, one of the successors of this condbr is dead.  Convert it to a
+        // uncond br.
+        assert(ThisCases.size() == 1 && "Branch can only have one case!");
+        Value *Cond = BTI->getCondition();
+        // Insert the new branch.
+        Instruction *NI = new BranchInst(ThisDef, TI);
+
+        // Remove PHI node entries for the dead edge.
+        ThisCases[0].second->removePredecessor(TI->getParent());
+
+        DEBUG(std::cerr << "Threading pred instr: " << *Pred->getTerminator()
+              << "Through successor TI: " << *TI << "Leaving: " << *NI << "\n");
+
+        TI->eraseFromParent();   // Nuke the old one.
+        // If condition is now dead, nuke it.
+        if (Instruction *CondI = dyn_cast<Instruction>(Cond))
+          ErasePossiblyDeadInstructionTree(CondI);
+        return true;
+
+      } else {
+        SwitchInst *SI = cast<SwitchInst>(TI);
+        // Okay, TI has cases that are statically dead, prune them away.
+        std::set<Constant*> DeadCases;
+        for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
+          DeadCases.insert(PredCases[i].first);
+
+        DEBUG(std::cerr << "Threading pred instr: " << *Pred->getTerminator()
+                  << "Through successor TI: " << *TI);
+
+        for (unsigned i = SI->getNumCases()-1; i != 0; --i)
+          if (DeadCases.count(SI->getCaseValue(i))) {
+            SI->getSuccessor(i)->removePredecessor(TI->getParent());
+            SI->removeCase(i);
+          }
+
+        DEBUG(std::cerr << "Leaving: " << *TI << "\n");
+        return true;
+      }
+    }
+
+  } else {
+    // Otherwise, TI's block must correspond to some matched value.  Find out
+    // which value (or set of values) this is.
+    ConstantInt *TIV = 0;
+    BasicBlock *TIBB = TI->getParent();
+    for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
+      if (PredCases[i].second == TIBB)
+        if (TIV == 0)
+          TIV = PredCases[i].first;
+        else
+          return false;  // Cannot handle multiple values coming to this block.
+    assert(TIV && "No edge from pred to succ?");
+
+    // Okay, we found the one constant that our value can be if we get into TI's
+    // BB.  Find out which successor will unconditionally be branched to.
+    BasicBlock *TheRealDest = 0;
+    for (unsigned i = 0, e = ThisCases.size(); i != e; ++i)
+      if (ThisCases[i].first == TIV) {
+        TheRealDest = ThisCases[i].second;
+        break;
+      }
+
+    // If not handled by any explicit cases, it is handled by the default case.
+    if (TheRealDest == 0) TheRealDest = ThisDef;
+
+    // Remove PHI node entries for dead edges.
+    BasicBlock *CheckEdge = TheRealDest;
+    for (succ_iterator SI = succ_begin(TIBB), e = succ_end(TIBB); SI != e; ++SI)
+      if (*SI != CheckEdge)
+        (*SI)->removePredecessor(TIBB);
+      else
+        CheckEdge = 0;
+
+    // Insert the new branch.
+    Instruction *NI = new BranchInst(TheRealDest, TI);
+
+    DEBUG(std::cerr << "Threading pred instr: " << *Pred->getTerminator()
+          << "Through successor TI: " << *TI << "Leaving: " << *NI << "\n");
+    Instruction *Cond = 0;
+    if (BranchInst *BI = dyn_cast<BranchInst>(TI))
+      Cond = dyn_cast<Instruction>(BI->getCondition());
+    TI->eraseFromParent();   // Nuke the old one.
+
+    if (Cond) ErasePossiblyDeadInstructionTree(Cond);
+    return true;
+  }
+  return false;
+}
+
 // FoldValueComparisonIntoPredecessors - The specified terminator is a value
 // equality comparison instruction (either a switch or a branch on "X == c").
 // See if any of the predecessors of the terminator block are value comparisons
@@ -425,7 +652,7 @@ static bool FoldValueComparisonIntoPredecessors(TerminatorInst *TI) {
   while (!Preds.empty()) {
     BasicBlock *Pred = Preds.back();
     Preds.pop_back();
-    
+
     // See if the predecessor is a comparison with the same value.
     TerminatorInst *PTI = Pred->getTerminator();
     Value *PCV = isValueEqualityComparison(PTI);  // PredCondVal
@@ -509,11 +736,19 @@ static bool FoldValueComparisonIntoPredecessors(TerminatorInst *TI) {
         AddPredecessorToBlock(NewSuccessors[i], Pred, BB);
 
       // Now that the successors are updated, create the new Switch instruction.
-      SwitchInst *NewSI = new SwitchInst(CV, PredDefault, PTI);
+      SwitchInst *NewSI = new SwitchInst(CV, PredDefault, PredCases.size(),PTI);
       for (unsigned i = 0, e = PredCases.size(); i != e; ++i)
         NewSI->addCase(PredCases[i].first, PredCases[i].second);
+
+      Instruction *DeadCond = 0;
+      if (BranchInst *BI = dyn_cast<BranchInst>(PTI))
+        // If PTI is a branch, remember the condition.
+        DeadCond = dyn_cast<Instruction>(BI->getCondition());
       Pred->getInstList().erase(PTI);
 
+      // If the condition is dead now, remove the instruction tree.
+      if (DeadCond) ErasePossiblyDeadInstructionTree(DeadCond);
+
       // Okay, last check.  If BB is still a successor of PSI, then we must
       // have an infinite loop case.  If so, add an infinitely looping block
       // to handle the case to preserve the behavior of the code.
@@ -528,13 +763,96 @@ static bool FoldValueComparisonIntoPredecessors(TerminatorInst *TI) {
           }
           NewSI->setSuccessor(i, InfLoopBlock);
         }
-          
+
       Changed = true;
     }
   }
   return Changed;
 }
 
+/// HoistThenElseCodeToIf - Given a conditional branch that codes to BB1 and
+/// BB2, hoist any common code in the two blocks up into the branch block.  The
+/// caller of this function guarantees that BI's block dominates BB1 and BB2.
+static bool HoistThenElseCodeToIf(BranchInst *BI) {
+  // This does very trivial matching, with limited scanning, to find identical
+  // instructions in the two blocks.  In particular, we don't want to get into
+  // O(M*N) situations here where M and N are the sizes of BB1 and BB2.  As
+  // such, we currently just scan for obviously identical instructions in an
+  // identical order.
+  BasicBlock *BB1 = BI->getSuccessor(0);  // The true destination.
+  BasicBlock *BB2 = BI->getSuccessor(1);  // The false destination
+
+  Instruction *I1 = BB1->begin(), *I2 = BB2->begin();
+  if (I1->getOpcode() != I2->getOpcode() || !I1->isIdenticalTo(I2))
+    return false;
+
+  // If we get here, we can hoist at least one instruction.
+  BasicBlock *BIParent = BI->getParent();
+
+  do {
+    // If we are hoisting the terminator instruction, don't move one (making a
+    // broken BB), instead clone it, and remove BI.
+    if (isa<TerminatorInst>(I1))
+      goto HoistTerminator;
+
+    // For a normal instruction, we just move one to right before the branch,
+    // then replace all uses of the other with the first.  Finally, we remove
+    // the now redundant second instruction.
+    BIParent->getInstList().splice(BI, BB1->getInstList(), I1);
+    if (!I2->use_empty())
+      I2->replaceAllUsesWith(I1);
+    BB2->getInstList().erase(I2);
+
+    I1 = BB1->begin();
+    I2 = BB2->begin();
+  } while (I1->getOpcode() == I2->getOpcode() && I1->isIdenticalTo(I2));
+
+  return true;
+
+HoistTerminator:
+  // Okay, it is safe to hoist the terminator.
+  Instruction *NT = I1->clone();
+  BIParent->getInstList().insert(BI, NT);
+  if (NT->getType() != Type::VoidTy) {
+    I1->replaceAllUsesWith(NT);
+    I2->replaceAllUsesWith(NT);
+    NT->setName(I1->getName());
+  }
+
+  // Hoisting one of the terminators from our successor is a great thing.
+  // Unfortunately, the successors of the if/else blocks may have PHI nodes in
+  // them.  If they do, all PHI entries for BB1/BB2 must agree for all PHI
+  // nodes, so we insert select instruction to compute the final result.
+  std::map<std::pair<Value*,Value*>, SelectInst*> InsertedSelects;
+  for (succ_iterator SI = succ_begin(BB1), E = succ_end(BB1); SI != E; ++SI) {
+    PHINode *PN;
+    for (BasicBlock::iterator BBI = SI->begin();
+         (PN = dyn_cast<PHINode>(BBI)); ++BBI) {
+      Value *BB1V = PN->getIncomingValueForBlock(BB1);
+      Value *BB2V = PN->getIncomingValueForBlock(BB2);
+      if (BB1V != BB2V) {
+        // These values do not agree.  Insert a select instruction before NT
+        // that determines the right value.
+        SelectInst *&SI = InsertedSelects[std::make_pair(BB1V, BB2V)];
+        if (SI == 0)
+          SI = new SelectInst(BI->getCondition(), BB1V, BB2V,
+                              BB1V->getName()+"."+BB2V->getName(), NT);
+        // Make the PHI node use the select for all incoming values for BB1/BB2
+        for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
+          if (PN->getIncomingBlock(i) == BB1 || PN->getIncomingBlock(i) == BB2)
+            PN->setIncomingValue(i, SI);
+      }
+    }
+  }
+
+  // Update any PHI nodes in our new successors.
+  for (succ_iterator SI = succ_begin(BB1), E = succ_end(BB1); SI != E; ++SI)
+    AddPredecessorToBlock(*SI, BIParent, BB1);
+
+  BI->eraseFromParent();
+  return true;
+}
+
 namespace {
   /// ConstantIntOrdering - This class implements a stable ordering of constant
   /// integers that does not depend on their address.  This is important for
@@ -546,7 +864,6 @@ namespace {
   };
 }
 
-
 // SimplifyCFG - This function is used to do simplification of a CFG.  For
 // example, it adjusts branches to branches to eliminate the extra hop, it
 // eliminates unreachable basic blocks, and does other "peephole" optimization
@@ -569,20 +886,20 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
 
     // Loop through all of our successors and make sure they know that one
     // of their predecessors is going away.
-    for_each(succ_begin(BB), succ_end(BB),
-            std::bind2nd(std::mem_fun(&BasicBlock::removePredecessor), BB));
+    for (succ_iterator SI = succ_begin(BB), E = succ_end(BB); SI != E; ++SI)
+      SI->removePredecessor(BB);
 
     while (!BB->empty()) {
       Instruction &I = BB->back();
       // If this instruction is used, replace uses with an arbitrary
-      // constant value.  Because control flow can't get here, we don't care
-      // what we replace the value with.  Note that since this block is 
+      // value.  Because control flow can't get here, we don't care
+      // what we replace the value with.  Note that since this block is
       // unreachable, and all values contained within it must dominate their
       // uses, that all uses will eventually be removed.
-      if (!I.use_empty()) 
-        // Make all users of this instruction reference the constant instead
-        I.replaceAllUsesWith(Constant::getNullValue(I.getType()));
-      
+      if (!I.use_empty())
+        // Make all users of this instruction use undef instead
+        I.replaceAllUsesWith(UndefValue::get(I.getType()));
+
       // Remove the instruction from the basic block
       BB->getInstList().pop_back();
     }
@@ -594,69 +911,6 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
   // away...
   Changed |= ConstantFoldTerminator(BB);
 
-  // Check to see if this block has no non-phi instructions and only a single
-  // successor.  If so, replace references to this basic block with references
-  // to the successor.
-  succ_iterator SI(succ_begin(BB));
-  if (SI != succ_end(BB) && ++SI == succ_end(BB)) {  // One succ?
-
-    BasicBlock::iterator BBI = BB->begin();  // Skip over phi nodes...
-    while (isa<PHINode>(*BBI)) ++BBI;
-
-    if (BBI->isTerminator()) {   // Terminator is the only non-phi instruction!
-      BasicBlock *Succ = *succ_begin(BB); // There is exactly one successor
-     
-      if (Succ != BB) {   // Arg, don't hurt infinite loops!
-        // If our successor has PHI nodes, then we need to update them to
-        // include entries for BB's predecessors, not for BB itself.
-        // Be careful though, if this transformation fails (returns true) then
-        // we cannot do this transformation!
-        //
-       if (!PropagatePredecessorsForPHIs(BB, Succ)) {
-          DEBUG(std::cerr << "Killing Trivial BB: \n" << *BB);
-          std::string OldName = BB->getName();
-
-          std::vector<BasicBlock*>
-            OldSuccPreds(pred_begin(Succ), pred_end(Succ));
-
-          // Move all PHI nodes in BB to Succ if they are alive, otherwise
-          // delete them.
-          while (PHINode *PN = dyn_cast<PHINode>(&BB->front()))
-            if (PN->use_empty())
-              BB->getInstList().erase(BB->begin());  // Nuke instruction...
-            else {
-              // The instruction is alive, so this means that Succ must have
-              // *ONLY* had BB as a predecessor, and the PHI node is still valid
-              // now.  Simply move it into Succ, because we know that BB
-              // strictly dominated Succ.
-              BB->getInstList().remove(BB->begin());
-              Succ->getInstList().push_front(PN);
-
-              // We need to add new entries for the PHI node to account for
-              // predecessors of Succ that the PHI node does not take into
-              // account.  At this point, since we know that BB dominated succ,
-              // this means that we should any newly added incoming edges should
-              // use the PHI node as the value for these edges, because they are
-              // loop back edges.
-              for (unsigned i = 0, e = OldSuccPreds.size(); i != e; ++i)
-                if (OldSuccPreds[i] != BB)
-                  PN->addIncoming(PN, OldSuccPreds[i]);
-            }
-
-          // Everything that jumped to BB now goes to Succ...
-          BB->replaceAllUsesWith(Succ);
-
-          // Delete the old basic block...
-          M->getBasicBlockList().erase(BB);
-       
-          if (!OldName.empty() && !Succ->hasName())  // Transfer name if we can
-            Succ->setName(OldName);
-          return true;
-       }
-      }
-    }
-  }
-
   // If this is a returning block with only PHI nodes in it, fold the return
   // instruction into any unconditional branch predecessors.
   //
@@ -677,7 +931,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
           else
             CondBranchPreds.push_back(BI);
       }
-      
+
       // If we found some, do the transformation!
       if (!UncondBranchPreds.empty()) {
         while (!UncondBranchPreds.empty()) {
@@ -751,10 +1005,19 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
             FalseSucc->removePredecessor(BI->getParent());
 
             // Insert a new select instruction.
-            Value *NewRetVal = new SelectInst(BI->getCondition(), TrueValue,
-                                              FalseValue, "retval", BI);
+            Value *NewRetVal;
+            Value *BrCond = BI->getCondition();
+            if (TrueValue != FalseValue)
+              NewRetVal = new SelectInst(BrCond, TrueValue,
+                                         FalseValue, "retval", BI);
+            else
+              NewRetVal = TrueValue;
+
             new ReturnInst(NewRetVal, BI);
             BI->getParent()->getInstList().erase(BI);
+            if (BrCond->use_empty())
+              if (Instruction *BrCondI = dyn_cast<Instruction>(BrCond))
+                BrCondI->getParent()->getInstList().erase(BrCondI);
             return true;
           }
         }
@@ -781,17 +1044,18 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
           // is now a fall through...
           BranchInst *BI = new BranchInst(II->getNormalDest(), II);
           Pred->getInstList().remove(II);   // Take out of symbol table
-          
+
           // Insert the call now...
           std::vector<Value*> Args(II->op_begin()+3, II->op_end());
           CallInst *CI = new CallInst(II->getCalledValue(), Args,
                                       II->getName(), BI);
+          CI->setCallingConv(II->getCallingConv());
           // If the invoke produced a value, the Call now does instead
           II->replaceAllUsesWith(CI);
           delete II;
           Changed = true;
         }
-      
+
       Preds.pop_back();
     }
 
@@ -802,13 +1066,40 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
       return true;
     }
 
-  } else if (SwitchInst *SI = dyn_cast<SwitchInst>(BB->begin())) {
-    if (isValueEqualityComparison(SI))
-      if (FoldValueComparisonIntoPredecessors(SI))
-        return SimplifyCFG(BB) || 1;
+  } else if (SwitchInst *SI = dyn_cast<SwitchInst>(BB->getTerminator())) {
+    if (isValueEqualityComparison(SI)) {
+      // If we only have one predecessor, and if it is a branch on this value,
+      // see if that predecessor totally determines the outcome of this switch.
+      if (BasicBlock *OnlyPred = BB->getSinglePredecessor())
+        if (SimplifyEqualityComparisonWithOnlyPredecessor(SI, OnlyPred))
+          return SimplifyCFG(BB) || 1;
+
+      // If the block only contains the switch, see if we can fold the block
+      // away into any preds.
+      if (SI == &BB->front())
+        if (FoldValueComparisonIntoPredecessors(SI))
+          return SimplifyCFG(BB) || 1;
+    }
   } else if (BranchInst *BI = dyn_cast<BranchInst>(BB->getTerminator())) {
-    if (BI->isConditional()) {
+    if (BI->isUnconditional()) {
+      BasicBlock::iterator BBI = BB->begin();  // Skip over phi nodes...
+      while (isa<PHINode>(*BBI)) ++BBI;
+
+      BasicBlock *Succ = BI->getSuccessor(0);
+      if (BBI->isTerminator() &&  // Terminator is the only non-phi instruction!
+          Succ != BB)             // Don't hurt infinite loops!
+        if (TryToSimplifyUncondBranchFromEmptyBlock(BB, Succ))
+          return 1;
+      
+    } else {  // Conditional branch
       if (Value *CompVal = isValueEqualityComparison(BI)) {
+        // If we only have one predecessor, and if it is a branch on this value,
+        // see if that predecessor totally determines the outcome of this
+        // switch.
+        if (BasicBlock *OnlyPred = BB->getSinglePredecessor())
+          if (SimplifyEqualityComparisonWithOnlyPredecessor(BI, OnlyPred))
+            return SimplifyCFG(BB) || 1;
+
         // This block must be empty, except for the setcond inst, if it exists.
         BasicBlock::iterator I = BB->begin();
         if (&*I == BI ||
@@ -856,7 +1147,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
                   Instruction::BinaryOps Opcode =
                     PBI->getSuccessor(0) == TrueDest ?
                     Instruction::Or : Instruction::And;
-                  Value *NewCond = 
+                  Value *NewCond =
                     BinaryOperator::create(Opcode, PBI->getCondition(),
                                            New, "bothcond", PBI);
                   PBI->setCondition(NewCond);
@@ -875,15 +1166,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
       // If this block ends with a branch instruction, and if there is one
       // predecessor, see if the previous block ended with a branch on the same
       // condition, which makes this conditional branch redundant.
-      pred_iterator PI(pred_begin(BB)), PE(pred_end(BB));
-      BasicBlock *OnlyPred = *PI++;
-      for (; PI != PE; ++PI)// Search all predecessors, see if they are all same
-        if (*PI != OnlyPred) {
-          OnlyPred = 0;       // There are multiple different predecessors...
-          break;
-        }
-      
-      if (OnlyPred)
+      if (BasicBlock *OnlyPred = BB->getSinglePredecessor())
         if (BranchInst *PBI = dyn_cast<BranchInst>(OnlyPred->getTerminator()))
           if (PBI->isConditional() &&
               PBI->getCondition() == BI->getCondition() &&
@@ -898,6 +1181,114 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
             return SimplifyCFG(BB) | true;
           }
     }
+  } else if (isa<UnreachableInst>(BB->getTerminator())) {
+    // If there are any instructions immediately before the unreachable that can
+    // be removed, do so.
+    Instruction *Unreachable = BB->getTerminator();
+    while (Unreachable != BB->begin()) {
+      BasicBlock::iterator BBI = Unreachable;
+      --BBI;
+      if (isa<CallInst>(BBI)) break;
+      // Delete this instruction
+      BB->getInstList().erase(BBI);
+      Changed = true;
+    }
+
+    // If the unreachable instruction is the first in the block, take a gander
+    // at all of the predecessors of this instruction, and simplify them.
+    if (&BB->front() == Unreachable) {
+      std::vector<BasicBlock*> Preds(pred_begin(BB), pred_end(BB));
+      for (unsigned i = 0, e = Preds.size(); i != e; ++i) {
+        TerminatorInst *TI = Preds[i]->getTerminator();
+
+        if (BranchInst *BI = dyn_cast<BranchInst>(TI)) {
+          if (BI->isUnconditional()) {
+            if (BI->getSuccessor(0) == BB) {
+              new UnreachableInst(TI);
+              TI->eraseFromParent();
+              Changed = true;
+            }
+          } else {
+            if (BI->getSuccessor(0) == BB) {
+              new BranchInst(BI->getSuccessor(1), BI);
+              BI->eraseFromParent();
+            } else if (BI->getSuccessor(1) == BB) {
+              new BranchInst(BI->getSuccessor(0), BI);
+              BI->eraseFromParent();
+              Changed = true;
+            }
+          }
+        } else if (SwitchInst *SI = dyn_cast<SwitchInst>(TI)) {
+          for (unsigned i = 1, e = SI->getNumCases(); i != e; ++i)
+            if (SI->getSuccessor(i) == BB) {
+              BB->removePredecessor(SI->getParent());
+              SI->removeCase(i);
+              --i; --e;
+              Changed = true;
+            }
+          // If the default value is unreachable, figure out the most popular
+          // destination and make it the default.
+          if (SI->getSuccessor(0) == BB) {
+            std::map<BasicBlock*, unsigned> Popularity;
+            for (unsigned i = 1, e = SI->getNumCases(); i != e; ++i)
+              Popularity[SI->getSuccessor(i)]++;
+
+            // Find the most popular block.
+            unsigned MaxPop = 0;
+            BasicBlock *MaxBlock = 0;
+            for (std::map<BasicBlock*, unsigned>::iterator
+                   I = Popularity.begin(), E = Popularity.end(); I != E; ++I) {
+              if (I->second > MaxPop) {
+                MaxPop = I->second;
+                MaxBlock = I->first;
+              }
+            }
+            if (MaxBlock) {
+              // Make this the new default, allowing us to delete any explicit
+              // edges to it.
+              SI->setSuccessor(0, MaxBlock);
+              Changed = true;
+
+              // If MaxBlock has phinodes in it, remove MaxPop-1 entries from
+              // it.
+              if (isa<PHINode>(MaxBlock->begin()))
+                for (unsigned i = 0; i != MaxPop-1; ++i)
+                  MaxBlock->removePredecessor(SI->getParent());
+
+              for (unsigned i = 1, e = SI->getNumCases(); i != e; ++i)
+                if (SI->getSuccessor(i) == MaxBlock) {
+                  SI->removeCase(i);
+                  --i; --e;
+                }
+            }
+          }
+        } else if (InvokeInst *II = dyn_cast<InvokeInst>(TI)) {
+          if (II->getUnwindDest() == BB) {
+            // Convert the invoke to a call instruction.  This would be a good
+            // place to note that the call does not throw though.
+            BranchInst *BI = new BranchInst(II->getNormalDest(), II);
+            II->removeFromParent();   // Take out of symbol table
+
+            // Insert the call now...
+            std::vector<Value*> Args(II->op_begin()+3, II->op_end());
+            CallInst *CI = new CallInst(II->getCalledValue(), Args,
+                                        II->getName(), BI);
+            CI->setCallingConv(II->getCallingConv());
+            // If the invoke produced a value, the Call does now instead.
+            II->replaceAllUsesWith(CI);
+            delete II;
+            Changed = true;
+          }
+        }
+      }
+
+      // If this block is now dead, remove it.
+      if (pred_begin(BB) == pred_end(BB)) {
+        // We know there are no successors, so just nuke the block.
+        M->getBasicBlockList().erase(BB);
+        return true;
+      }
+    }
   }
 
   // Merge basic blocks into their predecessor if there is only one distinct
@@ -942,26 +1333,45 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
 
     // Delete the unconditional branch from the predecessor...
     OnlyPred->getInstList().pop_back();
-      
+
     // Move all definitions in the successor to the predecessor...
     OnlyPred->getInstList().splice(OnlyPred->end(), BB->getInstList());
-                                     
+
     // Make all PHI nodes that referred to BB now refer to Pred as their
     // source...
     BB->replaceAllUsesWith(OnlyPred);
 
     std::string OldName = BB->getName();
 
-    // Erase basic block from the function... 
+    // Erase basic block from the function...
     M->getBasicBlockList().erase(BB);
 
     // Inherit predecessors name if it exists...
     if (!OldName.empty() && !OnlyPred->hasName())
       OnlyPred->setName(OldName);
-      
+
     return true;
   }
 
+  // Otherwise, if this block only has a single predecessor, and if that block
+  // is a conditional branch, see if we can hoist any code from this block up
+  // into our predecessor.
+  if (OnlyPred)
+    if (BranchInst *BI = dyn_cast<BranchInst>(OnlyPred->getTerminator()))
+      if (BI->isConditional()) {
+        // Get the other block.
+        BasicBlock *OtherBB = BI->getSuccessor(BI->getSuccessor(0) == BB);
+        PI = pred_begin(OtherBB);
+        ++PI;
+        if (PI == pred_end(OtherBB)) {
+          // We have a conditional branch to two blocks that are only reachable
+          // from the condbr.  We know that the condbr dominates the two blocks,
+          // so see if there is any identical code in the "then" and "else"
+          // blocks.  If so, we can hoist it up to the branching block.
+          Changed |= HoistThenElseCodeToIf(BI);
+        }
+      }
+
   for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI)
     if (BranchInst *BI = dyn_cast<BranchInst>((*PI)->getTerminator()))
       // Change br (X == 0 | X == 1), T, F into a switch instruction.
@@ -977,24 +1387,25 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
           // instruction can't handle, remove them now.
           std::sort(Values.begin(), Values.end(), ConstantIntOrdering());
           Values.erase(std::unique(Values.begin(), Values.end()), Values.end());
-          
+
           // Figure out which block is which destination.
           BasicBlock *DefaultBB = BI->getSuccessor(1);
           BasicBlock *EdgeBB    = BI->getSuccessor(0);
           if (!TrueWhenEqual) std::swap(DefaultBB, EdgeBB);
-          
+
           // Create the new switch instruction now.
-          SwitchInst *New = new SwitchInst(CompVal, DefaultBB, BI);
-          
+          SwitchInst *New = new SwitchInst(CompVal, DefaultBB,Values.size(),BI);
+
           // Add all of the 'cases' to the switch instruction.
           for (unsigned i = 0, e = Values.size(); i != e; ++i)
             New->addCase(Values[i], EdgeBB);
-          
+
           // We added edges from PI to the EdgeBB.  As such, if there were any
           // PHI nodes in EdgeBB, they need entries to be added corresponding to
           // the number of edges added.
           for (BasicBlock::iterator BBI = EdgeBB->begin();
-               PHINode *PN = dyn_cast<PHINode>(BBI); ++BBI) {
+               isa<PHINode>(BBI); ++BBI) {
+            PHINode *PN = cast<PHINode>(BBI);
             Value *InVal = PN->getIncomingValueForBlock(*PI);
             for (unsigned i = 0, e = Values.size()-1; i != e; ++i)
               PN->addIncoming(InVal, *PI);
@@ -1026,57 +1437,105 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
         DEBUG(std::cerr << "FOUND IF CONDITION!  " << *IfCond << "  T: "
               << IfTrue->getName() << "  F: " << IfFalse->getName() << "\n");
 
-        // Figure out where to insert instructions as necessary.
+        // Loop over the PHI's seeing if we can promote them all to select
+        // instructions.  While we are at it, keep track of the instructions
+        // that need to be moved to the dominating block.
+        std::set<Instruction*> AggressiveInsts;
+        bool CanPromote = true;
+
         BasicBlock::iterator AfterPHIIt = BB->begin();
-        while (isa<PHINode>(AfterPHIIt)) ++AfterPHIIt;
+        while (isa<PHINode>(AfterPHIIt)) {
+          PHINode *PN = cast<PHINode>(AfterPHIIt++);
+          if (PN->getIncomingValue(0) == PN->getIncomingValue(1)) {
+            if (PN->getIncomingValue(0) != PN)
+              PN->replaceAllUsesWith(PN->getIncomingValue(0));
+            else
+              PN->replaceAllUsesWith(UndefValue::get(PN->getType()));
+          } else if (!DominatesMergePoint(PN->getIncomingValue(0), BB,
+                                          &AggressiveInsts) ||
+                     !DominatesMergePoint(PN->getIncomingValue(1), BB,
+                                          &AggressiveInsts)) {
+            CanPromote = false;
+            break;
+          }
+        }
 
-        BasicBlock::iterator I = BB->begin();
-        while (PHINode *PN = dyn_cast<PHINode>(I)) {
-          ++I;
-
-          // If we can eliminate this PHI by directly computing it based on the
-          // condition, do so now.  We can't eliminate PHI nodes where the
-          // incoming values are defined in the conditional parts of the branch,
-          // so check for this.
-          //
-          if (DominatesMergePoint(PN->getIncomingValue(0), BB, true) &&
-              DominatesMergePoint(PN->getIncomingValue(1), BB, true)) {
+        // Did we eliminate all PHI's?
+        CanPromote |= AfterPHIIt == BB->begin();
+
+        // If we all PHI nodes are promotable, check to make sure that all
+        // instructions in the predecessor blocks can be promoted as well.  If
+        // not, we won't be able to get rid of the control flow, so it's not
+        // worth promoting to select instructions.
+        BasicBlock *DomBlock = 0, *IfBlock1 = 0, *IfBlock2 = 0;
+        if (CanPromote) {
+          PN = cast<PHINode>(BB->begin());
+          BasicBlock *Pred = PN->getIncomingBlock(0);
+          if (cast<BranchInst>(Pred->getTerminator())->isUnconditional()) {
+            IfBlock1 = Pred;
+            DomBlock = *pred_begin(Pred);
+            for (BasicBlock::iterator I = Pred->begin();
+                 !isa<TerminatorInst>(I); ++I)
+              if (!AggressiveInsts.count(I)) {
+                // This is not an aggressive instruction that we can promote.
+                // Because of this, we won't be able to get rid of the control
+                // flow, so the xform is not worth it.
+                CanPromote = false;
+                break;
+              }
+          }
+
+          Pred = PN->getIncomingBlock(1);
+          if (CanPromote &&
+              cast<BranchInst>(Pred->getTerminator())->isUnconditional()) {
+            IfBlock2 = Pred;
+            DomBlock = *pred_begin(Pred);
+            for (BasicBlock::iterator I = Pred->begin();
+                 !isa<TerminatorInst>(I); ++I)
+              if (!AggressiveInsts.count(I)) {
+                // This is not an aggressive instruction that we can promote.
+                // Because of this, we won't be able to get rid of the control
+                // flow, so the xform is not worth it.
+                CanPromote = false;
+                break;
+              }
+          }
+        }
+
+        // If we can still promote the PHI nodes after this gauntlet of tests,
+        // do all of the PHI's now.
+        if (CanPromote) {
+          // Move all 'aggressive' instructions, which are defined in the
+          // conditional parts of the if's up to the dominating block.
+          if (IfBlock1) {
+            DomBlock->getInstList().splice(DomBlock->getTerminator(),
+                                           IfBlock1->getInstList(),
+                                           IfBlock1->begin(),
+                                           IfBlock1->getTerminator());
+          }
+          if (IfBlock2) {
+            DomBlock->getInstList().splice(DomBlock->getTerminator(),
+                                           IfBlock2->getInstList(),
+                                           IfBlock2->begin(),
+                                           IfBlock2->getTerminator());
+          }
+
+          while (PHINode *PN = dyn_cast<PHINode>(BB->begin())) {
+            // Change the PHI node into a select instruction.
             Value *TrueVal =
               PN->getIncomingValue(PN->getIncomingBlock(0) == IfFalse);
             Value *FalseVal =
               PN->getIncomingValue(PN->getIncomingBlock(0) == IfTrue);
 
-            // If one of the incoming values is defined in the conditional
-            // region, move it into it's predecessor block, which we know is
-            // safe.
-            if (!DominatesMergePoint(TrueVal, BB, false)) {
-              Instruction *TrueI = cast<Instruction>(TrueVal);
-              BasicBlock *OldBB = TrueI->getParent();
-              OldBB->getInstList().remove(TrueI);
-              BasicBlock *NewBB = *pred_begin(OldBB);
-              NewBB->getInstList().insert(NewBB->getTerminator(), TrueI);
-            }
-            if (!DominatesMergePoint(FalseVal, BB, false)) {
-              Instruction *FalseI = cast<Instruction>(FalseVal);
-              BasicBlock *OldBB = FalseI->getParent();
-              OldBB->getInstList().remove(FalseI);
-              BasicBlock *NewBB = *pred_begin(OldBB);
-              NewBB->getInstList().insert(NewBB->getTerminator(), FalseI);
-            }
-
-            // Change the PHI node into a select instruction.
-            BasicBlock::iterator InsertPos = PN;
-            while (isa<PHINode>(InsertPos)) ++InsertPos;
-
             std::string Name = PN->getName(); PN->setName("");
             PN->replaceAllUsesWith(new SelectInst(IfCond, TrueVal, FalseVal,
-                                                  Name, InsertPos));
+                                                  Name, AfterPHIIt));
             BB->getInstList().erase(PN);
-            Changed = true;
           }
+          Changed = true;
         }
       }
     }
-  
+
   return Changed;
 }