Squelch warning
[oota-llvm.git] / lib / Transforms / Utils / SimplifyCFG.cpp
index c93217333041609bc4f63b98506597a6093a27b6..4b7071d4e18bb31e3a1d73c3684dc37972395193 100644 (file)
@@ -21,7 +21,7 @@
 #include <algorithm>
 #include <functional>
 #include <set>
-
+#include <map>
 using namespace llvm;
 
 // PropagatePredecessorsForPHIs - This gets "Succ" ready to have the
@@ -49,11 +49,11 @@ static bool PropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
   // with incompatible values coming in from the two edges!
   //
   for (pred_iterator PI = pred_begin(Succ), PE = pred_end(Succ); PI != PE; ++PI)
-    if (find(BBPreds.begin(), BBPreds.end(), *PI) != BBPreds.end()) {
+    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) {
+      for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
+        PHINode *PN = cast<PHINode>(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
@@ -68,8 +68,8 @@ static bool PropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
     }
 
   // 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!");
 
@@ -185,7 +185,13 @@ 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();
@@ -199,7 +205,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.
@@ -232,9 +238,10 @@ static bool DominatesMergePoint(Value *V, BasicBlock *BB, bool AllowAggressive){
       // 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;
@@ -341,10 +348,12 @@ static bool SafeToMergeTerminators(TerminatorInst *SI1, TerminatorInst *SI2) {
   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)
+           isa<PHINode>(BBI); ++BBI) {
+        PHINode *PN = cast<PHINode>(BBI);
         if (PN->getIncomingValueForBlock(SI1BB) !=
             PN->getIncomingValueForBlock(SI2BB))
           return false;
+      }
         
   return true;
 }
@@ -359,8 +368,8 @@ static void AddPredecessorToBlock(BasicBlock *Succ, BasicBlock *NewPred,
          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) {
+  for (BasicBlock::iterator I = Succ->begin(); isa<PHINode>(I); ++I) {
+    PHINode *PN = cast<PHINode>(I);
     Value *V = PN->getIncomingValueForBlock(ExistPred);
     PN->addIncoming(V, NewPred);
   }
@@ -535,6 +544,89 @@ static bool FoldValueComparisonIntoPredecessors(TerminatorInst *TI) {
   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
@@ -599,31 +691,29 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
   // 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();
-
+    BasicBlock *Succ = *succ_begin(BB); // There is exactly one successor.
+    if (BBI->isTerminator() &&  // Terminator is the only non-phi instruction!
+        Succ != BB) {           // 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);
+        
+        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())
-              BB->getInstList().erase(BB->begin());  // Nuke instruction...
+              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
@@ -631,7 +721,7 @@ bool llvm::SimplifyCFG(BasicBlock *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,
@@ -642,17 +732,16 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
                 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.
 
-          // 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 (!OldName.empty() && !Succ->hasName())  // Transfer name if we can
+          Succ->setName(OldName);
+        return true;
       }
     }
   }
@@ -751,10 +840,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;
           }
         }
@@ -898,6 +996,106 @@ 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) {
+              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;
+
+              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);
+            // 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
@@ -962,6 +1160,26 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
     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())) {
+      // This is guaranteed to be a condbr at this point.
+      assert(BI->isConditional() && "Should have folded bb into pred!");
+      // 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.
@@ -994,7 +1212,8 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
           // 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,54 +1245,99 @@ 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))
+            PN->replaceAllUsesWith(PN->getIncomingValue(0));
+          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;
         }
       }
     }