[llvm] r279216 - [PM] Rework the new PM support for building the ModuleSummaryIndex to

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 19 00:49:20 PDT 2016


Author: chandlerc
Date: Fri Aug 19 02:49:19 2016
New Revision: 279216

URL: http://llvm.org/viewvc/llvm-project?rev=279216&view=rev
Log:
[PM] Rework the new PM support for building the ModuleSummaryIndex to
directly produce the index as the value type result.

This requires making the index movable which is straightforward. It
greatly simplifies things by allowing us to completely avoid the builder
API and the layers of abstraction inherent there. Instead both pass
managers can directly construct these when run by value. They still
won't be constructed truly eagerly thanks to the optional in the legacy
PM. The code that directly builds the index can also just share a direct
function.

A notable change here is that the result type of the analysis for the
new PM is no longer a reference type. This was really problematic when
making changes to how we handle result types to make our interface
requirements *much* more strict and precise. But I think this is an
overall improvement.

Differential Revision: https://reviews.llvm.org/D23701

Modified:
    llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h
    llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
    llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
    llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

Modified: llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h?rev=279216&r1=279215&r2=279216&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h (original)
+++ llvm/trunk/include/llvm/Analysis/ModuleSummaryAnalysis.h Fri Aug 19 02:49:19 2016
@@ -20,41 +20,18 @@
 #include "llvm/Pass.h"
 
 namespace llvm {
-
 class BlockFrequencyInfo;
 
-/// Class to build a module summary index for the given Module, possibly from
-/// a Pass.
-class ModuleSummaryIndexBuilder {
-  /// The index being built
-  std::unique_ptr<ModuleSummaryIndex> Index;
-  /// The module for which we are building an index
-  const Module *M;
-
-public:
-  /// Default constructor
-  ModuleSummaryIndexBuilder() = default;
-
-  /// Constructor that builds an index for the given Module. An optional
-  /// callback can be supplied to obtain the frequency info for a function.
-  ModuleSummaryIndexBuilder(
-      const Module *M,
-      std::function<BlockFrequencyInfo *(const Function &F)> Ftor = nullptr);
-
-  /// Get a reference to the index owned by builder
-  ModuleSummaryIndex &getIndex() const { return *Index; }
-
-  /// Take ownership of the built index
-  std::unique_ptr<ModuleSummaryIndex> takeIndex() { return std::move(Index); }
-
-private:
-  /// Compute summary for given function with optional frequency information
-  void computeFunctionSummary(const Function &F,
-                              BlockFrequencyInfo *BFI = nullptr);
-
-  /// Compute summary for given variable with optional frequency information
-  void computeVariableSummary(const GlobalVariable &V);
-};
+/// Direct function to compute a \c ModuleSummaryIndex from a given module.
+///
+/// If operating within a pass manager which has defined ways to compute the \c
+/// BlockFrequencyInfo for a given function, that can be provided via
+/// a std::function callback. Otherwise, this routine will manually construct
+/// that information.
+ModuleSummaryIndex buildModuleSummaryIndex(
+    const Module &M,
+    std::function<BlockFrequencyInfo *(const Function &F)> GetBFICallback =
+        nullptr);
 
 /// Analysis pass to provide the ModuleSummaryIndex object.
 class ModuleSummaryIndexAnalysis
@@ -62,26 +39,15 @@ class ModuleSummaryIndexAnalysis
   friend AnalysisInfoMixin<ModuleSummaryIndexAnalysis>;
   static char PassID;
 
-  std::unique_ptr<ModuleSummaryIndexBuilder> IndexBuilder;
-
 public:
-  typedef const ModuleSummaryIndex &Result;
-
-  // FIXME: Remove these once MSVC can synthesize them.
-  ModuleSummaryIndexAnalysis() {}
-  ModuleSummaryIndexAnalysis(ModuleSummaryIndexAnalysis &&Arg)
-      : IndexBuilder(std::move(Arg.IndexBuilder)) {}
-  ModuleSummaryIndexAnalysis &operator=(ModuleSummaryIndexAnalysis &&RHS) {
-    IndexBuilder = std::move(RHS.IndexBuilder);
-    return *this;
-  }
+  typedef ModuleSummaryIndex Result;
 
-  const ModuleSummaryIndex &run(Module &M, ModuleAnalysisManager &AM);
+  Result run(Module &M, ModuleAnalysisManager &AM);
 };
 
 /// Legacy wrapper pass to provide the ModuleSummaryIndex object.
 class ModuleSummaryIndexWrapperPass : public ModulePass {
-  std::unique_ptr<ModuleSummaryIndexBuilder> IndexBuilder;
+  Optional<ModuleSummaryIndex> Index;
 
 public:
   static char ID;
@@ -89,10 +55,8 @@ public:
   ModuleSummaryIndexWrapperPass();
 
   /// Get the index built by pass
-  ModuleSummaryIndex &getIndex() { return IndexBuilder->getIndex(); }
-  const ModuleSummaryIndex &getIndex() const {
-    return IndexBuilder->getIndex();
-  }
+  ModuleSummaryIndex &getIndex() { return *Index; }
+  const ModuleSummaryIndex &getIndex() const { return *Index; }
 
   bool runOnModule(Module &M) override;
   bool doFinalization(Module &M) override;

Modified: llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h?rev=279216&r1=279215&r2=279216&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h (original)
+++ llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h Fri Aug 19 02:49:19 2016
@@ -363,11 +363,14 @@ private:
 
 public:
   ModuleSummaryIndex() = default;
-
-  // Disable the copy constructor and assignment operators, so
-  // no unexpected copying/moving occurs.
-  ModuleSummaryIndex(const ModuleSummaryIndex &) = delete;
-  void operator=(const ModuleSummaryIndex &) = delete;
+  ModuleSummaryIndex(ModuleSummaryIndex &&Arg)
+      : GlobalValueMap(std::move(Arg.GlobalValueMap)),
+        ModulePathStringTable(std::move(Arg.ModulePathStringTable)) {}
+  ModuleSummaryIndex &operator=(ModuleSummaryIndex &&RHS) {
+    GlobalValueMap = std::move(RHS.GlobalValueMap);
+    ModulePathStringTable = std::move(RHS.ModulePathStringTable);
+    return *this;
+  }
 
   gvsummary_iterator begin() { return GlobalValueMap.begin(); }
   const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }

Modified: llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp?rev=279216&r1=279215&r2=279216&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/ModuleSummaryAnalysis.cpp Fri Aug 19 02:49:19 2016
@@ -63,8 +63,8 @@ static void findRefEdges(const User *Cur
   }
 }
 
-void ModuleSummaryIndexBuilder::computeFunctionSummary(
-    const Function &F, BlockFrequencyInfo *BFI) {
+static void computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M,
+                                   const Function &F, BlockFrequencyInfo *BFI) {
   // Summary not currently supported for anonymous functions, they must
   // be renamed.
   if (!F.hasName())
@@ -91,7 +91,7 @@ void ModuleSummaryIndexBuilder::computeF
           if (CalledFunction->hasName() && !CalledFunction->isIntrinsic()) {
             auto ScaledCount = BFI ? BFI->getBlockProfileCount(&BB) : None;
             auto *CalleeId =
-                M->getValueSymbolTable().lookup(CalledFunction->getName());
+                M.getValueSymbolTable().lookup(CalledFunction->getName());
             CallGraphEdges[CalleeId] +=
                 (ScaledCount ? ScaledCount.getValue() : 0);
           }
@@ -120,11 +120,11 @@ void ModuleSummaryIndexBuilder::computeF
   FuncSummary->addCallGraphEdges(CallGraphEdges);
   FuncSummary->addCallGraphEdges(IndirectCallEdges);
   FuncSummary->addRefEdges(RefEdges);
-  Index->addGlobalValueSummary(F.getName(), std::move(FuncSummary));
+  Index.addGlobalValueSummary(F.getName(), std::move(FuncSummary));
 }
 
-void ModuleSummaryIndexBuilder::computeVariableSummary(
-    const GlobalVariable &V) {
+static void computeVariableSummary(ModuleSummaryIndex &Index,
+                                   const GlobalVariable &V) {
   DenseSet<const Value *> RefEdges;
   SmallPtrSet<const User *, 8> Visited;
   findRefEdges(&V, RefEdges, Visited);
@@ -132,29 +132,29 @@ void ModuleSummaryIndexBuilder::computeV
   std::unique_ptr<GlobalVarSummary> GVarSummary =
       llvm::make_unique<GlobalVarSummary>(Flags);
   GVarSummary->addRefEdges(RefEdges);
-  Index->addGlobalValueSummary(V.getName(), std::move(GVarSummary));
+  Index.addGlobalValueSummary(V.getName(), std::move(GVarSummary));
 }
 
-ModuleSummaryIndexBuilder::ModuleSummaryIndexBuilder(
-    const Module *M,
-    std::function<BlockFrequencyInfo *(const Function &F)> Ftor)
-    : Index(llvm::make_unique<ModuleSummaryIndex>()), M(M) {
+ModuleSummaryIndex llvm::buildModuleSummaryIndex(
+    const Module &M,
+    std::function<BlockFrequencyInfo *(const Function &F)> GetBFICallback) {
+  ModuleSummaryIndex Index;
   // Check if the module can be promoted, otherwise just disable importing from
   // it by not emitting any summary.
   // FIXME: we could still import *into* it most of the time.
-  if (!moduleCanBeRenamedForThinLTO(*M))
-    return;
+  if (!moduleCanBeRenamedForThinLTO(M))
+    return Index;
 
   // Compute summaries for all functions defined in module, and save in the
   // index.
-  for (auto &F : *M) {
+  for (auto &F : M) {
     if (F.isDeclaration())
       continue;
 
     BlockFrequencyInfo *BFI = nullptr;
     std::unique_ptr<BlockFrequencyInfo> BFIPtr;
-    if (Ftor)
-      BFI = Ftor(F);
+    if (GetBFICallback)
+      BFI = GetBFICallback(F);
     else if (F.getEntryCount().hasValue()) {
       LoopInfo LI{DominatorTree(const_cast<Function &>(F))};
       BranchProbabilityInfo BPI{F, LI};
@@ -162,29 +162,27 @@ ModuleSummaryIndexBuilder::ModuleSummary
       BFI = BFIPtr.get();
     }
 
-    computeFunctionSummary(F, BFI);
+    computeFunctionSummary(Index, M, F, BFI);
   }
 
   // Compute summaries for all variables defined in module, and save in the
   // index.
-  for (const GlobalVariable &G : M->globals()) {
+  for (const GlobalVariable &G : M.globals()) {
     if (G.isDeclaration())
       continue;
-    computeVariableSummary(G);
+    computeVariableSummary(Index, G);
   }
+  return Index;
 }
 
 char ModuleSummaryIndexAnalysis::PassID;
 
-const ModuleSummaryIndex &
+ModuleSummaryIndex
 ModuleSummaryIndexAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
   auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
-  IndexBuilder = llvm::make_unique<ModuleSummaryIndexBuilder>(
-      &M, [&FAM](const Function &F) {
-        return &(
-            FAM.getResult<BlockFrequencyAnalysis>(*const_cast<Function *>(&F)));
-      });
-  return IndexBuilder->getIndex();
+  return buildModuleSummaryIndex(M, [&FAM](const Function &F) {
+    return &FAM.getResult<BlockFrequencyAnalysis>(*const_cast<Function *>(&F));
+  });
 }
 
 char ModuleSummaryIndexWrapperPass::ID = 0;
@@ -204,17 +202,16 @@ ModuleSummaryIndexWrapperPass::ModuleSum
 }
 
 bool ModuleSummaryIndexWrapperPass::runOnModule(Module &M) {
-  IndexBuilder = llvm::make_unique<ModuleSummaryIndexBuilder>(
-      &M, [this](const Function &F) {
-        return &(this->getAnalysis<BlockFrequencyInfoWrapperPass>(
-                         *const_cast<Function *>(&F))
-                     .getBFI());
-      });
+  Index = buildModuleSummaryIndex(M, [this](const Function &F) {
+    return &(this->getAnalysis<BlockFrequencyInfoWrapperPass>(
+                     *const_cast<Function *>(&F))
+                 .getBFI());
+  });
   return false;
 }
 
 bool ModuleSummaryIndexWrapperPass::doFinalization(Module &M) {
-  IndexBuilder.reset();
+  Index.reset();
   return false;
 }
 

Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=279216&r1=279215&r2=279216&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
+++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Fri Aug 19 02:49:19 2016
@@ -377,8 +377,8 @@ ProcessThinLTOModule(Module &TheModule,
     SmallVector<char, 128> OutputBuffer;
     {
       raw_svector_ostream OS(OutputBuffer);
-      ModuleSummaryIndexBuilder IndexBuilder(&TheModule);
-      WriteBitcodeToFile(&TheModule, OS, true, &IndexBuilder.getIndex());
+      auto Index = buildModuleSummaryIndex(TheModule);
+      WriteBitcodeToFile(&TheModule, OS, true, &Index);
     }
     return make_unique<ObjectMemoryBuffer>(std::move(OutputBuffer));
   }




More information about the llvm-commits mailing list