<div dir="ltr">Hans, here is the second patch I'd liked pulled into 3.6 to potentially address some compile time problems.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 28, 2015 at 1:47 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: chandlerc<br>
Date: Wed Jan 28 03:47:21 2015<br>
New Revision: 227299<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=227299&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=227299&view=rev</a><br>
Log:<br>
[LPM] A targeted but somewhat horrible fix to the legacy pass manager's<br>
querying of the pass registry.<br>
<br>
The pass manager relies on the static registry of PassInfo objects to<br>
perform all manner of its functionality. I don't understand why it does<br>
much of this. My very vague understanding is that this registry is<br>
touched both during static initialization *and* while each pass is being<br>
constructed. As a consequence it is hard to make accessing it not<br>
require a acquiring some lock. This lock ends up in the hot path of<br>
setting up, tearing down, and invaliditing analyses in the legacy pass<br>
manager.<br>
<br>
On most systems you can observe this as a non-trivial % of the time<br>
spent in 'ninja check-llvm'. However, I haven't really seen it be more<br>
than 1% in extreme cases of compiling more real-world software,<br>
including LTO.<br>
<br>
Unfortunately, some of the GPU JITs are seeing this taking essentially<br>
all of their time because they have very small IR running through<br>
a small pass pipeline very many times (at least, this is the vague<br>
understanding I have of it).<br>
<br>
This patch tries to minimize the cost of looking up PassInfo objects by<br>
leveraging the fact that the objects themselves are immutable and they<br>
are allocated separately on the heap and so don't have their address<br>
change. It also requires a change I made the last time I tried to debug<br>
this problem which removed the ability to de-register a pass from the<br>
registry. This patch creates a single access path to these objects<br>
inside the PMTopLevelManager which memoizes the result of querying the<br>
registry. This is somewhat gross as I don't really know if<br>
PMTopLevelManager is the *right* place to put it, and I dislike using<br>
a mutable member to memoize things, but it seems to work.<br>
<br>
For long-lived pass managers this should completely eliminate<br>
the cost of acquiring locks to look into the pass registry once the<br>
memoized cache is warm. For 'ninja check' I measured about 1.5%<br>
reduction in CPU time and in total time on a machine with 32 hardware<br>
threads. For normal compilation, I don't know how much this will help,<br>
sadly. We will still pay the cost while we populate the memoized cache.<br>
I don't think it will hurt though, and for LTO or compiles with many<br>
small functions it should still be a win. However, for tight loops<br>
around a pass manager with many passes and small modules, this will help<br>
tremendously. On the AArch64 backend I saw nearly 50% reductions in time<br>
to complete 2000 cycles of spinning up and tearing down the pipeline.<br>
Measurements from Owen of an actual long-lived pass manager show more<br>
along the lines of 10% improvements.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D7213" target="_blank">http://reviews.llvm.org/D7213</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/IR/LegacyPassManagers.h<br>
llvm/trunk/lib/IR/LegacyPassManager.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/IR/LegacyPassManagers.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LegacyPassManagers.h?rev=227299&r1=227298&r2=227299&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LegacyPassManagers.h?rev=227299&r1=227298&r2=227299&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/IR/LegacyPassManagers.h (original)<br>
+++ llvm/trunk/include/llvm/IR/LegacyPassManagers.h Wed Jan 28 03:47:21 2015<br>
@@ -195,6 +195,9 @@ public:<br>
/// then return NULL.<br>
Pass *findAnalysisPass(AnalysisID AID);<br>
<br>
+ /// Retrieve the PassInfo for an analysis.<br>
+ const PassInfo *findAnalysisPassInfo(AnalysisID AID) const;<br>
+<br>
/// Find analysis usage information for the pass P.<br>
AnalysisUsage *findAnalysisUsage(Pass *P);<br>
<br>
@@ -251,6 +254,12 @@ private:<br>
SmallVector<ImmutablePass *, 16> ImmutablePasses;<br>
<br>
DenseMap<Pass *, AnalysisUsage *> AnUsageMap;<br>
+<br>
+ /// Collection of PassInfo objects found via analysis IDs and in this top<br>
+ /// level manager. This is used to memoize queries to the pass registry.<br>
+ /// FIXME: This is an egregious hack because querying the pass registry is<br>
+ /// either slow or racy.<br>
+ mutable DenseMap<AnalysisID, const PassInfo *> AnalysisPassInfos;<br>
};<br>
<br>
<br>
<br>
Modified: llvm/trunk/lib/IR/LegacyPassManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LegacyPassManager.cpp?rev=227299&r1=227298&r2=227299&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LegacyPassManager.cpp?rev=227299&r1=227298&r2=227299&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/LegacyPassManager.cpp (original)<br>
+++ llvm/trunk/lib/IR/LegacyPassManager.cpp Wed Jan 28 03:47:21 2015<br>
@@ -600,8 +600,7 @@ void PMTopLevelManager::schedulePass(Pas<br>
// If P is an analysis pass and it is available then do not<br>
// generate the analysis again. Stale analysis info should not be<br>
// available at this point.<br>
- const PassInfo *PI =<br>
- PassRegistry::getPassRegistry()->getPassInfo(P->getPassID());<br>
+ const PassInfo *PI = findAnalysisPassInfo(P->getPassID());<br>
if (PI && PI->isAnalysis() && findAnalysisPass(P->getPassID())) {<br>
delete P;<br>
return;<br>
@@ -619,7 +618,7 @@ void PMTopLevelManager::schedulePass(Pas<br>
<br>
Pass *AnalysisPass = findAnalysisPass(*I);<br>
if (!AnalysisPass) {<br>
- const PassInfo *PI = PassRegistry::getPassRegistry()->getPassInfo(*I);<br>
+ const PassInfo *PI = findAnalysisPassInfo(*I);<br>
<br>
if (!PI) {<br>
// Pass P is not in the global PassRegistry<br>
@@ -716,8 +715,7 @@ Pass *PMTopLevelManager::findAnalysisPas<br>
return *I;<br>
<br>
// If Pass not found then check the interfaces implemented by Immutable Pass<br>
- const PassInfo *PassInf =<br>
- PassRegistry::getPassRegistry()->getPassInfo(PI);<br>
+ const PassInfo *PassInf = findAnalysisPassInfo(PI);<br>
assert(PassInf && "Expected all immutable passes to be initialized");<br>
const std::vector<const PassInfo*> &ImmPI =<br>
PassInf->getInterfacesImplemented();<br>
@@ -731,6 +729,17 @@ Pass *PMTopLevelManager::findAnalysisPas<br>
return nullptr;<br>
}<br>
<br>
+const PassInfo *PMTopLevelManager::findAnalysisPassInfo(AnalysisID AID) const {<br>
+ const PassInfo *&PI = AnalysisPassInfos[AID];<br>
+ if (!PI)<br>
+ PI = PassRegistry::getPassRegistry()->getPassInfo(AID);<br>
+ else<br>
+ assert(PI == PassRegistry::getPassRegistry()->getPassInfo(AID) &&<br>
+ "The pass info pointer changed for an analysis ID!");<br>
+<br>
+ return PI;<br>
+}<br>
+<br>
// Print passes managed by this top level manager.<br>
void PMTopLevelManager::dumpPasses() const {<br>
<br>
@@ -759,8 +768,7 @@ void PMTopLevelManager::dumpArguments()<br>
dbgs() << "Pass Arguments: ";<br>
for (SmallVectorImpl<ImmutablePass *>::const_iterator I =<br>
ImmutablePasses.begin(), E = ImmutablePasses.end(); I != E; ++I)<br>
- if (const PassInfo *PI =<br>
- PassRegistry::getPassRegistry()->getPassInfo((*I)->getPassID())) {<br>
+ if (const PassInfo *PI = findAnalysisPassInfo((*I)->getPassID())) {<br>
assert(PI && "Expected all immutable passes to be initialized");<br>
if (!PI->isAnalysisGroup())<br>
dbgs() << " -" << PI->getPassArgument();<br>
@@ -824,7 +832,7 @@ void PMDataManager::recordAvailableAnaly<br>
<br>
// This pass is the current implementation of all of the interfaces it<br>
// implements as well.<br>
- const PassInfo *PInf = PassRegistry::getPassRegistry()->getPassInfo(PI);<br>
+ const PassInfo *PInf = TPM->findAnalysisPassInfo(PI);<br>
if (!PInf) return;<br>
const std::vector<const PassInfo*> &II = PInf->getInterfacesImplemented();<br>
for (unsigned i = 0, e = II.size(); i != e; ++i)<br>
@@ -957,7 +965,7 @@ void PMDataManager::freePass(Pass *P, St<br>
}<br>
<br>
AnalysisID PI = P->getPassID();<br>
- if (const PassInfo *PInf = PassRegistry::getPassRegistry()->getPassInfo(PI)) {<br>
+ if (const PassInfo *PInf = TPM->findAnalysisPassInfo(PI)) {<br>
// Remove the pass itself (if it is not already removed).<br>
AvailableAnalysis.erase(PI);<br>
<br>
@@ -1037,7 +1045,7 @@ void PMDataManager::add(Pass *P, bool Pr<br>
for (SmallVectorImpl<AnalysisID>::iterator<br>
I = ReqAnalysisNotAvailable.begin(),<br>
E = ReqAnalysisNotAvailable.end() ;I != E; ++I) {<br>
- const PassInfo *PI = PassRegistry::getPassRegistry()->getPassInfo(*I);<br>
+ const PassInfo *PI = TPM->findAnalysisPassInfo(*I);<br>
Pass *AnalysisPass = PI->createPass();<br>
this->addLowerLevelRequiredPass(P, AnalysisPass);<br>
}<br>
@@ -1142,7 +1150,7 @@ void PMDataManager::dumpPassArguments()<br>
PMD->dumpPassArguments();<br>
else<br>
if (const PassInfo *PI =<br>
- PassRegistry::getPassRegistry()->getPassInfo((*I)->getPassID()))<br>
+ TPM->findAnalysisPassInfo((*I)->getPassID()))<br>
if (!PI->isAnalysisGroup())<br>
dbgs() << " -" << PI->getPassArgument();<br>
}<br>
@@ -1218,7 +1226,7 @@ void PMDataManager::dumpAnalysisUsage(St<br>
dbgs() << (const void*)P << std::string(getDepth()*2+3, ' ') << Msg << " Analyses:";<br>
for (unsigned i = 0; i != Set.size(); ++i) {<br>
if (i) dbgs() << ',';<br>
- const PassInfo *PInf = PassRegistry::getPassRegistry()->getPassInfo(Set[i]);<br>
+ const PassInfo *PInf = TPM->findAnalysisPassInfo(Set[i]);<br>
if (!PInf) {<br>
// Some preserved passes, such as AliasAnalysis, may not be initialized by<br>
// all drivers.<br>
@@ -1658,8 +1666,8 @@ void MPPassManager::addLowerLevelRequire<br>
<br>
OnTheFlyManagers[P] = FPP;<br>
}<br>
- const PassInfo * RequiredPassPI =<br>
- PassRegistry::getPassRegistry()->getPassInfo(RequiredPass->getPassID());<br>
+ const PassInfo *RequiredPassPI =<br>
+ TPM->findAnalysisPassInfo(RequiredPass->getPassID());<br>
<br>
Pass *FoundPass = nullptr;<br>
if (RequiredPassPI && RequiredPassPI->isAnalysis()) {<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>