[llvm-commits] [llvm] r70820 - in /llvm/trunk: lib/Transforms/Scalar/JumpThreading.cpp test/Transforms/JumpThreading/no-irreducible-loops.ll
Chris Lattner
sabre at nondot.org
Sun May 3 19:28:08 PDT 2009
Author: lattner
Date: Sun May 3 21:28:08 2009
New Revision: 70820
URL: http://llvm.org/viewvc/llvm-project?rev=70820&view=rev
Log:
* Sink 4 duplicates of edge threading validity checks and DOUT prints into
ThreadEdge directly. This shares the code, but is just a refactoring.
* Make JumpThreading compute the set of loop headers and avoid threading
across them. This prevents jump threading from forming irreducible
loops (goodness) but also prevents it from threading in other cases that
are beneficial (see the comment above FindFunctionBackedges).
Added:
llvm/trunk/test/Transforms/JumpThreading/no-irreducible-loops.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=70820&r1=70819&r2=70820&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Sun May 3 21:28:08 2009
@@ -15,17 +15,19 @@
#include "llvm/Transforms/Scalar.h"
#include "llvm/IntrinsicInst.h"
#include "llvm/Pass.h"
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/Statistic.h"
-#include "llvm/ADT/STLExtras.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");
@@ -55,6 +57,11 @@
///
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) {}
@@ -64,8 +71,11 @@
}
bool runOnFunction(Function &F);
+ void FindLoopHeaders(Function &F);
+
bool ProcessBlock(BasicBlock *BB);
- void ThreadEdge(BasicBlock *BB, BasicBlock *PredBB, BasicBlock *SuccBB);
+ bool ThreadEdge(BasicBlock *BB, BasicBlock *PredBB, BasicBlock *SuccBB,
+ unsigned JumpThreadCost);
BasicBlock *FactorCommonPHIPreds(PHINode *PN, Constant *CstVal);
bool ProcessBranchOnDuplicateCond(BasicBlock *PredBB, BasicBlock *DestBB);
bool ProcessSwitchOnDuplicateCond(BasicBlock *PredBB, BasicBlock *DestBB);
@@ -91,6 +101,8 @@
DOUT << "Jump threading on function '" << F.getNameStart() << "'\n";
TD = &getAnalysis<TargetData>();
+ FindLoopHeaders(F);
+
bool AnotherIteration = true, EverChanged = false;
while (AnotherIteration) {
AnotherIteration = false;
@@ -109,15 +121,42 @@
DOUT << " JT: Deleting dead block '" << BB->getNameStart()
<< "' with terminator: " << *BB->getTerminator();
DeleteDeadBlock(BB);
+ LoopHeaders.erase(BB);
Changed = true;
}
}
AnotherIteration = Changed;
EverChanged |= Changed;
}
+
+ LoopHeaders.clear();
return EverChanged;
}
+/// FindLoopHeaders - We do not wan 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.
@@ -191,6 +230,10 @@
if (BasicBlock *SinglePred = BB->getSinglePredecessor())
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();
@@ -389,22 +432,8 @@
// Next, figure out which successor we are threading to.
BasicBlock *SuccBB = DestBI->getSuccessor(!BranchDir);
- // 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);
}
/// ProcessSwitchOnDuplicateCond - We found a block and a predecessor of that
@@ -675,22 +704,8 @@
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
@@ -751,22 +766,8 @@
// '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);
}
/// ProcessBranchOnCompare - We found a branch on a comparison between a phi
@@ -829,32 +830,40 @@
// 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.
@@ -938,4 +947,8 @@
RecursivelyDeleteTriviallyDeadInstructions(Inst);
}
+
+ // Threaded an edge!
+ ++NumThreads;
+ return true;
}
Added: llvm/trunk/test/Transforms/JumpThreading/no-irreducible-loops.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/no-irreducible-loops.ll?rev=70820&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/no-irreducible-loops.ll (added)
+++ llvm/trunk/test/Transforms/JumpThreading/no-irreducible-loops.ll Sun May 3 21:28:08 2009
@@ -0,0 +1,38 @@
+; RUN: llvm-as < %s | opt -jump-threading -loop-rotate -instcombine -indvars -loop-unroll -simplifycfg | llvm-dis > %t
+; RUN: grep {volatile store} %t | count 3
+; RUN: not grep {br label} %t
+
+; Jump threading should not prevent this loop from being unrolled.
+
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
+target triple = "i386-apple-darwin9.6"
+ at v1 = external global i32 ; <i32*> [#uses=2]
+
+define i32 @unroll() nounwind {
+entry:
+ br label %bb4
+
+bb: ; preds = %bb4
+ %0 = icmp eq i32 %i.0, 0 ; <i1> [#uses=1]
+ br i1 %0, label %bb1, label %bb2
+
+bb1: ; preds = %bb
+ volatile store i32 1000, i32* @v1, align 4
+ br label %bb3
+
+bb2: ; preds = %bb
+ volatile store i32 1001, i32* @v1, align 4
+ br label %bb3
+
+bb3: ; preds = %bb2, %bb1
+ %1 = add i32 %i.0, 1 ; <i32> [#uses=1]
+ br label %bb4
+
+bb4: ; preds = %bb3, %entry
+ %i.0 = phi i32 [ 0, %entry ], [ %1, %bb3 ] ; <i32> [#uses=3]
+ %2 = icmp sgt i32 %i.0, 2 ; <i1> [#uses=1]
+ br i1 %2, label %bb5, label %bb
+
+bb5: ; preds = %bb4
+ ret i32 0
+}
More information about the llvm-commits
mailing list