diff --git a/lib/Transforms/Utils/SSAUpdater.cpp b/lib/Transforms/Utils/SSAUpdater.cpp index 161bf217852..a31235a1f5c 100644 --- a/lib/Transforms/Utils/SSAUpdater.cpp +++ b/lib/Transforms/Utils/SSAUpdater.cpp @@ -71,6 +71,50 @@ void SSAUpdater::AddAvailableValue(BasicBlock *BB, Value *V) { getAvailableVals(AV)[BB] = V; } +/// IsEquivalentPHI - Check if PHI has the same incoming value as specified +/// in ValueMapping for each predecessor block. +static bool IsEquivalentPHI(PHINode *PHI, + DenseMap &ValueMapping) { + unsigned PHINumValues = PHI->getNumIncomingValues(); + if (PHINumValues != ValueMapping.size()) + return false; + + // Scan the phi to see if it matches. + for (unsigned i = 0, e = PHINumValues; i != e; ++i) + if (ValueMapping[PHI->getIncomingBlock(i)] != + PHI->getIncomingValue(i)) { + return false; + } + + 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 &ValueMapping) { + PHINode *SomePHI; + for (BasicBlock::iterator It = BB->begin(); + (SomePHI = dyn_cast(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 +/// objects that specify the mapping from predecessor blocks to incoming values. +template +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(BB->begin())) + return 0; + DenseMap 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) { @@ -149,28 +193,11 @@ Value *SSAUpdater::GetValueInMiddleOfBlock(BasicBlock *BB) { if (SingularValue != 0) return SingularValue; - // Otherwise, we do need a PHI: check to see if we already have one available - // in this block that produces the right value. - if (isa(BB->begin())) { - DenseMap ValueMapping(PredValues.begin(), - PredValues.end()); - PHINode *SomePHI; - for (BasicBlock::iterator It = BB->begin(); - (SomePHI = dyn_cast(It)); ++It) { - // Scan this phi to see if it is what we need. - bool Equal = true; - for (unsigned i = 0, e = SomePHI->getNumIncomingValues(); i != e; ++i) - if (ValueMapping[SomePHI->getIncomingBlock(i)] != - SomePHI->getIncomingValue(i)) { - Equal = false; - break; - } - - if (Equal) - return SomePHI; - } - } - + // Otherwise, we do need a PHI. + if (Value *ExistingPHI = GetExistingPHI(BB, PredValues.begin(), + PredValues.end())) + return ExistingPHI; + // Ok, we have no way out, insert a new one now. PHINode *InsertedPHI = PHINode::Create(PrototypeValue->getType(), PrototypeValue->getName(), @@ -255,7 +282,7 @@ Value *SSAUpdater::GetValueAtEndOfBlockInternal(BasicBlock *BB) { // producing the same value. If so, this value will capture it, if not, it // will get reset to null. We distinguish the no-predecessor case explicitly // below. - TrackingVH SingularValue; + TrackingVH ExistingValue; // 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 @@ -266,11 +293,11 @@ Value *SSAUpdater::GetValueAtEndOfBlockInternal(BasicBlock *BB) { Value *PredVal = GetValueAtEndOfBlockInternal(PredBB); IncomingPredInfo.push_back(std::make_pair(PredBB, PredVal)); - // Compute SingularValue. + // Set ExistingValue to singular value from all predecessors so far. if (i == 0) - SingularValue = PredVal; - else if (PredVal != SingularValue) - SingularValue = 0; + ExistingValue = PredVal; + else if (PredVal != ExistingValue) + ExistingValue = 0; } } else { bool isFirstPred = true; @@ -279,12 +306,12 @@ Value *SSAUpdater::GetValueAtEndOfBlockInternal(BasicBlock *BB) { Value *PredVal = GetValueAtEndOfBlockInternal(PredBB); IncomingPredInfo.push_back(std::make_pair(PredBB, PredVal)); - // Compute SingularValue. + // Set ExistingValue to singular value from all predecessors so far. if (isFirstPred) { - SingularValue = PredVal; + ExistingValue = PredVal; isFirstPred = false; - } else if (PredVal != SingularValue) - SingularValue = 0; + } else if (PredVal != ExistingValue) + ExistingValue = 0; } } @@ -300,31 +327,38 @@ Value *SSAUpdater::GetValueAtEndOfBlockInternal(BasicBlock *BB) { /// above. TrackingVH &InsertedVal = AvailableVals[BB]; - // If all the predecessor values are the same then we don't need to insert a + // 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 (SingularValue) { - // If a PHI node got inserted, replace it with the singlar value and delete + if (ExistingValue) { + // If a PHI node got inserted, replace it with the existing value and delete // it. if (InsertedVal) { PHINode *OldVal = cast(InsertedVal); // Be careful about dead loops. These RAUW's also update InsertedVal. - if (InsertedVal != SingularValue) - OldVal->replaceAllUsesWith(SingularValue); + if (InsertedVal != ExistingValue) + OldVal->replaceAllUsesWith(ExistingValue); else OldVal->replaceAllUsesWith(UndefValue::get(InsertedVal->getType())); OldVal->eraseFromParent(); } else { - InsertedVal = SingularValue; + InsertedVal = ExistingValue; } - // Either path through the 'if' should have set insertedVal -> SingularVal. - assert((InsertedVal == SingularValue || isa(InsertedVal)) && - "RAUW didn't change InsertedVal to be SingularVal"); + // Either path through the 'if' should have set InsertedVal -> ExistingVal. + assert((InsertedVal == ExistingValue || isa(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 SingularValue; + return ExistingValue; } // Otherwise, we do need a PHI: insert one now if we don't already have one. diff --git a/test/Transforms/GVN/rle-nonlocal.ll b/test/Transforms/GVN/rle-nonlocal.ll index 51b89867a15..5c73dad399e 100644 --- a/test/Transforms/GVN/rle-nonlocal.ll +++ b/test/Transforms/GVN/rle-nonlocal.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -gvn -S | grep {%DEAD = phi i32. } +; RUN: opt < %s -gvn -S | FileCheck %s define i32 @main(i32** %p) { block1: @@ -13,7 +13,12 @@ block3: br label %block4 block4: +; CHECK-NOT: %existingPHI = phi +; CHECK: %DEAD = phi + %existingPHI = phi i32* [ %a, %block2 ], [ %b, %block3 ] %DEAD = load i32** %p %c = load i32* %DEAD - ret i32 %c + %d = load i32* %existingPHI + %e = add i32 %c, %d + ret i32 %e }