Refactor the code to pass down ACT to SampleProfileLoader correctly.

[danielcdh]

Summary

This change passes down ACT to SampleProfileLoader for the new PM. Also remove the default value for SampleProfileLoader class as it is not used.

[eraman]

lib/Transforms/IPO/SampleProfile.cpp

150

Name = SampleProfileFile ? Looking below, this is a separate refactoring. Perhaps separate the two out? Otherwise, at least make the desccription clear that there are two different refactoring.

753

Any reason why you want a named temporary?

1485

Since ACT is an immutable pass that doesn't require the module, you can move the initialization of ACT to the constructor of the legacy pass.

1512–1518

you don't need the std::function here. Just use auto.

[danielcdh]

update

[eraman]

LGTM

lib/Transforms/IPO/SampleProfile.cpp

1485

(Dehao explained offline why this won't work)

[danielcdh]

lib/Transforms/IPO/SampleProfile.cpp

753

revert, this was in preparation for the follow-up patch.

1485

Discussed offline, this need to be here (after calling getAnalysisUsage)

lib/Transforms/IPO/SampleProfile.cpp

 
 /// \brief Sample profile pass.
 ///
 /// This pass reads profile data from the file specified by
 /// -sample-profile-file and annotates every affected function with the
 /// profile information found in that file.
 class SampleProfileLoader {
 public:
-  SampleProfileLoader(StringRef Name = SampleProfileFile)
-      : DT(nullptr), PDT(nullptr), LI(nullptr), ACT(nullptr), Reader(),
-        Samples(nullptr), Filename(Name), ProfileIsValid(false),
+  SampleProfileLoader(
+      StringRef Name,
+      std::function<AssumptionCache &(Function &)> GetAssumptionCache)
+      : DT(nullptr), PDT(nullptr), LI(nullptr), GetAC(GetAssumptionCache),
+        Reader(), Samples(nullptr), Filename(Name), ProfileIsValid(false),
         TotalCollectedSamples(0), ORE(nullptr) {}
 
   bool doInitialization(Module &M);
   bool runOnModule(Module &M, ModuleAnalysisManager *AM);
-  void setACT(AssumptionCacheTracker *A) { ACT = A; }
 
   void dump() { Reader->dump(); }
 
 protected:
   bool runOnFunction(Function &F, ModuleAnalysisManager *AM);
   unsigned getFunctionLoc(Function &F);
   bool emitAnnotations(Function &F);
   ErrorOr<uint64_t> getInstWeight(const Instruction &I);
protected:
   /// is one-to-one mapping.
   StringMap<Function *> SymbolMap;
 
   /// \brief Dominance, post-dominance and loop information.
   std::unique_ptr<DominatorTree> DT;
   std::unique_ptr<PostDomTreeBase<BasicBlock>> PDT;
   std::unique_ptr<LoopInfo> LI;
 
-  AssumptionCacheTracker *ACT;
+  std::function<AssumptionCache &(Function &)> GetAC;
 
   /// \brief Predecessors for each basic block in the CFG.
   BlockEdgeMap Predecessors;
 
   /// \brief Successors for each basic block in the CFG.
   BlockEdgeMap Successors;
 
   SampleCoverageTracker CoverageTracker;

 };
 
 class SampleProfileLoaderLegacyPass : public ModulePass {
 public:
   // Class identification, replacement for typeinfo
   static char ID;
 
   SampleProfileLoaderLegacyPass(StringRef Name = SampleProfileFile)
-      : ModulePass(ID), SampleLoader(Name) {
+      : ModulePass(ID), SampleLoader(Name,
+                                     [&](Function &F) -> AssumptionCache & {
+                                       return ACT->getAssumptionCache(F);
+                                     }),
+        ACT(nullptr) {
     initializeSampleProfileLoaderLegacyPassPass(
         *PassRegistry::getPassRegistry());
   }
 
   void dump() { SampleLoader.dump(); }
 
   bool doInitialization(Module &M) override {
     return SampleLoader.doInitialization(M);
   }
   StringRef getPassName() const override { return "Sample profile pass"; }
   bool runOnModule(Module &M) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<AssumptionCacheTracker>();
   }
 
 private:
   SampleProfileLoader SampleLoader;
+  AssumptionCacheTracker *ACT;
 };
 
 /// Return true if the given callsite is hot wrt to its caller.
 ///
 /// Functions that were inlined in the original binary will be represented
 /// in the inline stack in the sample profile. If the profile shows that
 /// the original inline decision was "good" (i.e., the callsite is executed
 /// frequently), then we will recreate the inline decision and apply the

 /// \param ImportGUIDs a set to be updated to include all GUIDs that come
 ///     from a different module but inlined in the profiled binary.
 ///
 /// \returns True if there is any inline happened.
 bool SampleProfileLoader::inlineHotFunctions(
     Function &F, DenseSet<GlobalValue::GUID> &ImportGUIDs) {
   DenseSet<Instruction *> PromotedInsns;
   bool Changed = false;
-  std::function<AssumptionCache &(Function &)> GetAssumptionCache = [&](
-      Function &F) -> AssumptionCache & { return ACT->getAssumptionCache(F); };
   while (true) {
     bool LocalChanged = false;
     SmallVector<Instruction *, 10> CIS;
     for (auto &BB : F) {
       bool Hot = false;
       SmallVector<Instruction *, 10> Candidates;
       for (auto &I : BB.getInstList()) {
         const FunctionSamples *FS = nullptr;
         if ((isa<CallInst>(I) || isa<InvokeInst>(I)) &&
             !isa<IntrinsicInst>(I) && (FS = findCalleeFunctionSamples(I))) {
           Candidates.push_back(&I);
           if (callsiteIsHot(Samples, FS))
             Hot = true;
         }
       }
       if (Hot) {
         CIS.insert(CIS.begin(), Candidates.begin(), Candidates.end());
       }
     }
     for (auto I : CIS) {
-      InlineFunctionInfo IFI(nullptr, ACT ? &GetAssumptionCache : nullptr);
+      InlineFunctionInfo IFI(nullptr, &GetAC);
       Function *CalledFunction = CallSite(I).getCalledFunction();
       // Do not inline recursive calls.
       if (CalledFunction == &F)
         continue;
       Instruction *DI = I;
       if (!CalledFunction && !PromotedInsns.count(I) &&
           CallSite(I).isIndirectCall())
         for (const auto *FS : findIndirectCallFunctionSamples(*I)) {
for (auto I : CIS) {
         findCalleeFunctionSamples(*I)->findImportedFunctions(
             ImportGUIDs, F.getParent(),
             Samples->getTotalSamples() * SampleProfileHotThreshold / 100);
         continue;
       }
       DebugLoc DLoc = I->getDebugLoc();
       BasicBlock *BB = I->getParent();
       if (InlineFunction(CallSite(DI), IFI)) {
         LocalChanged = true;
         // The call to InlineFunction erases DI, so we can't pass it here.
         ORE->emit(OptimizationRemark(DEBUG_TYPE, "HotInline", DLoc, BB)
                   << "inlined hot callee '"
                   << ore::NV("Callee", CalledFunction) << "' into '"
                   << ore::NV("Caller", &F) << "'");
       }
     }
     if (LocalChanged) {
if (!F.isDeclaration()) {
       retval |= runOnFunction(F, AM);
     }
   if (M.getProfileSummary() == nullptr)
     M.setProfileSummary(Reader->getSummary().getMD(M.getContext()));
   return retval;
 }
 
 bool SampleProfileLoaderLegacyPass::runOnModule(Module &M) {
-  // FIXME: pass in AssumptionCache correctly for the new pass manager.
-  SampleLoader.setACT(&getAnalysis<AssumptionCacheTracker>());
+  ACT = &getAnalysis<AssumptionCacheTracker>();
   return SampleLoader.runOnModule(M, nullptr);
 }
 
 bool SampleProfileLoader::runOnFunction(Function &F, ModuleAnalysisManager *AM) {
   F.setEntryCount(0);
   std::unique_ptr<OptimizationRemarkEmitter> OwnedORE;
   if (AM) {
     auto &FAM =
         AM->getResult<FunctionAnalysisManagerModuleProxy>(*F.getParent())
             .getManager();
     ORE = &FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
   } else {
     OwnedORE = make_unique<OptimizationRemarkEmitter>(&F);
     ORE = OwnedORE.get();
   }
   Samples = Reader->getSamplesFor(F);
   if (Samples && !Samples->empty())
     return emitAnnotations(F);
   return false;
 }
 
 PreservedAnalyses SampleProfileLoaderPass::run(Module &M,
                                                ModuleAnalysisManager &AM) {
+  FunctionAnalysisManager &FAM =
+      AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+
+  auto GetAssumptionCache = [&](Function &F) -> AssumptionCache & {
+    return FAM.getResult<AssumptionAnalysis>(F);
+  };
 
-  SampleProfileLoader SampleLoader(
-      ProfileFileName.empty() ? SampleProfileFile : ProfileFileName);
+  SampleProfileLoader SampleLoader(ProfileFileName.empty() ? SampleProfileFile
+                                                           : ProfileFileName,
+                                   GetAssumptionCache);
 
   SampleLoader.doInitialization(M);
 
   if (!SampleLoader.runOnModule(M, &AM))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
 }