Refactor the ProfileSummaryInfo to use doInitialization and doFinalization to handle Module update.

[danielcdh]

Summary

This refactors the change in r282616

[mehdi_amini]

LGTM!
Thanks.

[mehdi_amini]

lib/Analysis/ProfileSummaryInfo.cpp

137

Can you also have ProfileSummaryInfo take and store a reference again please?

[davide]

Something to keep in mind, there's no doInitialization/doFinalization equivalent in the new PM so this needs to use a different mechanism when the inliner will be ported.

[danielcdh]

update

[mehdi_amini]

Look great. Thanks.

[danielcdh]

So what's the call? Shall we keep the original implementation to accommodate new pass manager or keep the refactored version?

Thanks,
Dehao

[mehdi_amini]

Current version of this patch is OK for now.

[davide]

In D25041#555966, @danielcdh wrote:

So what's the call? Shall we keep the original implementation to accommodate new pass manager or keep the refactored version?

Thanks,
Dehao

Probably this is fine for now but please double check with Chandler.

include/llvm/Analysis/ProfileSummaryInfo.h

 /// isColdCount that tells whether a given profile count is considered hot/cold
 /// based on the profile summary. This also provides convenience methods to
 /// check whether a function is hot or cold.
 
 // FIXME: Provide convenience methods to determine hotness/coldness of other IR
 // units. This would require making this depend on BFI.
 class ProfileSummaryInfo {
 private:
-  Module *M;
+  Module &M;
   std::unique_ptr<ProfileSummary> Summary;
   void computeSummary();
   void computeThresholds();
   // Count thresholds to answer isHotCount and isColdCount queries.
   Optional<uint64_t> HotCountThreshold, ColdCountThreshold;
 
 public:
-  ProfileSummaryInfo(Module *M) : M(M) {}
+  ProfileSummaryInfo(Module &M) : M(M) {}
   ProfileSummaryInfo(ProfileSummaryInfo &&Arg)
       : M(Arg.M), Summary(std::move(Arg.Summary)) {}
   /// \brief Returns true if \p F is a hot function.
   bool isHotFunction(const Function *F);
   /// \brief Returns true if \p F is a cold function.
   bool isColdFunction(const Function *F);
   /// \brief Returns true if count \p C is considered hot.
   bool isHotCount(uint64_t C);
   /// \brief Returns true if count \p C is considered cold.
   bool isColdCount(uint64_t C);
-  /// \brief Checks if \p NewM is up-to-date, if not, invalidate Summary.
-  void resetModule(Module *NewM);
 };
 
 /// An analysis pass based on legacy pass manager to deliver ProfileSummaryInfo.
 class ProfileSummaryInfoWrapperPass : public ImmutablePass {
   std::unique_ptr<ProfileSummaryInfo> PSI;
 
 public:
   static char ID;
   ProfileSummaryInfoWrapperPass();
 
-  ProfileSummaryInfo *getPSI(Module &M);
+  ProfileSummaryInfo *getPSI() {
+    return &*PSI;
+  }
+
+  bool doInitialization(Module &M) override;
+  bool doFinalization(Module &M) override;
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
   }
 };
 
 /// An analysis pass based on the new PM to deliver ProfileSummaryInfo.
 class ProfileSummaryAnalysis
     : public AnalysisInfoMixin<ProfileSummaryAnalysis> {

lib/Analysis/ModuleSummaryAnalysis.cpp

 }
 
 ModuleSummaryIndexWrapperPass::ModuleSummaryIndexWrapperPass()
     : ModulePass(ID) {
   initializeModuleSummaryIndexWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
 bool ModuleSummaryIndexWrapperPass::runOnModule(Module &M) {
-  auto &PSI = *getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI(M);
+  auto &PSI = *getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
   Index = buildModuleSummaryIndex(
       M,
       [this](const Function &F) {
         return &(this->getAnalysis<BlockFrequencyInfoWrapperPass>(
                          *const_cast<Function *>(&F))
                      .getBFI());
       },
       &PSI);

lib/Analysis/ProfileSummaryInfo.cpp

 
 // The profile summary metadata may be attached either by the frontend or by
 // any backend passes (IR level instrumentation, for example). This method
 // checks if the Summary is null and if so checks if the summary metadata is now
 // available in the module and parses it to get the Summary object.
 void ProfileSummaryInfo::computeSummary() {
   if (Summary)
     return;
-  auto *SummaryMD = M->getProfileSummary();
+  auto *SummaryMD = M.getProfileSummary();
   if (!SummaryMD)
     return;
   Summary.reset(ProfileSummary::getFromMD(SummaryMD));
 }
 
 // Returns true if the function is a hot function. If it returns false, it
 // either means it is not hot or it is unknown whether F is hot or not (for
 // example, no profile data is available).
if (!Summary)
     return;
   auto &DetailedSummary = Summary->getDetailedSummary();
   HotCountThreshold =
       getMinCountForPercentile(DetailedSummary, ProfileSummaryCutoffHot);
   ColdCountThreshold =
       getMinCountForPercentile(DetailedSummary, ProfileSummaryCutoffCold);
 }
 
-void ProfileSummaryInfo::resetModule(Module *NewM) {
-  if (NewM == M)
-    return;
-  M = NewM;
-  Summary.reset(nullptr);
-}
-
 bool ProfileSummaryInfo::isHotCount(uint64_t C) {
   if (!HotCountThreshold)
     computeThresholds();
   return HotCountThreshold && C >= HotCountThreshold.getValue();
 }
 
 bool ProfileSummaryInfo::isColdCount(uint64_t C) {
   if (!ColdCountThreshold)
     computeThresholds();
   return ColdCountThreshold && C <= ColdCountThreshold.getValue();
 }
 
-ProfileSummaryInfo *ProfileSummaryInfoWrapperPass::getPSI(Module &M) {
-  if (!PSI)
-    PSI.reset(new ProfileSummaryInfo(&M));
-  else
-    PSI->resetModule(&M);
-  return PSI.get();
-}
-
 INITIALIZE_PASS(ProfileSummaryInfoWrapperPass, "profile-summary-info",
                 "Profile summary info", false, true)
 
 ProfileSummaryInfoWrapperPass::ProfileSummaryInfoWrapperPass()
     : ImmutablePass(ID) {
   initializeProfileSummaryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
+bool ProfileSummaryInfoWrapperPass::doInitialization(Module &M) {
+  PSI.reset(new ProfileSummaryInfo(M));
+  return false;
+}
+
+bool ProfileSummaryInfoWrapperPass::doFinalization(Module &M) {
+  PSI.reset();
+  return false;
+}
+
 char ProfileSummaryAnalysis::PassID;
 ProfileSummaryInfo ProfileSummaryAnalysis::run(Module &M,
                                                ModuleAnalysisManager &) {
-  return ProfileSummaryInfo(&M);
+  return ProfileSummaryInfo(M);
 }
 
 // FIXME: This only tests isHotFunction and isColdFunction and not the
 // isHotCount and isColdCount calls.
 PreservedAnalyses ProfileSummaryPrinterPass::run(Module &M,
                                                  ModuleAnalysisManager &AM) {
   ProfileSummaryInfo &PSI = AM.getResult<ProfileSummaryAnalysis>(M);
 

lib/LTO/ThinLTOCodeGenerator.cpp

ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index,
 
   saveTempBitcode(TheModule, SaveTempsDir, count, ".4.opt.bc");
 
   if (DisableCodeGen) {
     // Configured to stop before CodeGen, serialize the bitcode and return.
     SmallVector<char, 128> OutputBuffer;
     {
       raw_svector_ostream OS(OutputBuffer);
-      ProfileSummaryInfo PSI(&TheModule);
+      ProfileSummaryInfo PSI(TheModule);
       auto Index = buildModuleSummaryIndex(TheModule, nullptr, nullptr);
       WriteBitcodeToFile(&TheModule, OS, true, &Index);
     }
     return make_unique<ObjectMemoryBuffer>(std::move(OutputBuffer));
   }
 
   return codegenModule(TheModule, TM);
 }

lib/Transforms/IPO/Inliner.cpp

inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG,
   } while (LocalChange);
 
   return Changed;
 }
 
 bool Inliner::inlineCalls(CallGraphSCC &SCC) {
   CallGraph &CG = getAnalysis<CallGraphWrapperPass>().getCallGraph();
   ACT = &getAnalysis<AssumptionCacheTracker>();
-  PSI = getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI(CG.getModule());
+  PSI = getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
   auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
   // We compute dedicated AA results for each function in the SCC as needed. We
   // use a lambda referencing external objects so that they live long enough to
   // be queried, but we re-use them each time.
   Optional<BasicAAResult> BAR;
   Optional<AAResults> AAR;
   auto AARGetter = [&](Function &F) -> AAResults & {
     BAR.emplace(createLegacyPMBasicAAResult(*this, F));