Fix the bug when -compile-twice is specified, the PSI will be invalidated.

[danielcdh]

Summary

When using llc with -compile-twice, module is generated twice, but getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI will still get the old PSI with the original (invalidated) Module. This patch checks if the module has changed when calling getPSI, if yes, update the module and invalidate the Summary.
The bug does not show up in the current llc because PSI is not used in CodeGen yet. But with https://reviews.llvm.org/D24989, the bug will be exposed by test/CodeGen/PowerPC/pr26378.ll

[davidxl]

How about a test case?

include/llvm/Analysis/ProfileSummaryInfo.h

63

resetModule

lib/Analysis/ProfileSummaryInfo.cpp

120

Summary.reset(nullptr);

[danielcdh]

update

[danielcdh]

not sure how to test this: it requires -compile-twice, which is an llc flag, but llc does not use PSI for now. https://reviews.llvm.org/D24989 will be the first patch that makes llc uses PSI.

[davidxl]

lgtm

[mehdi_amini]

This sounds like not very idiomatic. I believe the usual solution is to handle this inside doInitialization. Can you clarify why you didn't do it this way?

[danielcdh]

You are right, doInitialization indeed made the analysis impl more cleaner. A fix is sent out in https://reviews.llvm.org/D25041

Thanks

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;

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));
+    PSI.reset(new ProfileSummaryInfo(&M));
+  else
+    PSI->resetM(&M);
   return PSI.get();
 }
 
 INITIALIZE_PASS(ProfileSummaryInfoWrapperPass, "profile-summary-info",
                 "Profile summary info", false, true)
 
 ProfileSummaryInfoWrapperPass::ProfileSummaryInfoWrapperPass()
     : ImmutablePass(ID) {
   initializeProfileSummaryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
 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);