[llvm] r227299 - [LPM] A targeted but somewhat horrible fix to the legacy pass manager's

[Chandler Carruth]

Hans, here is the second patch I'd liked pulled into 3.6 to potentially
address some compile time problems.

On Wed, Jan 28, 2015 at 1:47 AM, Chandler Carruth <chandlerc at gmail.com>
wrote:

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