[llvm-commits] [llvm] r94690 - in /llvm/trunk: lib/Transforms/Utils/SSAUpdater.cpp test/Transforms/GVN/rle-nonlocal.ll
Bob Wilson
bob.wilson at apple.com
Wed Jan 27 14:01:02 PST 2010
Author: bwilson
Date: Wed Jan 27 16:01:02 2010
New Revision: 94690
URL: http://llvm.org/viewvc/llvm-project?rev=94690&view=rev
Log:
Avoid creating redundant PHIs in SSAUpdater::GetValueInMiddleOfBlock.
This was already being done in SSAUpdater::GetValueAtEndOfBlock so I've
just changed SSAUpdater to check for existing PHIs in both places.
Modified:
llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp
llvm/trunk/test/Transforms/GVN/rle-nonlocal.ll
Modified: llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp?rev=94690&r1=94689&r2=94690&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SSAUpdater.cpp Wed Jan 27 16:01:02 2010
@@ -71,6 +71,50 @@
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<BasicBlock*, Value*> &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<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) {
@@ -149,28 +193,11 @@
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<PHINode>(BB->begin())) {
- DenseMap<BasicBlock*, Value*> ValueMapping(PredValues.begin(),
- PredValues.end());
- PHINode *SomePHI;
- for (BasicBlock::iterator It = BB->begin();
- (SomePHI = dyn_cast<PHINode>(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 @@
// 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<Value> SingularValue;
+ TrackingVH<Value> 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 *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 *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 @@
/// above.
TrackingVH<Value> &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<PHINode>(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<UndefValue>(InsertedVal)) &&
- "RAUW didn't change InsertedVal to be SingularVal");
+ // 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 SingularValue;
+ return ExistingValue;
}
// Otherwise, we do need a PHI: insert one now if we don't already have one.
Modified: llvm/trunk/test/Transforms/GVN/rle-nonlocal.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/rle-nonlocal.ll?rev=94690&r1=94689&r2=94690&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GVN/rle-nonlocal.ll (original)
+++ llvm/trunk/test/Transforms/GVN/rle-nonlocal.ll Wed Jan 27 16:01:02 2010
@@ -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 @@
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
}
More information about the llvm-commits
mailing list