This started as a small change, I swear. Unfortunately, lots of things call the...
[oota-llvm.git] / lib / Transforms / Scalar / JumpThreading.cpp
index ec35a891e05f9b3f124ee70253f84c78b567eb66..f3536c10c5756026dbd80355f011602882aae029 100644 (file)
 #define DEBUG_TYPE "jump-threading"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/IntrinsicInst.h"
+#include "llvm/LLVMContext.h"
 #include "llvm/Pass.h"
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/Statistic.h"
+#include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Target/TargetData.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
-#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Support/ValueHandle.h"
 using namespace llvm;
 
 STATISTIC(NumThreads, "Number of jumps threaded");
@@ -51,14 +57,29 @@ namespace {
   /// revectored to the false side of the second if.
   ///
   class VISIBILITY_HIDDEN JumpThreading : public FunctionPass {
+    TargetData *TD;
+#ifdef NDEBUG
+    SmallPtrSet<BasicBlock*, 16> LoopHeaders;
+#else
+    SmallSet<AssertingVH<BasicBlock>, 16> LoopHeaders;
+#endif
   public:
     static char ID; // Pass identification
     JumpThreading() : FunctionPass(&ID) {}
 
+    virtual void getAnalysisUsage(AnalysisUsage &AU) const {
+      AU.addRequired<TargetData>();
+    }
+
     bool runOnFunction(Function &F);
+    void FindLoopHeaders(Function &F);
+    
     bool ProcessBlock(BasicBlock *BB);
-    void ThreadEdge(BasicBlock *BB, BasicBlock *PredBB, BasicBlock *SuccBB);
-    BasicBlock *FactorCommonPHIPreds(PHINode *PN, Constant *CstVal);
+    bool ThreadEdge(BasicBlock *BB, BasicBlock *PredBB, BasicBlock *SuccBB,
+                    unsigned JumpThreadCost);
+    BasicBlock *FactorCommonPHIPreds(PHINode *PN, Value *Val);
+    bool ProcessBranchOnDuplicateCond(BasicBlock *PredBB, BasicBlock *DestBB);
+    bool ProcessSwitchOnDuplicateCond(BasicBlock *PredBB, BasicBlock *DestBB);
 
     bool ProcessJumpOnPHI(PHINode *PN);
     bool ProcessBranchOnLogical(Value *V, BasicBlock *BB, bool isAnd);
@@ -79,30 +100,74 @@ FunctionPass *llvm::createJumpThreadingPass() { return new JumpThreading(); }
 ///
 bool JumpThreading::runOnFunction(Function &F) {
   DOUT << "Jump threading on function '" << F.getNameStart() << "'\n";
+  TD = &getAnalysis<TargetData>();
+  
+  FindLoopHeaders(F);
   
   bool AnotherIteration = true, EverChanged = false;
   while (AnotherIteration) {
     AnotherIteration = false;
     bool Changed = false;
-    for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I)
-      while (ProcessBlock(I))
+    for (Function::iterator I = F.begin(), E = F.end(); I != E;) {
+      BasicBlock *BB = I;
+      while (ProcessBlock(BB))
         Changed = true;
+      
+      ++I;
+      
+      // If the block is trivially dead, zap it.  This eliminates the successor
+      // edges which simplifies the CFG.
+      if (pred_begin(BB) == pred_end(BB) &&
+          BB != &BB->getParent()->getEntryBlock()) {
+        DOUT << "  JT: Deleting dead block '" << BB->getNameStart()
+             << "' with terminator: " << *BB->getTerminator();
+        LoopHeaders.erase(BB);
+        DeleteDeadBlock(BB);
+        Changed = true;
+      }
+    }
     AnotherIteration = Changed;
     EverChanged |= Changed;
   }
+  
+  LoopHeaders.clear();
   return EverChanged;
 }
 
+/// FindLoopHeaders - We do not want jump threading to turn proper loop
+/// structures into irreducible loops.  Doing this breaks up the loop nesting
+/// hierarchy and pessimizes later transformations.  To prevent this from
+/// happening, we first have to find the loop headers.  Here we approximate this
+/// by finding targets of backedges in the CFG.
+///
+/// Note that there definitely are cases when we want to allow threading of
+/// edges across a loop header.  For example, threading a jump from outside the
+/// loop (the preheader) to an exit block of the loop is definitely profitable.
+/// It is also almost always profitable to thread backedges from within the loop
+/// to exit blocks, and is often profitable to thread backedges to other blocks
+/// within the loop (forming a nested loop).  This simple analysis is not rich
+/// enough to track all of these properties and keep it up-to-date as the CFG
+/// mutates, so we don't allow any of these transformations.
+///
+void JumpThreading::FindLoopHeaders(Function &F) {
+  SmallVector<std::pair<const BasicBlock*,const BasicBlock*>, 32> Edges;
+  FindFunctionBackedges(F, Edges);
+  
+  for (unsigned i = 0, e = Edges.size(); i != e; ++i)
+    LoopHeaders.insert(const_cast<BasicBlock*>(Edges[i].second));
+}
+
+
 /// FactorCommonPHIPreds - If there are multiple preds with the same incoming
 /// value for the PHI, factor them together so we get one block to thread for
 /// the whole group.
 /// This is important for things like "phi i1 [true, true, false, true, x]"
 /// where we only need to clone the block for the true blocks once.
 ///
-BasicBlock *JumpThreading::FactorCommonPHIPreds(PHINode *PN, Constant *CstVal) {
+BasicBlock *JumpThreading::FactorCommonPHIPreds(PHINode *PN, Value *Val) {
   SmallVector<BasicBlock*, 16> CommonPreds;
   for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i)
-    if (PN->getIncomingValue(i) == CstVal)
+    if (PN->getIncomingValue(i) == Val)
       CommonPreds.push_back(PN->getIncomingBlock(i));
   
   if (CommonPreds.size() == 1)
@@ -143,7 +208,7 @@ static unsigned getJumpThreadDuplicationCost(const BasicBlock *BB) {
     if (const CallInst *CI = dyn_cast<CallInst>(I)) {
       if (!isa<IntrinsicInst>(CI))
         Size += 3;
-      else if (isa<VectorType>(CI->getType()))
+      else if (!isa<VectorType>(CI->getType()))
         Size += 1;
     }
   }
@@ -164,8 +229,19 @@ bool JumpThreading::ProcessBlock(BasicBlock *BB) {
   // because now the condition in this block can be threaded through
   // predecessors of our predecessor block.
   if (BasicBlock *SinglePred = BB->getSinglePredecessor())
-    if (SinglePred->getTerminator()->getNumSuccessors() == 1) {
+    if (SinglePred->getTerminator()->getNumSuccessors() == 1 &&
+        SinglePred != BB) {
+      // If SinglePred was a loop header, BB becomes one.
+      if (LoopHeaders.erase(SinglePred))
+        LoopHeaders.insert(BB);
+      
+      // Remember if SinglePred was the entry block of the function.  If so, we
+      // will need to move BB back to the entry position.
+      bool isEntry = SinglePred == &SinglePred->getParent()->getEntryBlock();
       MergeBasicBlockIntoOnlyPred(BB);
+      
+      if (isEntry && BB != &BB->getParent()->getEntryBlock())
+        BB->moveBefore(&BB->getParent()->getEntryBlock());
       return true;
     }
   
@@ -193,33 +269,110 @@ bool JumpThreading::ProcessBlock(BasicBlock *BB) {
     return true;
   }
   
-  // If there is only a single predecessor of this block, nothing to fold.
-  if (BB->getSinglePredecessor())
-    return false;
+  // If the terminator is branching on an undef, we can pick any of the
+  // successors to branch to.  Since this is arbitrary, we pick the successor
+  // with the fewest predecessors.  This should reduce the in-degree of the
+  // others.
+  if (isa<UndefValue>(Condition)) {
+    TerminatorInst *BBTerm = BB->getTerminator();
+    unsigned MinSucc = 0;
+    BasicBlock *TestBB = BBTerm->getSuccessor(MinSucc);
+    // Compute the successor with the minimum number of predecessors.
+    unsigned MinNumPreds = std::distance(pred_begin(TestBB), pred_end(TestBB));
+    for (unsigned i = 1, e = BBTerm->getNumSuccessors(); i != e; ++i) {
+      TestBB = BBTerm->getSuccessor(i);
+      unsigned NumPreds = std::distance(pred_begin(TestBB), pred_end(TestBB));
+      if (NumPreds < MinNumPreds)
+        MinSucc = i;
+    }
+    
+    // Fold the branch/switch.
+    for (unsigned i = 0, e = BBTerm->getNumSuccessors(); i != e; ++i) {
+      if (i == MinSucc) continue;
+      BBTerm->getSuccessor(i)->removePredecessor(BB);
+    }
+    
+    DOUT << "  In block '" << BB->getNameStart()
+         << "' folding undef terminator: " << *BBTerm;
+    BranchInst::Create(BBTerm->getSuccessor(MinSucc), BBTerm);
+    BBTerm->eraseFromParent();
+    return true;
+  }
+  
+  Instruction *CondInst = dyn_cast<Instruction>(Condition);
+
+  // If the condition is an instruction defined in another block, see if a
+  // predecessor has the same condition:
+  //     br COND, BBX, BBY
+  //  BBX:
+  //     br COND, BBZ, BBW
+  if (!Condition->hasOneUse() && // Multiple uses.
+      (CondInst == 0 || CondInst->getParent() != BB)) { // Non-local definition.
+    pred_iterator PI = pred_begin(BB), E = pred_end(BB);
+    if (isa<BranchInst>(BB->getTerminator())) {
+      for (; PI != E; ++PI)
+        if (BranchInst *PBI = dyn_cast<BranchInst>((*PI)->getTerminator()))
+          if (PBI->isConditional() && PBI->getCondition() == Condition &&
+              ProcessBranchOnDuplicateCond(*PI, BB))
+            return true;
+    } else {
+      assert(isa<SwitchInst>(BB->getTerminator()) && "Unknown jump terminator");
+      for (; PI != E; ++PI)
+        if (SwitchInst *PSI = dyn_cast<SwitchInst>((*PI)->getTerminator()))
+          if (PSI->getCondition() == Condition &&
+              ProcessSwitchOnDuplicateCond(*PI, BB))
+            return true;
+    }
+  }
 
+  // All the rest of our checks depend on the condition being an instruction.
+  if (CondInst == 0)
+    return false;
+  
   // See if this is a phi node in the current block.
-  PHINode *PN = dyn_cast<PHINode>(Condition);
-  if (PN && PN->getParent() == BB)
-    return ProcessJumpOnPHI(PN);
+  if (PHINode *PN = dyn_cast<PHINode>(CondInst))
+    if (PN->getParent() == BB)
+      return ProcessJumpOnPHI(PN);
   
   // If this is a conditional branch whose condition is and/or of a phi, try to
   // simplify it.
-  if (BinaryOperator *CondI = dyn_cast<BinaryOperator>(Condition)) {
-    if ((CondI->getOpcode() == Instruction::And || 
-         CondI->getOpcode() == Instruction::Or) &&
-        isa<BranchInst>(BB->getTerminator()) &&
-        ProcessBranchOnLogical(CondI, BB,
-                               CondI->getOpcode() == Instruction::And))
-      return true;
-  }
+  if ((CondInst->getOpcode() == Instruction::And || 
+       CondInst->getOpcode() == Instruction::Or) &&
+      isa<BranchInst>(BB->getTerminator()) &&
+      ProcessBranchOnLogical(CondInst, BB,
+                             CondInst->getOpcode() == Instruction::And))
+    return true;
   
-  // If we have "br (phi != 42)" and the phi node has any constant values as 
-  // operands, we can thread through this block.
-  if (CmpInst *CondCmp = dyn_cast<CmpInst>(Condition))
-    if (isa<PHINode>(CondCmp->getOperand(0)) &&
-        isa<Constant>(CondCmp->getOperand(1)) &&
-        ProcessBranchOnCompare(CondCmp, BB))
-      return true;
+  if (CmpInst *CondCmp = dyn_cast<CmpInst>(CondInst)) {
+    if (isa<PHINode>(CondCmp->getOperand(0))) {
+      // If we have "br (phi != 42)" and the phi node has any constant values
+      // as operands, we can thread through this block.
+      // 
+      // If we have "br (cmp phi, x)" and the phi node contains x such that the
+      // comparison uniquely identifies the branch target, we can thread
+      // through this block.
+
+      if (ProcessBranchOnCompare(CondCmp, BB))
+        return true;      
+    }
+    
+    // If we have a comparison, loop over the predecessors to see if there is
+    // a condition with the same value.
+    pred_iterator PI = pred_begin(BB), E = pred_end(BB);
+    for (; PI != E; ++PI)
+      if (BranchInst *PBI = dyn_cast<BranchInst>((*PI)->getTerminator()))
+        if (PBI->isConditional() && *PI != BB) {
+          if (CmpInst *CI = dyn_cast<CmpInst>(PBI->getCondition())) {
+            if (CI->getOperand(0) == CondCmp->getOperand(0) &&
+                CI->getOperand(1) == CondCmp->getOperand(1) &&
+                CI->getPredicate() == CondCmp->getPredicate()) {
+              // TODO: Could handle things like (x != 4) --> (x == 17)
+              if (ProcessBranchOnDuplicateCond(*PI, BB))
+                return true;
+            }
+          }
+        }
+  }
 
   // Check for some cases that are worth simplifying.  Right now we want to look
   // for loads that are used by a switch or by the condition for the branch.  If
@@ -228,7 +381,7 @@ bool JumpThreading::ProcessBlock(BasicBlock *BB) {
   //
   // This is particularly important because reg2mem inserts loads and stores all
   // over the place, and this blocks jump threading if we don't zap them.
-  Value *SimplifyValue = Condition;
+  Value *SimplifyValue = CondInst;
   if (CmpInst *CondCmp = dyn_cast<CmpInst>(SimplifyValue))
     if (isa<Constant>(CondCmp->getOperand(1)))
       SimplifyValue = CondCmp->getOperand(0);
@@ -243,6 +396,140 @@ bool JumpThreading::ProcessBlock(BasicBlock *BB) {
   return false;
 }
 
+/// ProcessBranchOnDuplicateCond - We found a block and a predecessor of that
+/// block that jump on exactly the same condition.  This means that we almost
+/// always know the direction of the edge in the DESTBB:
+///  PREDBB:
+///     br COND, DESTBB, BBY
+///  DESTBB:
+///     br COND, BBZ, BBW
+///
+/// If DESTBB has multiple predecessors, we can't just constant fold the branch
+/// in DESTBB, we have to thread over it.
+bool JumpThreading::ProcessBranchOnDuplicateCond(BasicBlock *PredBB,
+                                                 BasicBlock *BB) {
+  BranchInst *PredBI = cast<BranchInst>(PredBB->getTerminator());
+  
+  // If both successors of PredBB go to DESTBB, we don't know anything.  We can
+  // fold the branch to an unconditional one, which allows other recursive
+  // simplifications.
+  bool BranchDir;
+  if (PredBI->getSuccessor(1) != BB)
+    BranchDir = true;
+  else if (PredBI->getSuccessor(0) != BB)
+    BranchDir = false;
+  else {
+    DOUT << "  In block '" << PredBB->getNameStart()
+         << "' folding terminator: " << *PredBB->getTerminator();
+    ++NumFolds;
+    ConstantFoldTerminator(PredBB);
+    return true;
+  }
+   
+  BranchInst *DestBI = cast<BranchInst>(BB->getTerminator());
+
+  // If the dest block has one predecessor, just fix the branch condition to a
+  // constant and fold it.
+  if (BB->getSinglePredecessor()) {
+    DOUT << "  In block '" << BB->getNameStart()
+         << "' folding condition to '" << BranchDir << "': "
+         << *BB->getTerminator();
+    ++NumFolds;
+    DestBI->setCondition(Context->getConstantInt(Type::Int1Ty, BranchDir));
+    ConstantFoldTerminator(BB);
+    return true;
+  }
+  
+  // Otherwise we need to thread from PredBB to DestBB's successor which
+  // involves code duplication.  Check to see if it is worth it.
+  unsigned JumpThreadCost = getJumpThreadDuplicationCost(BB);
+  if (JumpThreadCost > Threshold) {
+    DOUT << "  Not threading BB '" << BB->getNameStart()
+         << "' - Cost is too high: " << JumpThreadCost << "\n";
+    return false;
+  }
+  
+  // Next, figure out which successor we are threading to.
+  BasicBlock *SuccBB = DestBI->getSuccessor(!BranchDir);
+  
+  // Ok, try to thread it!
+  return ThreadEdge(BB, PredBB, SuccBB, JumpThreadCost);
+}
+
+/// ProcessSwitchOnDuplicateCond - We found a block and a predecessor of that
+/// block that switch on exactly the same condition.  This means that we almost
+/// always know the direction of the edge in the DESTBB:
+///  PREDBB:
+///     switch COND [... DESTBB, BBY ... ]
+///  DESTBB:
+///     switch COND [... BBZ, BBW ]
+///
+/// Optimizing switches like this is very important, because simplifycfg builds
+/// switches out of repeated 'if' conditions.
+bool JumpThreading::ProcessSwitchOnDuplicateCond(BasicBlock *PredBB,
+                                                 BasicBlock *DestBB) {
+  // Can't thread edge to self.
+  if (PredBB == DestBB)
+    return false;
+  
+  
+  SwitchInst *PredSI = cast<SwitchInst>(PredBB->getTerminator());
+  SwitchInst *DestSI = cast<SwitchInst>(DestBB->getTerminator());
+
+  // There are a variety of optimizations that we can potentially do on these
+  // blocks: we order them from most to least preferable.
+  
+  // If DESTBB *just* contains the switch, then we can forward edges from PREDBB
+  // directly to their destination.  This does not introduce *any* code size
+  // growth.  Skip debug info first.
+  BasicBlock::iterator BBI = DestBB->begin();
+  while (isa<DbgInfoIntrinsic>(BBI))
+    BBI++;
+  
+  // FIXME: Thread if it just contains a PHI.
+  if (isa<SwitchInst>(BBI)) {
+    bool MadeChange = false;
+    // Ignore the default edge for now.
+    for (unsigned i = 1, e = DestSI->getNumSuccessors(); i != e; ++i) {
+      ConstantInt *DestVal = DestSI->getCaseValue(i);
+      BasicBlock *DestSucc = DestSI->getSuccessor(i);
+      
+      // Okay, DestSI has a case for 'DestVal' that goes to 'DestSucc'.  See if
+      // PredSI has an explicit case for it.  If so, forward.  If it is covered
+      // by the default case, we can't update PredSI.
+      unsigned PredCase = PredSI->findCaseValue(DestVal);
+      if (PredCase == 0) continue;
+      
+      // If PredSI doesn't go to DestBB on this value, then it won't reach the
+      // case on this condition.
+      if (PredSI->getSuccessor(PredCase) != DestBB &&
+          DestSI->getSuccessor(i) != DestBB)
+        continue;
+
+      // Otherwise, we're safe to make the change.  Make sure that the edge from
+      // DestSI to DestSucc is not critical and has no PHI nodes.
+      DOUT << "FORWARDING EDGE " << *DestVal << "   FROM: " << *PredSI;
+      DOUT << "THROUGH: " << *DestSI;
+
+      // If the destination has PHI nodes, just split the edge for updating
+      // simplicity.
+      if (isa<PHINode>(DestSucc->begin()) && !DestSucc->getSinglePredecessor()){
+        SplitCriticalEdge(DestSI, i, this);
+        DestSucc = DestSI->getSuccessor(i);
+      }
+      FoldSingleEntryPHINodes(DestSucc);
+      PredSI->setSuccessor(PredCase, DestSucc);
+      MadeChange = true;
+    }
+    
+    if (MadeChange)
+      return true;
+  }
+  
+  return false;
+}
+
+
 /// SimplifyPartiallyRedundantLoad - If LI is an obviously partially redundant
 /// load instruction, eliminate it by replacing it with a PHI node.  This is an
 /// important optimization that encourages jump threading, and needs to be run
@@ -274,6 +561,10 @@ bool JumpThreading::SimplifyPartiallyRedundantLoad(LoadInst *LI) {
     // If the value if the load is locally available within the block, just use
     // it.  This frequently occurs for reg2mem'd allocas.
     //cerr << "LOAD ELIMINATED:\n" << *BBIt << *LI << "\n";
+    
+    // If the returned value is the load itself, replace with an undef. This can
+    // only happen in dead loops.
+    if (AvailableVal == LI) AvailableVal = Context->getUndef(LI->getType());
     LI->replaceAllUsesWith(AvailableVal);
     LI->eraseFromParent();
     return true;
@@ -365,7 +656,7 @@ bool JumpThreading::SimplifyPartiallyRedundantLoad(LoadInst *LI) {
   
   // Now we know that each predecessor of this block has a value in
   // AvailablePreds, sort them for efficient access as we're walking the preds.
-  std::sort(AvailablePreds.begin(), AvailablePreds.end());
+  array_pod_sort(AvailablePreds.begin(), AvailablePreds.end());
   
   // Create a PHI node at the start of the block for the PRE'd load value.
   PHINode *PN = PHINode::Create(LI->getType(), "", LoadBB->begin());
@@ -427,28 +718,14 @@ bool JumpThreading::ProcessJumpOnPHI(PHINode *PN) {
   // Next, figure out which successor we are threading to.
   BasicBlock *SuccBB;
   if (BranchInst *BI = dyn_cast<BranchInst>(BB->getTerminator()))
-    SuccBB = BI->getSuccessor(PredCst == ConstantInt::getFalse());
+    SuccBB = BI->getSuccessor(PredCst == Context->getConstantIntFalse());
   else {
     SwitchInst *SI = cast<SwitchInst>(BB->getTerminator());
     SuccBB = SI->getSuccessor(SI->findCaseValue(PredCst));
   }
   
-  // If threading to the same block as we come from, we would infinite loop.
-  if (SuccBB == BB) {
-    DOUT << "  Not threading BB '" << BB->getNameStart()
-         << "' - would thread to self!\n";
-    return false;
-  }
-  
-  // And finally, do it!
-  DOUT << "  Threading edge from '" << PredBB->getNameStart() << "' to '"
-       << SuccBB->getNameStart() << "' with cost: " << JumpThreadCost
-       << ", across block:\n    "
-       << *BB << "\n";
-       
-  ThreadEdge(BB, PredBB, SuccBB);
-  ++NumThreads;
-  return true;
+  // Ok, try to thread it!
+  return ThreadEdge(BB, PredBB, SuccBB, JumpThreadCost);
 }
 
 /// ProcessJumpOnLogicalPHI - PN's basic block contains a conditional branch
@@ -479,7 +756,7 @@ bool JumpThreading::ProcessBranchOnLogical(Value *V, BasicBlock *BB,
   // We can only do the simplification for phi nodes of 'false' with AND or
   // 'true' with OR.  See if we have any entries in the phi for this.
   unsigned PredNo = ~0U;
-  ConstantInt *PredCst = ConstantInt::get(Type::Int1Ty, !isAnd);
+  ConstantInt *PredCst = Context->getConstantInt(Type::Int1Ty, !isAnd);
   for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
     if (PN->getIncomingValue(i) == PredCst) {
       PredNo = i;
@@ -509,30 +786,35 @@ bool JumpThreading::ProcessBranchOnLogical(Value *V, BasicBlock *BB,
   // 'true' block.
   BasicBlock *SuccBB = BB->getTerminator()->getSuccessor(isAnd);
   
-  // If threading to the same block as we come from, we would infinite loop.
-  if (SuccBB == BB) {
-    DOUT << "  Not threading BB '" << BB->getNameStart()
-    << "' - would thread to self!\n";
-    return false;
-  }
-  
-  // And finally, do it!
-  DOUT << "  Threading edge through bool from '" << PredBB->getNameStart()
-       << "' to '" << SuccBB->getNameStart() << "' with cost: "
-       << JumpThreadCost << ", across block:\n    "
-       << *BB << "\n";
-  
-  ThreadEdge(BB, PredBB, SuccBB);
-  ++NumThreads;
-  return true;
+  // Ok, try to thread it!
+  return ThreadEdge(BB, PredBB, SuccBB, JumpThreadCost);
+}
+
+/// GetResultOfComparison - Given an icmp/fcmp predicate and the left and right
+/// hand sides of the compare instruction, try to determine the result. If the
+/// result can not be determined, a null pointer is returned.
+static Constant *GetResultOfComparison(CmpInst::Predicate pred,
+                                       Value *LHS, Value *RHS,
+                                       LLVMContext *Context) {
+  if (Constant *CLHS = dyn_cast<Constant>(LHS))
+    if (Constant *CRHS = dyn_cast<Constant>(RHS))
+      return Context->getConstantExprCompare(pred, CLHS, CRHS);
+
+  if (LHS == RHS)
+    if (isa<IntegerType>(LHS->getType()) || isa<PointerType>(LHS->getType()))
+      return ICmpInst::isTrueWhenEqual(pred) ? 
+                 Context->getConstantIntTrue() : Context->getConstantIntFalse();
+
+  return 0;
 }
 
 /// ProcessBranchOnCompare - We found a branch on a comparison between a phi
-/// node and a constant.  If the PHI node contains any constants as inputs, we
-/// can fold the compare for that edge and thread through it.
+/// node and a value.  If we can identify when the comparison is true between
+/// the phi inputs and the value, we can fold the compare for that edge and
+/// thread through it.
 bool JumpThreading::ProcessBranchOnCompare(CmpInst *Cmp, BasicBlock *BB) {
   PHINode *PN = cast<PHINode>(Cmp->getOperand(0));
-  Constant *RHS = cast<Constant>(Cmp->getOperand(1));
+  Value *RHS = Cmp->getOperand(1);
   
   // If the phi isn't in the current block, an incoming edge to this block
   // doesn't control the destination.
@@ -541,18 +823,18 @@ bool JumpThreading::ProcessBranchOnCompare(CmpInst *Cmp, BasicBlock *BB) {
   
   // We can do this simplification if any comparisons fold to true or false.
   // See if any do.
-  Constant *PredCst = 0;
+  Value *PredVal = 0;
   bool TrueDirection = false;
   for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
-    PredCst = dyn_cast<Constant>(PN->getIncomingValue(i));
-    if (PredCst == 0) continue;
+    PredVal = PN->getIncomingValue(i);
+    
+    Constant *Res = GetResultOfComparison(Cmp->getPredicate(), PredVal,
+                                          RHS, Context);
+    if (!Res) {
+      PredVal = 0;
+      continue;
+    }
     
-    Constant *Res;
-    if (ICmpInst *ICI = dyn_cast<ICmpInst>(Cmp))
-      Res = ConstantExpr::getICmp(ICI->getPredicate(), PredCst, RHS);
-    else
-      Res = ConstantExpr::getFCmp(cast<FCmpInst>(Cmp)->getPredicate(),
-                                  PredCst, RHS);
     // If this folded to a constant expr, we can't do anything.
     if (ConstantInt *ResC = dyn_cast<ConstantInt>(Res)) {
       TrueDirection = ResC->getZExtValue();
@@ -565,11 +847,11 @@ bool JumpThreading::ProcessBranchOnCompare(CmpInst *Cmp, BasicBlock *BB) {
     }
     
     // Otherwise, we can't fold this input.
-    PredCst = 0;
+    PredVal = 0;
   }
   
   // If no match, bail out.
-  if (PredCst == 0)
+  if (PredVal == 0)
     return false;
   
   // See if the cost of duplicating this block is low enough.
@@ -582,37 +864,45 @@ bool JumpThreading::ProcessBranchOnCompare(CmpInst *Cmp, BasicBlock *BB) {
   
   // If so, we can actually do this threading.  Merge any common predecessors
   // that will act the same.
-  BasicBlock *PredBB = FactorCommonPHIPreds(PN, PredCst);
+  BasicBlock *PredBB = FactorCommonPHIPreds(PN, PredVal);
   
   // Next, get our successor.
   BasicBlock *SuccBB = BB->getTerminator()->getSuccessor(!TrueDirection);
   
+  // Ok, try to thread it!
+  return ThreadEdge(BB, PredBB, SuccBB, JumpThreadCost);
+}
+
+
+/// ThreadEdge - We have decided that it is safe and profitable to thread an
+/// edge from PredBB to SuccBB across BB.  Transform the IR to reflect this
+/// change.
+bool JumpThreading::ThreadEdge(BasicBlock *BB, BasicBlock *PredBB, 
+                               BasicBlock *SuccBB, unsigned JumpThreadCost) {
+
   // If threading to the same block as we come from, we would infinite loop.
   if (SuccBB == BB) {
-    DOUT << "  Not threading BB '" << BB->getNameStart()
-    << "' - would thread to self!\n";
+    DOUT << "  Not threading across BB '" << BB->getNameStart()
+         << "' - would thread to self!\n";
     return false;
   }
   
-  
+  // If threading this would thread across a loop header, don't thread the edge.
+  // See the comments above FindLoopHeaders for justifications and caveats.
+  if (LoopHeaders.count(BB)) {
+    DOUT << "  Not threading from '" << PredBB->getNameStart()
+         << "' across loop header BB '" << BB->getNameStart()
+         << "' to dest BB '" << SuccBB->getNameStart()
+         << "' - it might create an irreducible loop!\n";
+    return false;
+  }
+
   // And finally, do it!
-  DOUT << "  Threading edge through bool from '" << PredBB->getNameStart()
-       << "' to '" << SuccBB->getNameStart() << "' with cost: "
-       << JumpThreadCost << ", across block:\n    "
+  DOUT << "  Threading edge from '" << PredBB->getNameStart() << "' to '"
+       << SuccBB->getNameStart() << "' with cost: " << JumpThreadCost
+       << ", across block:\n    "
        << *BB << "\n";
   
-  ThreadEdge(BB, PredBB, SuccBB);
-  ++NumThreads;
-  return true;
-}
-
-
-/// ThreadEdge - We have decided that it is safe and profitable to thread an
-/// edge from PredBB to SuccBB across BB.  Transform the IR to reflect this
-/// change.
-void JumpThreading::ThreadEdge(BasicBlock *BB, BasicBlock *PredBB, 
-                               BasicBlock *SuccBB) {
-
   // Jump Threading can not update SSA properties correctly if the values
   // defined in the duplicated block are used outside of the block itself.  For
   // this reason, we spill all values that are used outside of BB to the stack.
@@ -641,16 +931,18 @@ void JumpThreading::ThreadEdge(BasicBlock *BB, BasicBlock *PredBB,
   // Clone the non-phi instructions of BB into NewBB, keeping track of the
   // mapping and using it to remap operands in the cloned instructions.
   for (; !isa<TerminatorInst>(BI); ++BI) {
-    Instruction *New = BI->clone();
+    Instruction *New = BI->clone(*Context);
     New->setName(BI->getNameStart());
     NewBB->getInstList().push_back(New);
     ValueMapping[BI] = New;
    
     // Remap operands to patch up intra-block references.
     for (unsigned i = 0, e = New->getNumOperands(); i != e; ++i)
-      if (Instruction *Inst = dyn_cast<Instruction>(New->getOperand(i)))
-        if (Value *Remapped = ValueMapping[Inst])
-          New->setOperand(i, Remapped);
+      if (Instruction *Inst = dyn_cast<Instruction>(New->getOperand(i))) {
+        DenseMap<Instruction*, Value*>::iterator I = ValueMapping.find(Inst);
+        if (I != ValueMapping.end())
+          New->setOperand(i, I->second);
+      }
   }
   
   // We didn't copy the terminator from BB over to NewBB, because there is now
@@ -666,13 +958,15 @@ void JumpThreading::ThreadEdge(BasicBlock *BB, BasicBlock *PredBB,
     Value *IV = PN->getIncomingValueForBlock(BB);
     
     // Remap the value if necessary.
-    if (Instruction *Inst = dyn_cast<Instruction>(IV))
-      if (Value *MappedIV = ValueMapping[Inst])
-        IV = MappedIV;
+    if (Instruction *Inst = dyn_cast<Instruction>(IV)) {
+      DenseMap<Instruction*, Value*>::iterator I = ValueMapping.find(Inst);
+      if (I != ValueMapping.end())
+        IV = I->second;
+    }
     PN->addIncoming(IV, NewBB);
   }
   
-  // Finally, NewBB is good to go.  Update the terminator of PredBB to jump to
+  // Ok, NewBB is good to go.  Update the terminator of PredBB to jump to
   // NewBB instead of BB.  This eliminates predecessors from BB, which requires
   // us to simplify any PHI nodes in BB.
   TerminatorInst *PredTerm = PredBB->getTerminator();
@@ -681,4 +975,23 @@ void JumpThreading::ThreadEdge(BasicBlock *BB, BasicBlock *PredBB,
       BB->removePredecessor(PredBB);
       PredTerm->setSuccessor(i, NewBB);
     }
+  
+  // At this point, the IR is fully up to date and consistent.  Do a quick scan
+  // over the new instructions and zap any that are constants or dead.  This
+  // frequently happens because of phi translation.
+  BI = NewBB->begin();
+  for (BasicBlock::iterator E = NewBB->end(); BI != E; ) {
+    Instruction *Inst = BI++;
+    if (Constant *C = ConstantFoldInstruction(Inst, BB->getContext(), TD)) {
+      Inst->replaceAllUsesWith(C);
+      Inst->eraseFromParent();
+      continue;
+    }
+    
+    RecursivelyDeleteTriviallyDeadInstructions(Inst);
+  }
+  
+  // Threaded an edge!
+  ++NumThreads;
+  return true;
 }