[llvm-commits] [llvm] r162023 - in /llvm/trunk: include/llvm/Analysis/Dominators.h lib/Transforms/Scalar/GVN.cpp lib/VMCore/Dominators.cpp test/Transforms/GVN/edge.ll
Rafael Espindola
rafael.espindola at gmail.com
Thu Aug 16 08:09:44 PDT 2012
Author: rafael
Date: Thu Aug 16 10:09:43 2012
New Revision: 162023
URL: http://llvm.org/viewvc/llvm-project?rev=162023&view=rev
Log:
Teach GVN to reason about edges dominating uses. This allows it to handle cases
where some fact lake a=b dominates a use in a phi, but doesn't dominate the
basic block itself.
This feature could also be implemented by splitting critical edges, but at least
with the current algorithm reasoning about the dominance directly is faster.
The time for running "opt -O2" in the testcase in pr10584 is 1.003 times slower
and on gcc as a single file it is 1.0007 times faster.
Added:
llvm/trunk/test/Transforms/GVN/edge.ll
Modified:
llvm/trunk/include/llvm/Analysis/Dominators.h
llvm/trunk/lib/Transforms/Scalar/GVN.cpp
llvm/trunk/lib/VMCore/Dominators.cpp
Modified: llvm/trunk/include/llvm/Analysis/Dominators.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/Dominators.h?rev=162023&r1=162022&r2=162023&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/Dominators.h (original)
+++ llvm/trunk/include/llvm/Analysis/Dominators.h Thu Aug 16 10:09:43 2012
@@ -705,7 +705,20 @@
EXTERN_TEMPLATE_INSTANTIATION(class DominatorTreeBase<BasicBlock>);
-class BasicBlockEdge;
+class BasicBlockEdge {
+ const BasicBlock *Start;
+ const BasicBlock *End;
+public:
+ BasicBlockEdge(const BasicBlock *Start_, const BasicBlock *End_) :
+ Start(Start_), End(End_) { }
+ const BasicBlock *getStart() const {
+ return Start;
+ }
+ const BasicBlock *getEnd() const {
+ return End;
+ }
+ bool isSingleEdge() const;
+};
//===-------------------------------------
/// DominatorTree Class - Concrete subclass of DominatorTreeBase that is used to
Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=162023&r1=162022&r2=162023&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Thu Aug 16 10:09:43 2012
@@ -613,8 +613,8 @@
void verifyRemoved(const Instruction *I) const;
bool splitCriticalEdges();
unsigned replaceAllDominatedUsesWith(Value *From, Value *To,
- const BasicBlock *Root);
- bool propagateEquality(Value *LHS, Value *RHS, const BasicBlock *Root);
+ const BasicBlockEdge &Root);
+ bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root);
};
char GVN::ID = 0;
@@ -2004,22 +2004,13 @@
/// use is dominated by the given basic block. Returns the number of uses that
/// were replaced.
unsigned GVN::replaceAllDominatedUsesWith(Value *From, Value *To,
- const BasicBlock *Root) {
+ const BasicBlockEdge &Root) {
unsigned Count = 0;
for (Value::use_iterator UI = From->use_begin(), UE = From->use_end();
UI != UE; ) {
Use &U = (UI++).getUse();
- // If From occurs as a phi node operand then the use implicitly lives in the
- // corresponding incoming block. Otherwise it is the block containing the
- // user that must be dominated by Root.
- BasicBlock *UsingBlock;
- if (PHINode *PN = dyn_cast<PHINode>(U.getUser()))
- UsingBlock = PN->getIncomingBlock(U);
- else
- UsingBlock = cast<Instruction>(U.getUser())->getParent();
-
- if (DT->dominates(Root, UsingBlock)) {
+ if (DT->dominates(Root, U)) {
U.set(To);
++Count;
}
@@ -2027,13 +2018,34 @@
return Count;
}
+/// isOnlyReachableViaThisEdge - There is an edge from 'Src' to 'Dst'. Return
+/// true if every path from the entry block to 'Dst' passes via this edge. In
+/// particular 'Dst' must not be reachable via another edge from 'Src'.
+static bool isOnlyReachableViaThisEdge(const BasicBlockEdge &E,
+ DominatorTree *DT) {
+ // While in theory it is interesting to consider the case in which Dst has
+ // more than one predecessor, because Dst might be part of a loop which is
+ // only reachable from Src, in practice it is pointless since at the time
+ // GVN runs all such loops have preheaders, which means that Dst will have
+ // been changed to have only one predecessor, namely Src.
+ const BasicBlock *Pred = E.getEnd()->getSinglePredecessor();
+ const BasicBlock *Src = E.getStart();
+ assert((!Pred || Pred == Src) && "No edge between these basic blocks!");
+ (void)Src;
+ return Pred != 0;
+}
+
/// propagateEquality - The given values are known to be equal in every block
/// dominated by 'Root'. Exploit this, for example by replacing 'LHS' with
/// 'RHS' everywhere in the scope. Returns whether a change was made.
-bool GVN::propagateEquality(Value *LHS, Value *RHS, const BasicBlock *Root) {
+bool GVN::propagateEquality(Value *LHS, Value *RHS,
+ const BasicBlockEdge &Root) {
SmallVector<std::pair<Value*, Value*>, 4> Worklist;
Worklist.push_back(std::make_pair(LHS, RHS));
bool Changed = false;
+ // For speed, compute a conservative fast approximation to
+ // DT->dominates(Root, Root.getEnd());
+ bool RootDominatesEnd = isOnlyReachableViaThisEdge(Root, DT);
while (!Worklist.empty()) {
std::pair<Value*, Value*> Item = Worklist.pop_back_val();
@@ -2065,9 +2077,6 @@
LVN = RVN;
}
}
- assert((!isa<Instruction>(RHS) ||
- DT->properlyDominates(cast<Instruction>(RHS)->getParent(), Root)) &&
- "Instruction doesn't dominate scope!");
// If value numbering later sees that an instruction in the scope is equal
// to 'LHS' then ensure it will be turned into 'RHS'. In order to preserve
@@ -2076,8 +2085,10 @@
// if RHS is an instruction (if an instruction in the scope is morphed into
// LHS then it will be turned into RHS by the next GVN iteration anyway, so
// using the leader table is about compiling faster, not optimizing better).
- if (!isa<Instruction>(RHS))
- addToLeaderTable(LVN, RHS, Root);
+ // The leader table only tracks basic blocks, not edges. Only add to if we
+ // have the simple case where the edge dominates the end.
+ if (RootDominatesEnd && !isa<Instruction>(RHS))
+ addToLeaderTable(LVN, RHS, Root.getEnd());
// Replace all occurrences of 'LHS' with 'RHS' everywhere in the scope. As
// LHS always has at least one use that is not dominated by Root, this will
@@ -2136,7 +2147,7 @@
// If the number we were assigned was brand new then there is no point in
// looking for an instruction realizing it: there cannot be one!
if (Num < NextNum) {
- Value *NotCmp = findLeader(Root, Num);
+ Value *NotCmp = findLeader(Root.getEnd(), Num);
if (NotCmp && isa<Instruction>(NotCmp)) {
unsigned NumReplacements =
replaceAllDominatedUsesWith(NotCmp, NotVal, Root);
@@ -2146,7 +2157,10 @@
}
// Ensure that any instruction in scope that gets the "A < B" value number
// is replaced with false.
- addToLeaderTable(Num, NotVal, Root);
+ // The leader table only tracks basic blocks, not edges. Only add to if we
+ // have the simple case where the edge dominates the end.
+ if (RootDominatesEnd)
+ addToLeaderTable(Num, NotVal, Root.getEnd());
continue;
}
@@ -2155,22 +2169,6 @@
return Changed;
}
-/// isOnlyReachableViaThisEdge - There is an edge from 'Src' to 'Dst'. Return
-/// true if every path from the entry block to 'Dst' passes via this edge. In
-/// particular 'Dst' must not be reachable via another edge from 'Src'.
-static bool isOnlyReachableViaThisEdge(BasicBlock *Src, BasicBlock *Dst,
- DominatorTree *DT) {
- // While in theory it is interesting to consider the case in which Dst has
- // more than one predecessor, because Dst might be part of a loop which is
- // only reachable from Src, in practice it is pointless since at the time
- // GVN runs all such loops have preheaders, which means that Dst will have
- // been changed to have only one predecessor, namely Src.
- BasicBlock *Pred = Dst->getSinglePredecessor();
- assert((!Pred || Pred == Src) && "No edge between these basic blocks!");
- (void)Src;
- return Pred != 0;
-}
-
/// processInstruction - When calculating availability, handle an instruction
/// by inserting it into the appropriate sets
bool GVN::processInstruction(Instruction *I) {
@@ -2210,18 +2208,20 @@
BasicBlock *TrueSucc = BI->getSuccessor(0);
BasicBlock *FalseSucc = BI->getSuccessor(1);
+ // Avoid multiple edges early.
+ if (TrueSucc == FalseSucc)
+ return false;
+
BasicBlock *Parent = BI->getParent();
bool Changed = false;
- if (isOnlyReachableViaThisEdge(Parent, TrueSucc, DT))
- Changed |= propagateEquality(BranchCond,
- ConstantInt::getTrue(TrueSucc->getContext()),
- TrueSucc);
-
- if (isOnlyReachableViaThisEdge(Parent, FalseSucc, DT))
- Changed |= propagateEquality(BranchCond,
- ConstantInt::getFalse(FalseSucc->getContext()),
- FalseSucc);
+ Value *TrueVal = ConstantInt::getTrue(TrueSucc->getContext());
+ BasicBlockEdge TrueE(Parent, TrueSucc);
+ Changed |= propagateEquality(BranchCond, TrueVal, TrueE);
+
+ Value *FalseVal = ConstantInt::getFalse(FalseSucc->getContext());
+ BasicBlockEdge FalseE(Parent, FalseSucc);
+ Changed |= propagateEquality(BranchCond, FalseVal, FalseE);
return Changed;
}
@@ -2234,8 +2234,9 @@
for (SwitchInst::CaseIt i = SI->case_begin(), e = SI->case_end();
i != e; ++i) {
BasicBlock *Dst = i.getCaseSuccessor();
- if (isOnlyReachableViaThisEdge(Parent, Dst, DT))
- Changed |= propagateEquality(SwitchCond, i.getCaseValue(), Dst);
+ BasicBlockEdge E(Parent, Dst);
+ if (E.isSingleEdge())
+ Changed |= propagateEquality(SwitchCond, i.getCaseValue(), E);
}
return Changed;
}
Modified: llvm/trunk/lib/VMCore/Dominators.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Dominators.cpp?rev=162023&r1=162022&r2=162023&view=diff
==============================================================================
--- llvm/trunk/lib/VMCore/Dominators.cpp (original)
+++ llvm/trunk/lib/VMCore/Dominators.cpp Thu Aug 16 10:09:43 2012
@@ -39,20 +39,17 @@
VerifyDomInfoX("verify-dom-info", cl::location(VerifyDomInfo),
cl::desc("Verify dominator info (time consuming)"));
-namespace llvm {
- class BasicBlockEdge {
- const BasicBlock *Start;
- const BasicBlock *End;
- public:
- BasicBlockEdge(const BasicBlock *Start_, const BasicBlock *End_) :
- Start(Start_), End(End_) { }
- const BasicBlock *getStart() const {
- return Start;
- }
- const BasicBlock *getEnd() const {
- return End;
- }
- };
+bool BasicBlockEdge::isSingleEdge() const {
+ const TerminatorInst *TI = Start->getTerminator();
+ unsigned NumEdgesToEnd = 0;
+ for (unsigned int i = 0, n = TI->getNumSuccessors(); i < n; ++i) {
+ if (TI->getSuccessor(i) == End)
+ ++NumEdgesToEnd;
+ if (NumEdgesToEnd >= 2)
+ return false;
+ }
+ assert(NumEdgesToEnd == 1);
+ return true;
}
//===----------------------------------------------------------------------===//
Added: llvm/trunk/test/Transforms/GVN/edge.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/edge.ll?rev=162023&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/edge.ll (added)
+++ llvm/trunk/test/Transforms/GVN/edge.ll Thu Aug 16 10:09:43 2012
@@ -0,0 +1,60 @@
+; RUN: opt %s -gvn -S -o - | FileCheck %s
+
+define i32 @f1(i32 %x) {
+ ; CHECK: define i32 @f1(
+bb0:
+ %cmp = icmp eq i32 %x, 0
+ br i1 %cmp, label %bb2, label %bb1
+bb1:
+ br label %bb2
+bb2:
+ %cond = phi i32 [ %x, %bb0 ], [ 0, %bb1 ]
+ %foo = add i32 %cond, %x
+ ret i32 %foo
+ ; CHECK: bb2:
+ ; CHECK: ret i32 %x
+}
+
+define i32 @f2(i32 %x) {
+ ; CHECK: define i32 @f2(
+bb0:
+ %cmp = icmp ne i32 %x, 0
+ br i1 %cmp, label %bb1, label %bb2
+bb1:
+ br label %bb2
+bb2:
+ %cond = phi i32 [ %x, %bb0 ], [ 0, %bb1 ]
+ %foo = add i32 %cond, %x
+ ret i32 %foo
+ ; CHECK: bb2:
+ ; CHECK: ret i32 %x
+}
+
+define i32 @f3(i32 %x) {
+ ; CHECK: define i32 @f3(
+bb0:
+ switch i32 %x, label %bb1 [ i32 0, label %bb2]
+bb1:
+ br label %bb2
+bb2:
+ %cond = phi i32 [ %x, %bb0 ], [ 0, %bb1 ]
+ %foo = add i32 %cond, %x
+ ret i32 %foo
+ ; CHECK: bb2:
+ ; CHECK: ret i32 %x
+}
+
+declare void @g(i1)
+define void @f4(i8 * %x) {
+; CHECK: define void @f4(
+bb0:
+ %y = icmp eq i8* null, %x
+ br i1 %y, label %bb2, label %bb1
+bb1:
+ br label %bb2
+bb2:
+ %zed = icmp eq i8* null, %x
+ call void @g(i1 %zed)
+; CHECK: call void @g(i1 %y)
+ ret void
+}
More information about the llvm-commits
mailing list