Changes For Bug 352
[oota-llvm.git] / lib / Transforms / Utils / SimplifyCFG.cpp
index 09153ed3bb096d338757a5eaaeeb183c80838d4b..c93217333041609bc4f63b98506597a6093a27b6 100644 (file)
 //
 //===----------------------------------------------------------------------===//
 
+#define DEBUG_TYPE "simplifycfg"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Constants.h"
 #include "llvm/Instructions.h"
 #include "llvm/Type.h"
 #include "llvm/Support/CFG.h"
+#include "llvm/Support/Debug.h"
 #include <algorithm>
 #include <functional>
 #include <set>
+
 using namespace llvm;
 
 // PropagatePredecessorsForPHIs - This gets "Succ" ready to have the
@@ -64,19 +67,19 @@ static bool PropagatePredecessorsForPHIs(BasicBlock *BB, BasicBlock *Succ) {
       }
     }
 
-  // Loop over all of the PHI nodes in the successor BB
+  // Loop over all of the PHI nodes in the successor BB.
   for (BasicBlock::iterator I = Succ->begin();
        PHINode *PN = dyn_cast<PHINode>(I); ++I) {
     Value *OldVal = PN->removeIncomingValue(BB, false);
     assert(OldVal && "No entry in PHI for Pred BB!");
 
-    // If this incoming value is one of the PHI nodes in BB...
+    // If this incoming value is one of the PHI nodes in BB, the new entries in
+    // the PHI node are the entries from the old PHI.
     if (isa<PHINode>(OldVal) && cast<PHINode>(OldVal)->getParent() == BB) {
       PHINode *OldValPN = cast<PHINode>(OldVal);
-      for (std::vector<BasicBlock*>::const_iterator PredI = BBPreds.begin(), 
-             End = BBPreds.end(); PredI != End; ++PredI) {
-        PN->addIncoming(OldValPN->getIncomingValueForBlock(*PredI), *PredI);
-      }
+      for (unsigned i = 0, e = OldValPN->getNumIncomingValues(); i != e; ++i)
+        PN->addIncoming(OldValPN->getIncomingValue(i),
+                        OldValPN->getIncomingBlock(i));
     } else {
       for (std::vector<BasicBlock*>::const_iterator PredI = BBPreds.begin(), 
              End = BBPreds.end(); PredI != End; ++PredI) {
@@ -182,36 +185,71 @@ 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) {
-  if (Instruction *I = dyn_cast<Instruction>(V)) {
-    BasicBlock *PBB = I->getParent();
-    // If this instruction is defined in a block that contains an unconditional
-    // branch to BB, then it must be in the 'conditional' part of the "if
-    // statement".
-    if (isa<BranchInst>(PBB->getTerminator()) && 
-        cast<BranchInst>(PBB->getTerminator())->isUnconditional() && 
-        cast<BranchInst>(PBB->getTerminator())->getSuccessor(0) == BB)
-      return false;
-
-    // We also don't want to allow wierd loops that might have the "if
-    // condition" in the bottom of this block.
-    if (PBB == BB) return false;
-  }
+static bool DominatesMergePoint(Value *V, BasicBlock *BB, bool AllowAggressive){
+  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
+  // the bottom of this block.
+  if (PBB == BB) return false;
+
+  // If this instruction is defined in a block that contains an unconditional
+  // branch to BB, then it must be in the 'conditional' part of the "if
+  // statement".
+  if (BranchInst *BI = dyn_cast<BranchInst>(PBB->getTerminator()))
+    if (BI->isUnconditional() && BI->getSuccessor(0) == BB) {
+      if (!AllowAggressive) 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.
+      switch (I->getOpcode()) {
+      default: return false;  // Cannot hoist this out safely.
+      case Instruction::Load:
+        // We can hoist loads that are non-volatile and obviously cannot trap.
+        if (cast<LoadInst>(I)->isVolatile())
+          return false;
+        if (!isa<AllocaInst>(I->getOperand(0)) &&
+            !isa<Constant>(I->getOperand(0)))
+          return false;
+
+        // Finally, we have to check to make sure there are no instructions
+        // before the load in its basic block, as we are going to hoist the loop
+        // out to its predecessor.
+        if (PBB->begin() != BasicBlock::iterator(I))
+          return false;
+        break;
+      case Instruction::Add:
+      case Instruction::Sub:
+      case Instruction::And:
+      case Instruction::Or:
+      case Instruction::Xor:
+      case Instruction::Shl:
+      case Instruction::Shr:
+        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))
+          return false;
+      // Okay, it's safe to do this!
+    }
 
-  // Non-instructions all dominate instructions.
   return true;
 }
 
 // GatherConstantSetEQs - Given a potentially 'or'd together collection of seteq
 // instructions that compare a value against a constant, return the value being
 // compared, and stick the constant into the Values vector.
-static Value *GatherConstantSetEQs(Value *V, std::vector<Constant*> &Values) {
+static Value *GatherConstantSetEQs(Value *V, std::vector<ConstantInt*> &Values){
   if (Instruction *Inst = dyn_cast<Instruction>(V))
     if (Inst->getOpcode() == Instruction::SetEQ) {
-      if (Constant *C = dyn_cast<Constant>(Inst->getOperand(1))) {
+      if (ConstantInt *C = dyn_cast<ConstantInt>(Inst->getOperand(1))) {
         Values.push_back(C);
         return Inst->getOperand(0);
-      } else if (Constant *C = dyn_cast<Constant>(Inst->getOperand(0))) {
+      } else if (ConstantInt *C = dyn_cast<ConstantInt>(Inst->getOperand(0))) {
         Values.push_back(C);
         return Inst->getOperand(1);
       }
@@ -227,20 +265,20 @@ static Value *GatherConstantSetEQs(Value *V, std::vector<Constant*> &Values) {
 // GatherConstantSetNEs - Given a potentially 'and'd together collection of
 // setne instructions that compare a value against a constant, return the value
 // being compared, and stick the constant into the Values vector.
-static Value *GatherConstantSetNEs(Value *V, std::vector<Constant*> &Values) {
+static Value *GatherConstantSetNEs(Value *V, std::vector<ConstantInt*> &Values){
   if (Instruction *Inst = dyn_cast<Instruction>(V))
     if (Inst->getOpcode() == Instruction::SetNE) {
-      if (Constant *C = dyn_cast<Constant>(Inst->getOperand(1))) {
+      if (ConstantInt *C = dyn_cast<ConstantInt>(Inst->getOperand(1))) {
         Values.push_back(C);
         return Inst->getOperand(0);
-      } else if (Constant *C = dyn_cast<Constant>(Inst->getOperand(0))) {
+      } else if (ConstantInt *C = dyn_cast<ConstantInt>(Inst->getOperand(0))) {
         Values.push_back(C);
         return Inst->getOperand(1);
       }
     } else if (Inst->getOpcode() == Instruction::Cast) {
       // Cast of X to bool is really a comparison against zero.
       assert(Inst->getType() == Type::BoolTy && "Can only handle bool values!");
-      Values.push_back(Constant::getNullValue(Inst->getOperand(0)->getType()));
+      Values.push_back(ConstantInt::get(Inst->getOperand(0)->getType(), 0));
       return Inst->getOperand(0);
     } else if (Inst->getOpcode() == Instruction::And) {
       if (Value *LHS = GatherConstantSetNEs(Inst->getOperand(0), Values))
@@ -257,7 +295,7 @@ static Value *GatherConstantSetNEs(Value *V, std::vector<Constant*> &Values) {
 /// bunch of comparisons of one value against constants, return the value and
 /// the constants being compared.
 static bool GatherValueComparisons(Instruction *Cond, Value *&CompVal,
-                                   std::vector<Constant*> &Values) {
+                                   std::vector<ConstantInt*> &Values) {
   if (Cond->getOpcode() == Instruction::Or) {
     CompVal = GatherConstantSetEQs(Cond, Values);
 
@@ -294,7 +332,7 @@ 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
+  // 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();
@@ -314,7 +352,7 @@ static bool SafeToMergeTerminators(TerminatorInst *SI1, TerminatorInst *SI2) {
 /// 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, and existing predecessor of Succ.
+/// 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) !=
@@ -497,6 +535,17 @@ static bool FoldValueComparisonIntoPredecessors(TerminatorInst *TI) {
   return Changed;
 }
 
+namespace {
+  /// ConstantIntOrdering - This class implements a stable ordering of constant
+  /// integers that does not depend on their address.  This is important for
+  /// applications that sort ConstantInt's to ensure uniqueness.
+  struct ConstantIntOrdering {
+    bool operator()(const ConstantInt *LHS, const ConstantInt *RHS) const {
+      return LHS->getRawValue() < RHS->getRawValue();
+    }
+  };
+}
+
 
 // SimplifyCFG - This function is used to do simplification of a CFG.  For
 // example, it adjusts branches to branches to eliminate the extra hop, it
@@ -516,7 +565,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
   // Remove basic blocks that have no predecessors... which are unreachable.
   if (pred_begin(BB) == pred_end(BB) ||
       *pred_begin(BB) == BB && ++pred_begin(BB) == pred_end(BB)) {
-    //cerr << "Removing BB: \n" << BB;
+    DEBUG(std::cerr << "Removing BB: \n" << *BB);
 
     // Loop through all of our successors and make sure they know that one
     // of their predecessors is going away.
@@ -564,7 +613,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
         // we cannot do this transformation!
         //
        if (!PropagatePredecessorsForPHIs(BB, Succ)) {
-          //cerr << "Killing Trivial BB: \n" << BB;
+          DEBUG(std::cerr << "Killing Trivial BB: \n" << *BB);
           std::string OldName = BB->getName();
 
           std::vector<BasicBlock*>
@@ -589,7 +638,6 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
               // 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]);
@@ -603,8 +651,6 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
        
           if (!OldName.empty() && !Succ->hasName())  // Transfer name if we can
             Succ->setName(OldName);
-          
-          //cerr << "Function after removal: \n" << M;
           return true;
        }
       }
@@ -613,16 +659,23 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
 
   // If this is a returning block with only PHI nodes in it, fold the return
   // instruction into any unconditional branch predecessors.
+  //
+  // If any predecessor is a conditional branch that just selects among
+  // different return values, fold the replace the branch/return with a select
+  // and return.
   if (ReturnInst *RI = dyn_cast<ReturnInst>(BB->getTerminator())) {
     BasicBlock::iterator BBI = BB->getTerminator();
     if (BBI == BB->begin() || isa<PHINode>(--BBI)) {
-      // Find predecessors that end with unconditional branches.
+      // Find predecessors that end with branches.
       std::vector<BasicBlock*> UncondBranchPreds;
+      std::vector<BranchInst*> CondBranchPreds;
       for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {
         TerminatorInst *PTI = (*PI)->getTerminator();
         if (BranchInst *BI = dyn_cast<BranchInst>(PTI))
           if (BI->isUnconditional())
             UncondBranchPreds.push_back(*PI);
+          else
+            CondBranchPreds.push_back(BI);
       }
       
       // If we found some, do the transformation!
@@ -654,16 +707,75 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
 
         return true;
       }
+
+      // Check out all of the conditional branches going to this return
+      // instruction.  If any of them just select between returns, change the
+      // branch itself into a select/return pair.
+      while (!CondBranchPreds.empty()) {
+        BranchInst *BI = CondBranchPreds.back();
+        CondBranchPreds.pop_back();
+        BasicBlock *TrueSucc = BI->getSuccessor(0);
+        BasicBlock *FalseSucc = BI->getSuccessor(1);
+        BasicBlock *OtherSucc = TrueSucc == BB ? FalseSucc : TrueSucc;
+
+        // Check to see if the non-BB successor is also a return block.
+        if (isa<ReturnInst>(OtherSucc->getTerminator())) {
+          // Check to see if there are only PHI instructions in this block.
+          BasicBlock::iterator OSI = OtherSucc->getTerminator();
+          if (OSI == OtherSucc->begin() || isa<PHINode>(--OSI)) {
+            // Okay, we found a branch that is going to two return nodes.  If
+            // there is no return value for this function, just change the
+            // branch into a return.
+            if (RI->getNumOperands() == 0) {
+              TrueSucc->removePredecessor(BI->getParent());
+              FalseSucc->removePredecessor(BI->getParent());
+              new ReturnInst(0, BI);
+              BI->getParent()->getInstList().erase(BI);
+              return true;
+            }
+
+            // Otherwise, figure out what the true and false return values are
+            // so we can insert a new select instruction.
+            Value *TrueValue = TrueSucc->getTerminator()->getOperand(0);
+            Value *FalseValue = FalseSucc->getTerminator()->getOperand(0);
+
+            // Unwrap any PHI nodes in the return blocks.
+            if (PHINode *TVPN = dyn_cast<PHINode>(TrueValue))
+              if (TVPN->getParent() == TrueSucc)
+                TrueValue = TVPN->getIncomingValueForBlock(BI->getParent());
+            if (PHINode *FVPN = dyn_cast<PHINode>(FalseValue))
+              if (FVPN->getParent() == FalseSucc)
+                FalseValue = FVPN->getIncomingValueForBlock(BI->getParent());
+
+            TrueSucc->removePredecessor(BI->getParent());
+            FalseSucc->removePredecessor(BI->getParent());
+
+            // Insert a new select instruction.
+            Value *NewRetVal = new SelectInst(BI->getCondition(), TrueValue,
+                                              FalseValue, "retval", BI);
+            new ReturnInst(NewRetVal, BI);
+            BI->getParent()->getInstList().erase(BI);
+            return true;
+          }
+        }
+      }
     }
   } else if (UnwindInst *UI = dyn_cast<UnwindInst>(BB->begin())) {
     // Check to see if the first instruction in this block is just an unwind.
     // If so, replace any invoke instructions which use this as an exception
-    // destination with call instructions.
+    // destination with call instructions, and any unconditional branch
+    // predecessor with an unwind.
     //
     std::vector<BasicBlock*> Preds(pred_begin(BB), pred_end(BB));
     while (!Preds.empty()) {
       BasicBlock *Pred = Preds.back();
-      if (InvokeInst *II = dyn_cast<InvokeInst>(Pred->getTerminator()))
+      if (BranchInst *BI = dyn_cast<BranchInst>(Pred->getTerminator())) {
+        if (BI->isUnconditional()) {
+          Pred->getInstList().pop_back();  // nuke uncond branch
+          new UnwindInst(Pred);            // Use unwind.
+          Changed = true;
+        }
+      } else if (InvokeInst *II = dyn_cast<InvokeInst>(Pred->getTerminator()))
         if (II->getUnwindDest() == BB) {
           // Insert a new branch instruction before the invoke, because this
           // is now a fall through...
@@ -691,17 +803,100 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
     }
 
   } else if (SwitchInst *SI = dyn_cast<SwitchInst>(BB->begin())) {
-    if (FoldValueComparisonIntoPredecessors(SI))
-      return SimplifyCFG(BB) || 1;
+    if (isValueEqualityComparison(SI))
+      if (FoldValueComparisonIntoPredecessors(SI))
+        return SimplifyCFG(BB) || 1;
   } else if (BranchInst *BI = dyn_cast<BranchInst>(BB->getTerminator())) {
-    if (Value *CompVal = isValueEqualityComparison(BB->getTerminator())) {
-      // This block must be empty, except for the setcond inst, if it exists.
-      BasicBlock::iterator I = BB->begin();
-      if (&*I == BI ||
-          (&*I == cast<Instruction>(BI->getCondition()) &&
-           &*++I == BI))
-        if (FoldValueComparisonIntoPredecessors(BI))
-          return SimplifyCFG(BB) || 1;
+    if (BI->isConditional()) {
+      if (Value *CompVal = isValueEqualityComparison(BI)) {
+        // This block must be empty, except for the setcond inst, if it exists.
+        BasicBlock::iterator I = BB->begin();
+        if (&*I == BI ||
+            (&*I == cast<Instruction>(BI->getCondition()) &&
+             &*++I == BI))
+          if (FoldValueComparisonIntoPredecessors(BI))
+            return SimplifyCFG(BB) | true;
+      }
+
+      // If this basic block is ONLY a setcc and a branch, and if a predecessor
+      // branches to us and one of our successors, fold the setcc into the
+      // predecessor and use logical operations to pick the right destination.
+      BasicBlock *TrueDest  = BI->getSuccessor(0);
+      BasicBlock *FalseDest = BI->getSuccessor(1);
+      if (BinaryOperator *Cond = dyn_cast<BinaryOperator>(BI->getCondition()))
+        if (Cond->getParent() == BB && &BB->front() == Cond &&
+            Cond->getNext() == BI && Cond->hasOneUse() &&
+            TrueDest != BB && FalseDest != BB)
+          for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI!=E; ++PI)
+            if (BranchInst *PBI = dyn_cast<BranchInst>((*PI)->getTerminator()))
+              if (PBI->isConditional() && SafeToMergeTerminators(BI, PBI)) {
+                BasicBlock *PredBlock = *PI;
+                if (PBI->getSuccessor(0) == FalseDest ||
+                    PBI->getSuccessor(1) == TrueDest) {
+                  // Invert the predecessors condition test (xor it with true),
+                  // which allows us to write this code once.
+                  Value *NewCond =
+                    BinaryOperator::createNot(PBI->getCondition(),
+                                    PBI->getCondition()->getName()+".not", PBI);
+                  PBI->setCondition(NewCond);
+                  BasicBlock *OldTrue = PBI->getSuccessor(0);
+                  BasicBlock *OldFalse = PBI->getSuccessor(1);
+                  PBI->setSuccessor(0, OldFalse);
+                  PBI->setSuccessor(1, OldTrue);
+                }
+
+                if (PBI->getSuccessor(0) == TrueDest ||
+                    PBI->getSuccessor(1) == FalseDest) {
+                  // Clone Cond into the predecessor basic block, and or/and the
+                  // two conditions together.
+                  Instruction *New = Cond->clone();
+                  New->setName(Cond->getName());
+                  Cond->setName(Cond->getName()+".old");
+                  PredBlock->getInstList().insert(PBI, New);
+                  Instruction::BinaryOps Opcode =
+                    PBI->getSuccessor(0) == TrueDest ?
+                    Instruction::Or : Instruction::And;
+                  Value *NewCond = 
+                    BinaryOperator::create(Opcode, PBI->getCondition(),
+                                           New, "bothcond", PBI);
+                  PBI->setCondition(NewCond);
+                  if (PBI->getSuccessor(0) == BB) {
+                    AddPredecessorToBlock(TrueDest, PredBlock, BB);
+                    PBI->setSuccessor(0, TrueDest);
+                  }
+                  if (PBI->getSuccessor(1) == BB) {
+                    AddPredecessorToBlock(FalseDest, PredBlock, BB);
+                    PBI->setSuccessor(1, FalseDest);
+                  }
+                  return SimplifyCFG(BB) | 1;
+                }
+              }
+
+      // 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 (BranchInst *PBI = dyn_cast<BranchInst>(OnlyPred->getTerminator()))
+          if (PBI->isConditional() &&
+              PBI->getCondition() == BI->getCondition() &&
+              (PBI->getSuccessor(0) != BB || PBI->getSuccessor(1) != BB)) {
+            // Okay, the outcome of this conditional branch is statically
+            // knowable.  Delete the outgoing CFG edge that is impossible to
+            // execute.
+            bool CondIsTrue = PBI->getSuccessor(0) == BB;
+            BI->getSuccessor(CondIsTrue)->removePredecessor(BB);
+            new BranchInst(BI->getSuccessor(!CondIsTrue), BB);
+            BB->getInstList().erase(BI);
+            return SimplifyCFG(BB) | true;
+          }
     }
   }
 
@@ -716,7 +911,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
       OnlyPred = 0;       // There are multiple different predecessors...
       break;
     }
-  
+
   BasicBlock *OnlySucc = 0;
   if (OnlyPred && OnlyPred != BB &&    // Don't break self loops
       OnlyPred->getTerminator()->getOpcode() != Instruction::Invoke) {
@@ -731,7 +926,7 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
   }
 
   if (OnlySucc) {
-    //cerr << "Merging: " << BB << "into: " << OnlyPred;
+    DEBUG(std::cerr << "Merging: " << *BB << "into: " << *OnlyPred);
     TerminatorInst *Term = OnlyPred->getTerminator();
 
     // Resolve any PHI nodes at the start of the block.  They are all
@@ -775,12 +970,12 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
         // If this is a bunch of seteq's or'd together, or if it's a bunch of
         // 'setne's and'ed together, collect them.
         Value *CompVal = 0;
-        std::vector<Constant*> Values;
+        std::vector<ConstantInt*> Values;
         bool TrueWhenEqual = GatherValueComparisons(Cond, CompVal, Values);
         if (CompVal && CompVal->getType()->isInteger()) {
           // There might be duplicate constants in the list, which the switch
           // instruction can't handle, remove them now.
-          std::sort(Values.begin(), Values.end());
+          std::sort(Values.begin(), Values.end(), ConstantIntOrdering());
           Values.erase(std::unique(Values.begin(), Values.end()), Values.end());
           
           // Figure out which block is which destination.
@@ -828,8 +1023,8 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
       //
       BasicBlock *IfTrue, *IfFalse;
       if (Value *IfCond = GetIfCondition(BB, IfTrue, IfFalse)) {
-        //std::cerr << "FOUND IF CONDITION!  " << *IfCond << "  T: "
-        //       << IfTrue->getName() << "  F: " << IfFalse->getName() << "\n";
+        DEBUG(std::cerr << "FOUND IF CONDITION!  " << *IfCond << "  T: "
+              << IfTrue->getName() << "  F: " << IfFalse->getName() << "\n");
 
         // Figure out where to insert instructions as necessary.
         BasicBlock::iterator AfterPHIIt = BB->begin();
@@ -844,91 +1039,40 @@ bool llvm::SimplifyCFG(BasicBlock *BB) {
           // incoming values are defined in the conditional parts of the branch,
           // so check for this.
           //
-          if (DominatesMergePoint(PN->getIncomingValue(0), BB) &&
-              DominatesMergePoint(PN->getIncomingValue(1), BB)) {
+          if (DominatesMergePoint(PN->getIncomingValue(0), BB, true) &&
+              DominatesMergePoint(PN->getIncomingValue(1), BB, true)) {
             Value *TrueVal =
               PN->getIncomingValue(PN->getIncomingBlock(0) == IfFalse);
             Value *FalseVal =
               PN->getIncomingValue(PN->getIncomingBlock(0) == IfTrue);
 
-            // FIXME: when we have a 'select' statement, we can be completely
-            // generic and clean here and let the instcombine pass clean up
-            // after us, by folding the select instructions away when possible.
-            //
-            if (TrueVal == FalseVal) {
-              // Degenerate case...
-              PN->replaceAllUsesWith(TrueVal);
-              BB->getInstList().erase(PN);
-              Changed = true;
-            } else if (isa<ConstantBool>(TrueVal) &&
-                       isa<ConstantBool>(FalseVal)) {
-              if (TrueVal == ConstantBool::True) {
-                // The PHI node produces the same thing as the condition.
-                PN->replaceAllUsesWith(IfCond);
-              } else {
-                // The PHI node produces the inverse of the condition.  Insert a
-                // "NOT" instruction, which is really a XOR.
-                Value *InverseCond =
-                  BinaryOperator::createNot(IfCond, IfCond->getName()+".inv",
-                                            AfterPHIIt);
-                PN->replaceAllUsesWith(InverseCond);
-              }
-              BB->getInstList().erase(PN);
-              Changed = true;
-            } else if (isa<ConstantInt>(TrueVal) && isa<ConstantInt>(FalseVal)){
-              // If this is a PHI of two constant integers, we insert a cast of
-              // the boolean to the integer type in question, giving us 0 or 1.
-              // Then we multiply this by the difference of the two constants,
-              // giving us 0 if false, and the difference if true.  We add this
-              // result to the base constant, giving us our final value.  We
-              // rely on the instruction combiner to eliminate many special
-              // cases, like turning multiplies into shifts when possible.
-              std::string Name = PN->getName(); PN->setName("");
-              Value *TheCast = new CastInst(IfCond, TrueVal->getType(),
-                                            Name, AfterPHIIt);
-              Constant *TheDiff = ConstantExpr::get(Instruction::Sub,
-                                                    cast<Constant>(TrueVal),
-                                                    cast<Constant>(FalseVal));
-              Value *V = TheCast;
-              if (TheDiff != ConstantInt::get(TrueVal->getType(), 1))
-                V = BinaryOperator::create(Instruction::Mul, TheCast,
-                                           TheDiff, TheCast->getName()+".scale",
-                                           AfterPHIIt);
-              if (!cast<Constant>(FalseVal)->isNullValue())
-                V = BinaryOperator::create(Instruction::Add, V, FalseVal,
-                                           V->getName()+".offs", AfterPHIIt);
-              PN->replaceAllUsesWith(V);
-              BB->getInstList().erase(PN);
-              Changed = true;
-            } else if (isa<ConstantInt>(FalseVal) &&
-                       cast<Constant>(FalseVal)->isNullValue()) {
-              // If the false condition is an integral zero value, we can
-              // compute the PHI by multiplying the condition by the other
-              // value.
-              std::string Name = PN->getName(); PN->setName("");
-              Value *TheCast = new CastInst(IfCond, TrueVal->getType(),
-                                            Name+".c", AfterPHIIt);
-              Value *V = BinaryOperator::create(Instruction::Mul, TrueVal,
-                                                TheCast, Name, AfterPHIIt);
-              PN->replaceAllUsesWith(V);
-              BB->getInstList().erase(PN);
-              Changed = true;
-            } else if (isa<ConstantInt>(TrueVal) &&
-                       cast<Constant>(TrueVal)->isNullValue()) {
-              // If the true condition is an integral zero value, we can compute
-              // the PHI by multiplying the inverse condition by the other
-              // value.
-              std::string Name = PN->getName(); PN->setName("");
-              Value *NotCond = BinaryOperator::createNot(IfCond, Name+".inv",
-                                                         AfterPHIIt);
-              Value *TheCast = new CastInst(NotCond, TrueVal->getType(),
-                                            Name+".inv", AfterPHIIt);
-              Value *V = BinaryOperator::create(Instruction::Mul, FalseVal,
-                                                TheCast, Name, AfterPHIIt);
-              PN->replaceAllUsesWith(V);
-              BB->getInstList().erase(PN);
-              Changed = true;
+            // 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));
+            BB->getInstList().erase(PN);
+            Changed = true;
           }
         }
       }