Re-commit my previous SSAUpdater changes. The previous version naively tried

to determine where to place PHIs by iteratively comparing reaching definitions
at each block.  That was just plain wrong.  This version now computes the
dominator tree within the subset of the CFG where PHIs may need to be placed,
and then places the PHIs in the iterated dominance frontier of each definition.
The rest of the patch is mostly the same, with a few more performance
improvements added in.

llvm-svn: 101612

include/llvm/Transforms/Utils/SSAUpdater.h

@@ -21,28 +21,31 @@ namespace llvm {
   class PHINode;
   template<typename T>
   class SmallVectorImpl;
+  class BumpPtrAllocator;
 
 /// SSAUpdater - This class updates SSA form for a set of values defined in
 /// multiple blocks.  This is used when code duplication or another unstructured
 /// transformation wants to rewrite a set of uses of one value with uses of a
 /// set of values.
 class SSAUpdater {
+public:
+  class BBInfo;
+  typedef SmallVectorImpl<BBInfo*> BlockListTy;
+
+private:
   /// AvailableVals - This keeps track of which value to use on a per-block
-  /// basis.  When we insert PHI nodes, we keep track of them here.  We use
+  /// basis.  When we insert PHI nodes, we keep track of them here.
-  /// TrackingVH's for the value of the map because we RAUW PHI nodes when we
+  //typedef DenseMap<BasicBlock*, Value*> AvailableValsTy;
-  /// eliminate them, and want the TrackingVH's to track this.
-  //typedef DenseMap<BasicBlock*, TrackingVH<Value> > AvailableValsTy;
   void *AV;
 
   /// PrototypeValue is an arbitrary representative value, which we derive names
   /// and a type for PHI nodes.
   Value *PrototypeValue;
 
-  /// IncomingPredInfo - We use this as scratch space when doing our recursive
+  /// BBMap - The GetValueAtEndOfBlock method maintains this mapping from
-  /// walk.  This should only be used in GetValueInBlockInternal, normally it
+  /// basic blocks to BBInfo structures.
-  /// should be empty.
+  /// typedef DenseMap<BasicBlock*, BBInfo*> BBMapTy;
-  //std::vector<std::pair<BasicBlock*, TrackingVH<Value> > > IncomingPredInfo;
+  void *BM;
-  void *IPI;
 
   /// InsertedPHIs - If this is non-null, the SSAUpdater adds all PHI nodes that
   /// it creates to the vector.
@@ -99,6 +102,15 @@ public:
 
 private:
   Value *GetValueAtEndOfBlockInternal(BasicBlock *BB);
+  void BuildBlockList(BasicBlock *BB, BlockListTy *BlockList,
+                      BumpPtrAllocator *Allocator);
+  void FindDominators(BlockListTy *BlockList);
+  void FindPHIPlacement(BlockListTy *BlockList);
+  void FindAvailableVals(BlockListTy *BlockList);
+  void FindExistingPHI(BasicBlock *BB, BlockListTy *BlockList);
+  bool CheckIfPHIMatches(PHINode *PHI);
+  void RecordMatchingPHI(PHINode *PHI);
+
   void operator=(const SSAUpdater&); // DO NOT IMPLEMENT
   SSAUpdater(const SSAUpdater&);     // DO NOT IMPLEMENT
 };

lib/Transforms/Utils/SSAUpdater.cpp

@@ -11,34 +11,52 @@
 //
 //===----------------------------------------------------------------------===//
 
+#define DEBUG_TYPE "ssaupdater"
 #include "llvm/Transforms/Utils/SSAUpdater.h"
 #include "llvm/Instructions.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/AlignOf.h"
+#include "llvm/Support/Allocator.h"
 #include "llvm/Support/CFG.h"
 #include "llvm/Support/Debug.h"
-#include "llvm/Support/ValueHandle.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
-typedef DenseMap<BasicBlock*, TrackingVH<Value> > AvailableValsTy;
+/// BBInfo - Per-basic block information used internally by SSAUpdater.
-typedef std::vector<std::pair<BasicBlock*, TrackingVH<Value> > >
+/// The predecessors of each block are cached here since pred_iterator is
-                IncomingPredInfoTy;
+/// slow and we need to iterate over the blocks at least a few times.
+class SSAUpdater::BBInfo {
+public:
+  BasicBlock *BB;      // Back-pointer to the corresponding block.
+  Value *AvailableVal; // Value to use in this block.
+  BBInfo *DefBB;       // Block that defines the available value.
+  int BlkNum;          // Postorder number.
+  BBInfo *IDom;        // Immediate dominator.
+  unsigned NumPreds;   // Number of predecessor blocks.
+  BBInfo **Preds;      // Array[NumPreds] of predecessor blocks.
+  PHINode *PHITag;     // Marker for existing PHIs that match.
 
+  BBInfo(BasicBlock *ThisBB, Value *V)
+    : BB(ThisBB), AvailableVal(V), DefBB(V ? this : 0), BlkNum(0), IDom(0),
+      NumPreds(0), Preds(0), PHITag(0) { }
+};
+
+typedef DenseMap<BasicBlock*, SSAUpdater::BBInfo*> BBMapTy;
+
+typedef DenseMap<BasicBlock*, Value*> AvailableValsTy;
 static AvailableValsTy &getAvailableVals(void *AV) {
   return *static_cast<AvailableValsTy*>(AV);
 }
 
-static IncomingPredInfoTy &getIncomingPredInfo(void *IPI) {
+static BBMapTy *getBBMap(void *BM) {
-  return *static_cast<IncomingPredInfoTy*>(IPI);
+  return static_cast<BBMapTy*>(BM);
 }
 
-
 SSAUpdater::SSAUpdater(SmallVectorImpl<PHINode*> *NewPHI)
-  : AV(0), PrototypeValue(0), IPI(0), InsertedPHIs(NewPHI) {}
+  : AV(0), PrototypeValue(0), BM(0), InsertedPHIs(NewPHI) {}
 
 SSAUpdater::~SSAUpdater() {
   delete &getAvailableVals(AV);
-  delete &getIncomingPredInfo(IPI);
 }
 
 /// Initialize - Reset this object to get ready for a new set of SSA
@@ -48,11 +66,6 @@ void SSAUpdater::Initialize(Value *ProtoValue) {
     AV = new AvailableValsTy();
   else
     getAvailableVals(AV).clear();
-
-  if (IPI == 0)
-    IPI = new IncomingPredInfoTy();
-  else
-    getIncomingPredInfo(IPI).clear();
   PrototypeValue = ProtoValue;
 }
 
@@ -73,7 +86,7 @@ void SSAUpdater::AddAvailableValue(BasicBlock *BB, Value *V) {
 
 /// IsEquivalentPHI - Check if PHI has the same incoming value as specified
 /// in ValueMapping for each predecessor block.
-static bool IsEquivalentPHI(PHINode *PHI, 
+static bool IsEquivalentPHI(PHINode *PHI,
                             DenseMap<BasicBlock*, Value*> &ValueMapping) {
   unsigned PHINumValues = PHI->getNumIncomingValues();
   if (PHINumValues != ValueMapping.size())
@@ -89,38 +102,12 @@ static bool IsEquivalentPHI(PHINode *PHI,
   return true;
 }
 
-/// GetExistingPHI - Check if BB already contains a phi node that is equivalent
-/// to the specified mapping from predecessor blocks to incoming values.
-static Value *GetExistingPHI(BasicBlock *BB,
-                             DenseMap<BasicBlock*, Value*> &ValueMapping) {
-  PHINode *SomePHI;
-  for (BasicBlock::iterator It = BB->begin();
-       (SomePHI = dyn_cast<PHINode>(It)); ++It) {
-    if (IsEquivalentPHI(SomePHI, ValueMapping))
-      return SomePHI;
-  }
-  return 0;
-}
-
-/// GetExistingPHI - Check if BB already contains an equivalent phi node.
-/// The InputIt type must be an iterator over std::pair<BasicBlock*, Value*>
-/// objects that specify the mapping from predecessor blocks to incoming values.
-template<typename InputIt>
-static Value *GetExistingPHI(BasicBlock *BB, const InputIt &I,
-                             const InputIt &E) {
-  // Avoid create the mapping if BB has no phi nodes at all.
-  if (!isa<PHINode>(BB->begin()))
-    return 0;
-  DenseMap<BasicBlock*, Value*> ValueMapping(I, E);
-  return GetExistingPHI(BB, ValueMapping);
-}
-
 /// GetValueAtEndOfBlock - Construct SSA form, materializing a value that is
 /// live at the end of the specified block.
 Value *SSAUpdater::GetValueAtEndOfBlock(BasicBlock *BB) {
-  assert(getIncomingPredInfo(IPI).empty() && "Unexpected Internal State");
+  assert(BM == 0 && "Unexpected Internal State");
   Value *Res = GetValueAtEndOfBlockInternal(BB);
-  assert(getIncomingPredInfo(IPI).empty() && "Unexpected Internal State");
+  assert(BM == 0 && "Unexpected Internal State");
   return Res;
 }
 
@@ -146,7 +133,7 @@ Value *SSAUpdater::GetValueAtEndOfBlock(BasicBlock *BB) {
 Value *SSAUpdater::GetValueInMiddleOfBlock(BasicBlock *BB) {
   // If there is no definition of the renamed variable in this block, just use
   // GetValueAtEndOfBlock to do our work.
-  if (!getAvailableVals(AV).count(BB))
+  if (!HasValueForBlock(BB))
     return GetValueAtEndOfBlock(BB);
 
   // Otherwise, we have the hard case.  Get the live-in values for each
@@ -193,10 +180,18 @@ Value *SSAUpdater::GetValueInMiddleOfBlock(BasicBlock *BB) {
   if (SingularValue != 0)
     return SingularValue;
 
-  // Otherwise, we do need a PHI.
+  // Otherwise, we do need a PHI: check to see if we already have one available
-  if (Value *ExistingPHI = GetExistingPHI(BB, PredValues.begin(),
+  // in this block that produces the right value.
-                                          PredValues.end()))
+  if (isa<PHINode>(BB->begin())) {
-    return ExistingPHI;
+    DenseMap<BasicBlock*, Value*> ValueMapping(PredValues.begin(),
+                                               PredValues.end());
+    PHINode *SomePHI;
+    for (BasicBlock::iterator It = BB->begin();
+         (SomePHI = dyn_cast<PHINode>(It)); ++It) {
+      if (IsEquivalentPHI(SomePHI, ValueMapping))
+        return SomePHI;
+    }
+  }
 
   // Ok, we have no way out, insert a new one now.
   PHINode *InsertedPHI = PHINode::Create(PrototypeValue->getType(),
@@ -226,7 +221,7 @@ Value *SSAUpdater::GetValueInMiddleOfBlock(BasicBlock *BB) {
 /// which use their value in the corresponding predecessor.
 void SSAUpdater::RewriteUse(Use &U) {
   Instruction *User = cast<Instruction>(U.getUser());
-  
+
   Value *V;
   if (PHINode *UserPN = dyn_cast<PHINode>(User))
     V = GetValueAtEndOfBlock(UserPN->getIncomingBlock(U));
@@ -236,161 +231,421 @@ void SSAUpdater::RewriteUse(Use &U) {
   U.set(V);
 }
 
-
 /// GetValueAtEndOfBlockInternal - Check to see if AvailableVals has an entry
 /// for the specified BB and if so, return it.  If not, construct SSA form by
-/// walking predecessors inserting PHI nodes as needed until we get to a block
+/// first calculating the required placement of PHIs and then inserting new
-/// where the value is available.
+/// PHIs where needed.
-///
 Value *SSAUpdater::GetValueAtEndOfBlockInternal(BasicBlock *BB) {
   AvailableValsTy &AvailableVals = getAvailableVals(AV);
+  if (Value *V = AvailableVals[BB])
+    return V;
 
-  // Query AvailableVals by doing an insertion of null.
+  // Pool allocation used internally by GetValueAtEndOfBlock.
-  std::pair<AvailableValsTy::iterator, bool> InsertRes =
+  BumpPtrAllocator Allocator;
-    AvailableVals.insert(std::make_pair(BB, TrackingVH<Value>()));
+  BBMapTy BBMapObj;
+  BM = &BBMapObj;
 
-  // Handle the case when the insertion fails because we have already seen BB.
+  SmallVector<BBInfo*, 100> BlockList;
-  if (!InsertRes.second) {
+  BuildBlockList(BB, &BlockList, &Allocator);
-    // If the insertion failed, there are two cases.  The first case is that the
-    // value is already available for the specified block.  If we get this, just
-    // return the value.
-    if (InsertRes.first->second != 0)
-      return InsertRes.first->second;
 
-    // Otherwise, if the value we find is null, then this is the value is not
+  // Special case: bail out if BB is unreachable.
-    // known but it is being computed elsewhere in our recursion.  This means
+  if (BlockList.size() == 0) {
-    // that we have a cycle.  Handle this by inserting a PHI node and returning
+    BM = 0;
-    // it.  When we get back to the first instance of the recursion we will fill
+    return UndefValue::get(PrototypeValue->getType());
-    // in the PHI node.
-    return InsertRes.first->second =
-      PHINode::Create(PrototypeValue->getType(), PrototypeValue->getName(),
-                      &BB->front());
   }
 
-  // Okay, the value isn't in the map and we just inserted a null in the entry
+  FindDominators(&BlockList);
-  // to indicate that we're processing the block.  Since we have no idea what
+  FindPHIPlacement(&BlockList);
-  // value is in this block, we have to recurse through our predecessors.
+  FindAvailableVals(&BlockList);
-  //
-  // While we're walking our predecessors, we keep track of them in a vector,
-  // then insert a PHI node in the end if we actually need one.  We could use a
-  // smallvector here, but that would take a lot of stack space for every level
-  // of the recursion, just use IncomingPredInfo as an explicit stack.
-  IncomingPredInfoTy &IncomingPredInfo = getIncomingPredInfo(IPI);
-  unsigned FirstPredInfoEntry = IncomingPredInfo.size();
 
-  // As we're walking the predecessors, keep track of whether they are all
+  BM = 0;
-  // producing the same value.  If so, this value will capture it, if not, it
+  return BBMapObj[BB]->DefBB->AvailableVal;
-  // will get reset to null.  We distinguish the no-predecessor case explicitly
+}
-  // below.
-  TrackingVH<Value> ExistingValue;
 
-  // We can get our predecessor info by walking the pred_iterator list, but it
+/// FindPredecessorBlocks - Put the predecessors of Info->BB into the Preds
-  // is relatively slow.  If we already have PHI nodes in this block, walk one
+/// vector, set Info->NumPreds, and allocate space in Info->Preds.
-  // of them to get the predecessor list instead.
+static void FindPredecessorBlocks(SSAUpdater::BBInfo *Info,
+                                  SmallVectorImpl<BasicBlock*> *Preds,
+                                  BumpPtrAllocator *Allocator) {
+  // We can get our predecessor info by walking the pred_iterator list,
+  // but it is relatively slow.  If we already have PHI nodes in this
+  // block, walk one of them to get the predecessor list instead.
+  BasicBlock *BB = Info->BB;
   if (PHINode *SomePhi = dyn_cast<PHINode>(BB->begin())) {
-    for (unsigned i = 0, e = SomePhi->getNumIncomingValues(); i != e; ++i) {
+    for (unsigned PI = 0, E = SomePhi->getNumIncomingValues(); PI != E; ++PI)
-      BasicBlock *PredBB = SomePhi->getIncomingBlock(i);
+      Preds->push_back(SomePhi->getIncomingBlock(PI));
-      Value *PredVal = GetValueAtEndOfBlockInternal(PredBB);
-      IncomingPredInfo.push_back(std::make_pair(PredBB, PredVal));
-
-      // Set ExistingValue to singular value from all predecessors so far.
-      if (i == 0)
-        ExistingValue = PredVal;
-      else if (PredVal != ExistingValue)
-        ExistingValue = 0;
-    }
   } else {
-    bool isFirstPred = true;
+    for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI)
-    for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {
+      Preds->push_back(*PI);
-      BasicBlock *PredBB = *PI;
+  }
-      Value *PredVal = GetValueAtEndOfBlockInternal(PredBB);
-      IncomingPredInfo.push_back(std::make_pair(PredBB, PredVal));
 
-      // Set ExistingValue to singular value from all predecessors so far.
+  Info->NumPreds = Preds->size();
-      if (isFirstPred) {
+  Info->Preds = static_cast<SSAUpdater::BBInfo**>
-        ExistingValue = PredVal;
+    (Allocator->Allocate(Info->NumPreds * sizeof(SSAUpdater::BBInfo*),
-        isFirstPred = false;
+                         AlignOf<SSAUpdater::BBInfo*>::Alignment));
-      } else if (PredVal != ExistingValue)
+}
-        ExistingValue = 0;
+
+/// BuildBlockList - Starting from the specified basic block, traverse back
+/// through its predecessors until reaching blocks with known values.  Create
+/// BBInfo structures for the blocks and append them to the block list.
+void SSAUpdater::BuildBlockList(BasicBlock *BB, BlockListTy *BlockList,
+                                BumpPtrAllocator *Allocator) {
+  AvailableValsTy &AvailableVals = getAvailableVals(AV);
+  BBMapTy *BBMap = getBBMap(BM);
+  SmallVector<BBInfo*, 10> RootList;
+  SmallVector<BBInfo*, 64> WorkList;
+
+  BBInfo *Info = new (*Allocator) BBInfo(BB, 0);
+  (*BBMap)[BB] = Info;
+  WorkList.push_back(Info);
+
+  // Search backward from BB, creating BBInfos along the way and stopping when
+  // reaching blocks that define the value.  Record those defining blocks on
+  // the RootList.
+  SmallVector<BasicBlock*, 10> Preds;
+  while (!WorkList.empty()) {
+    Info = WorkList.pop_back_val();
+    Preds.clear();
+    FindPredecessorBlocks(Info, &Preds, Allocator);
+
+    // Treat an unreachable predecessor as a definition with 'undef'.
+    if (Info->NumPreds == 0) {
+      Info->AvailableVal = UndefValue::get(PrototypeValue->getType());
+      Info->DefBB = Info;
+      RootList.push_back(Info);
+      continue;
+    }
+
+    for (unsigned p = 0; p != Info->NumPreds; ++p) {
+      BasicBlock *Pred = Preds[p];
+      // Check if BBMap already has a BBInfo for the predecessor block.
+      BBMapTy::value_type &BBMapBucket = BBMap->FindAndConstruct(Pred);
+      if (BBMapBucket.second) {
+        Info->Preds[p] = BBMapBucket.second;
+        continue;
+      }
+
+      // Create a new BBInfo for the predecessor.
+      Value *PredVal = AvailableVals.lookup(Pred);
+      BBInfo *PredInfo = new (*Allocator) BBInfo(Pred, PredVal);
+      BBMapBucket.second = PredInfo;
+      Info->Preds[p] = PredInfo;
+
+      if (PredInfo->AvailableVal) {
+        RootList.push_back(PredInfo);
+        continue;
+      }
+      WorkList.push_back(PredInfo);
     }
   }
 
-  // If there are no predecessors, then we must have found an unreachable block
+  // Now that we know what blocks are backwards-reachable from the starting
-  // just return 'undef'.  Since there are no predecessors, InsertRes must not
+  // block, do a forward depth-first traversal to assign postorder numbers
-  // be invalidated.
+  // to those blocks.
-  if (IncomingPredInfo.size() == FirstPredInfoEntry)
+  BBInfo *PseudoEntry = new (*Allocator) BBInfo(0, 0);
-    return InsertRes.first->second = UndefValue::get(PrototypeValue->getType());
+  unsigned BlkNum = 1;
 
-  /// Look up BB's entry in AvailableVals.  'InsertRes' may be invalidated.  If
+  // Initialize the worklist with the roots from the backward traversal.
-  /// this block is involved in a loop, a no-entry PHI node will have been
+  while (!RootList.empty()) {
-  /// inserted as InsertedVal.  Otherwise, we'll still have the null we inserted
+    Info = RootList.pop_back_val();
-  /// above.
+    Info->IDom = PseudoEntry;
-  TrackingVH<Value> &InsertedVal = AvailableVals[BB];
+    Info->BlkNum = -1;
-
+    WorkList.push_back(Info);
-  // If the predecessor values are not all the same, then check to see if there
-  // is an existing PHI that can be used.
-  if (!ExistingValue)
-    ExistingValue = GetExistingPHI(BB,
-                                   IncomingPredInfo.begin()+FirstPredInfoEntry,
-                                   IncomingPredInfo.end());
-
-  // If there is an existing value we can use, then we don't need to insert a
-  // PHI.  This is the simple and common case.
-  if (ExistingValue) {
-    // If a PHI node got inserted, replace it with the existing value and delete
-    // it.
-    if (InsertedVal) {
-      PHINode *OldVal = cast<PHINode>(InsertedVal);
-      // Be careful about dead loops.  These RAUW's also update InsertedVal.
-      if (InsertedVal != ExistingValue)
-        OldVal->replaceAllUsesWith(ExistingValue);
-      else
-        OldVal->replaceAllUsesWith(UndefValue::get(InsertedVal->getType()));
-      OldVal->eraseFromParent();
-    } else {
-      InsertedVal = ExistingValue;
-    }
-
-    // Either path through the 'if' should have set InsertedVal -> ExistingVal.
-    assert((InsertedVal == ExistingValue || isa<UndefValue>(InsertedVal)) &&
-           "RAUW didn't change InsertedVal to be ExistingValue");
-
-    // Drop the entries we added in IncomingPredInfo to restore the stack.
-    IncomingPredInfo.erase(IncomingPredInfo.begin()+FirstPredInfoEntry,
-                           IncomingPredInfo.end());
-    return ExistingValue;
   }
 
-  // Otherwise, we do need a PHI: insert one now if we don't already have one.
+  while (!WorkList.empty()) {
-  if (InsertedVal == 0)
+    Info = WorkList.back();
-    InsertedVal = PHINode::Create(PrototypeValue->getType(),
-                                  PrototypeValue->getName(), &BB->front());
 
-  PHINode *InsertedPHI = cast<PHINode>(InsertedVal);
+    if (Info->BlkNum == -2) {
-  InsertedPHI->reserveOperandSpace(IncomingPredInfo.size()-FirstPredInfoEntry);
+      // All the successors have been handled; assign the postorder number.
+      Info->BlkNum = BlkNum++;
+      // If not a root, put it on the BlockList.
+      if (!Info->AvailableVal)
+        BlockList->push_back(Info);
+      WorkList.pop_back();
+      continue;
+    }
 
-  // Fill in all the predecessors of the PHI.
+    // Leave this entry on the worklist, but set its BlkNum to mark that its
-  for (IncomingPredInfoTy::iterator I =
+    // successors have been put on the worklist.  When it returns to the top
-         IncomingPredInfo.begin()+FirstPredInfoEntry,
+    // the list, after handling its successors, it will be assigned a number.
-       E = IncomingPredInfo.end(); I != E; ++I)
+    Info->BlkNum = -2;
-    InsertedPHI->addIncoming(I->second, I->first);
 
-  // Drop the entries we added in IncomingPredInfo to restore the stack.
+    // Add unvisited successors to the work list.
-  IncomingPredInfo.erase(IncomingPredInfo.begin()+FirstPredInfoEntry,
+    for (succ_iterator SI = succ_begin(Info->BB), E = succ_end(Info->BB);
-                         IncomingPredInfo.end());
+         SI != E; ++SI) {
+      BBInfo *SuccInfo = (*BBMap)[*SI];
+      if (!SuccInfo || SuccInfo->BlkNum)
+        continue;
+      SuccInfo->BlkNum = -1;
+      WorkList.push_back(SuccInfo);
+    }
+  }
+  PseudoEntry->BlkNum = BlkNum;
+}
 
-  // See if the PHI node can be merged to a single value.  This can happen in
+/// IntersectDominators - This is the dataflow lattice "meet" operation for
-  // loop cases when we get a PHI of itself and one other value.
+/// finding dominators.  Given two basic blocks, it walks up the dominator
-  if (Value *ConstVal = InsertedPHI->hasConstantValue()) {
+/// tree until it finds a common dominator of both.  It uses the postorder
-    InsertedPHI->replaceAllUsesWith(ConstVal);
+/// number of the blocks to determine how to do that.
-    InsertedPHI->eraseFromParent();
+static SSAUpdater::BBInfo *IntersectDominators(SSAUpdater::BBInfo *Blk1,
-    InsertedVal = ConstVal;
+                                               SSAUpdater::BBInfo *Blk2) {
-  } else {
+  while (Blk1 != Blk2) {
-    DEBUG(dbgs() << "  Inserted PHI: " << *InsertedPHI << "\n");
+    while (Blk1->BlkNum < Blk2->BlkNum) {
+      Blk1 = Blk1->IDom;
+      if (!Blk1)
+        return Blk2;
+    }
+    while (Blk2->BlkNum < Blk1->BlkNum) {
+      Blk2 = Blk2->IDom;
+      if (!Blk2)
+        return Blk1;
+    }
+  }
+  return Blk1;
+}
+
+/// FindDominators - Calculate the dominator tree for the subset of the CFG
+/// corresponding to the basic blocks on the BlockList.  This uses the
+/// algorithm from: "A Simple, Fast Dominance Algorithm" by Cooper, Harvey and
+/// Kennedy, published in Software--Practice and Experience, 2001, 4:1-10.
+/// Because the CFG subset does not include any edges leading into blocks that
+/// define the value, the results are not the usual dominator tree.  The CFG
+/// subset has a single pseudo-entry node with edges to a set of root nodes
+/// for blocks that define the value.  The dominators for this subset CFG are
+/// not the standard dominators but they are adequate for placing PHIs within
+/// the subset CFG.
+void SSAUpdater::FindDominators(BlockListTy *BlockList) {
+  bool Changed;
+  do {
+    Changed = false;
+    // Iterate over the list in reverse order, i.e., forward on CFG edges.
+    for (BlockListTy::reverse_iterator I = BlockList->rbegin(),
+           E = BlockList->rend(); I != E; ++I) {
+      BBInfo *Info = *I;
+
+      // Start with the first predecessor.
+      assert(Info->NumPreds > 0 && "unreachable block");
+      BBInfo *NewIDom = Info->Preds[0];
+
+      // Iterate through the block's other predecessors.
+      for (unsigned p = 1; p != Info->NumPreds; ++p) {
+        BBInfo *Pred = Info->Preds[p];
+        NewIDom = IntersectDominators(NewIDom, Pred);
+      }
+
+      // Check if the IDom value has changed.
+      if (NewIDom != Info->IDom) {
+        Info->IDom = NewIDom;
+        Changed = true;
+      }
+    }
+  } while (Changed);
+}
+
+/// IsDefInDomFrontier - Search up the dominator tree from Pred to IDom for
+/// any blocks containing definitions of the value.  If one is found, then the
+/// successor of Pred is in the dominance frontier for the definition, and
+/// this function returns true.
+static bool IsDefInDomFrontier(const SSAUpdater::BBInfo *Pred,
+                               const SSAUpdater::BBInfo *IDom) {
+  for (; Pred != IDom; Pred = Pred->IDom) {
+    if (Pred->DefBB == Pred)
+      return true;
+  }
+  return false;
+}
+
+/// FindPHIPlacement - PHIs are needed in the iterated dominance frontiers of
+/// the known definitions.  Iteratively add PHIs in the dom frontiers until
+/// nothing changes.  Along the way, keep track of the nearest dominating
+/// definitions for non-PHI blocks.
+void SSAUpdater::FindPHIPlacement(BlockListTy *BlockList) {
+  bool Changed;
+  do {
+    Changed = false;
+    // Iterate over the list in reverse order, i.e., forward on CFG edges.
+    for (BlockListTy::reverse_iterator I = BlockList->rbegin(),
+           E = BlockList->rend(); I != E; ++I) {
+      BBInfo *Info = *I;
+
+      // If this block already needs a PHI, there is nothing to do here.
+      if (Info->DefBB == Info)
+        continue;
+
+      // Default to use the same def as the immediate dominator.
+      BBInfo *NewDefBB = Info->IDom->DefBB;
+      for (unsigned p = 0; p != Info->NumPreds; ++p) {
+        if (IsDefInDomFrontier(Info->Preds[p], Info->IDom)) {
+          // Need a PHI here.
+          NewDefBB = Info;
+          break;
+        }
+      }
+
+      // Check if anything changed.
+      if (NewDefBB != Info->DefBB) {
+        Info->DefBB = NewDefBB;
+        Changed = true;
+      }
+    }
+  } while (Changed);
+}
+
+/// FindAvailableVal - If this block requires a PHI, first check if an existing
+/// PHI matches the PHI placement and reaching definitions computed earlier,
+/// and if not, create a new PHI.  Visit all the block's predecessors to
+/// calculate the available value for each one and fill in the incoming values
+/// for a new PHI.
+void SSAUpdater::FindAvailableVals(BlockListTy *BlockList) {
+  AvailableValsTy &AvailableVals = getAvailableVals(AV);
+
+  // Go through the worklist in forward order (i.e., backward through the CFG)
+  // and check if existing PHIs can be used.  If not, create empty PHIs where
+  // they are needed.
+  for (BlockListTy::iterator I = BlockList->begin(), E = BlockList->end();
+       I != E; ++I) {
+    BBInfo *Info = *I;
+    // Check if there needs to be a PHI in BB.
+    if (Info->DefBB != Info)
+      continue;
+
+    // Look for an existing PHI.
+    FindExistingPHI(Info->BB, BlockList);
+    if (Info->AvailableVal)
+      continue;
+
+    PHINode *PHI = PHINode::Create(PrototypeValue->getType(),
+                                   PrototypeValue->getName(),
+                                   &Info->BB->front());
+    PHI->reserveOperandSpace(Info->NumPreds);
+    Info->AvailableVal = PHI;
+    AvailableVals[Info->BB] = PHI;
+  }
+
+  // Now go back through the worklist in reverse order to fill in the arguments
+  // for any new PHIs added in the forward traversal.
+  for (BlockListTy::reverse_iterator I = BlockList->rbegin(),
+         E = BlockList->rend(); I != E; ++I) {
+    BBInfo *Info = *I;
+
+    // Check if this block contains a newly added PHI.
+    if (Info->DefBB != Info)
+      continue;
+    PHINode *PHI = dyn_cast<PHINode>(Info->AvailableVal);
+    if (!PHI || PHI->getNumIncomingValues() == Info->NumPreds)
+      continue;
+
+    // Iterate through the block's predecessors.
+    for (unsigned p = 0; p != Info->NumPreds; ++p) {
+      BBInfo *PredInfo = Info->Preds[p];
+      BasicBlock *Pred = PredInfo->BB;
+      // Skip to the nearest preceding definition.
+      if (PredInfo->DefBB != PredInfo)
+        PredInfo = PredInfo->DefBB;
+      PHI->addIncoming(PredInfo->AvailableVal, Pred);
+    }
+
+    DEBUG(dbgs() << "  Inserted PHI: " << *PHI << "\n");
 
     // If the client wants to know about all new instructions, tell it.
-    if (InsertedPHIs) InsertedPHIs->push_back(InsertedPHI);
+    if (InsertedPHIs) InsertedPHIs->push_back(PHI);
+  }
+}
+
+/// FindExistingPHI - Look through the PHI nodes in a block to see if any of
+/// them match what is needed.
+void SSAUpdater::FindExistingPHI(BasicBlock *BB, BlockListTy *BlockList) {
+  PHINode *SomePHI;
+  for (BasicBlock::iterator It = BB->begin();
+       (SomePHI = dyn_cast<PHINode>(It)); ++It) {
+    if (CheckIfPHIMatches(SomePHI)) {
+      RecordMatchingPHI(SomePHI);
+      break;
+    }
+    // Match failed: clear all the PHITag values.
+    for (BlockListTy::iterator I = BlockList->begin(), E = BlockList->end();
+         I != E; ++I)
+      (*I)->PHITag = 0;
+  }
+}
+
+/// CheckIfPHIMatches - Check if a PHI node matches the placement and values
+/// in the BBMap.
+bool SSAUpdater::CheckIfPHIMatches(PHINode *PHI) {
+  BBMapTy *BBMap = getBBMap(BM);
+  SmallVector<PHINode*, 20> WorkList;
+  WorkList.push_back(PHI);
+
+  // Mark that the block containing this PHI has been visited.
+  (*BBMap)[PHI->getParent()]->PHITag = PHI;
+
+  while (!WorkList.empty()) {
+    PHI = WorkList.pop_back_val();
+
+    // Iterate through the PHI's incoming values.
+    for (unsigned i = 0, e = PHI->getNumIncomingValues(); i != e; ++i) {
+      Value *IncomingVal = PHI->getIncomingValue(i);
+      BBInfo *PredInfo = (*BBMap)[PHI->getIncomingBlock(i)];
+      // Skip to the nearest preceding definition.
+      if (PredInfo->DefBB != PredInfo)
+        PredInfo = PredInfo->DefBB;
+
+      // Check if it matches the expected value.
+      if (PredInfo->AvailableVal) {
+        if (IncomingVal == PredInfo->AvailableVal)
+          continue;
+        return false;
+      }
+
+      // Check if the value is a PHI in the correct block.
+      PHINode *IncomingPHIVal = dyn_cast<PHINode>(IncomingVal);
+      if (!IncomingPHIVal || IncomingPHIVal->getParent() != PredInfo->BB)
+        return false;
+
+      // If this block has already been visited, check if this PHI matches.
+      if (PredInfo->PHITag) {
+        if (IncomingPHIVal == PredInfo->PHITag)
+          continue;
+        return false;
+      }
+      PredInfo->PHITag = IncomingPHIVal;
+
+      WorkList.push_back(IncomingPHIVal);
+    }
+  }
+  return true;
+}
+
+/// RecordMatchingPHI - For a PHI node that matches, record it and its input
+/// PHIs in both the BBMap and the AvailableVals mapping.
+void SSAUpdater::RecordMatchingPHI(PHINode *PHI) {
+  BBMapTy *BBMap = getBBMap(BM);
+  AvailableValsTy &AvailableVals = getAvailableVals(AV);
+  SmallVector<PHINode*, 20> WorkList;
+  WorkList.push_back(PHI);
+
+  // Record this PHI.
+  BasicBlock *BB = PHI->getParent();
+  AvailableVals[BB] = PHI;
+  (*BBMap)[BB]->AvailableVal = PHI;
+
+  while (!WorkList.empty()) {
+    PHI = WorkList.pop_back_val();
+
+    // Iterate through the PHI's incoming values.
+    for (unsigned i = 0, e = PHI->getNumIncomingValues(); i != e; ++i) {
+      PHINode *IncomingPHIVal = dyn_cast<PHINode>(PHI->getIncomingValue(i));
+      if (!IncomingPHIVal) continue;
+      BB = IncomingPHIVal->getParent();
+      BBInfo *Info = (*BBMap)[BB];
+      if (!Info || Info->AvailableVal)
+        continue;
+
+      // Record the PHI and add it to the worklist.
+      AvailableVals[BB] = IncomingPHIVal;
+      Info->AvailableVal = IncomingPHIVal;
+      WorkList.push_back(IncomingPHIVal);
+    }
   }
-
-  return InsertedVal;
 }

test/Transforms/GVN/2010-03-31-RedundantPHIs.ll

@@ -0,0 +1,46 @@
+; RUN: opt < %s -gvn -enable-full-load-pre -S | FileCheck %s
+
+define i8* @cat(i8* %s1, ...) nounwind {
+entry:
+  br i1 undef, label %bb, label %bb3
+
+bb:                                               ; preds = %entry
+  unreachable
+
+bb3:                                              ; preds = %entry
+  store i8* undef, i8** undef, align 4
+  br i1 undef, label %bb5, label %bb6
+
+bb5:                                              ; preds = %bb3
+  unreachable
+
+bb6:                                              ; preds = %bb3
+  br label %bb12
+
+bb8:                                              ; preds = %bb12
+  br i1 undef, label %bb9, label %bb10
+
+bb9:                                              ; preds = %bb8
+  %0 = load i8** undef, align 4                   ; <i8*> [#uses=0]
+  %1 = load i8** undef, align 4                   ; <i8*> [#uses=0]
+  br label %bb11
+
+bb10:                                             ; preds = %bb8
+  br label %bb11
+
+bb11:                                             ; preds = %bb10, %bb9
+; CHECK: bb11:
+; CHECK: phi
+; CHECK-NOT: phi
+  br label %bb12
+
+bb12:                                             ; preds = %bb11, %bb6
+; CHECK: bb12:
+; CHECK: phi
+; CHECK-NOT: phi
+  br i1 undef, label %bb8, label %bb13
+
+bb13:                                             ; preds = %bb12
+; CHECK: bb13:
+  ret i8* undef
+}