[PATCH] D37773: Refactor the code to pass down ACT to SampleProfileLoader correctly.

[Easwaran Raman via Phabricator via llvm-commits]

eraman added inline comments.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:150
+  SampleProfileLoader(
+      StringRef Name,
+      std::function<AssumptionCache &(Function &)> GetAssumptionCache)
----------------
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.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:753
+      CallSite CS(DI);
+      if (InlineFunction(CS, IFI)) {
         LocalChanged = true;
----------------
Any reason why you want a named temporary?


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:1486
 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);
----------------
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.


================
Comment at: lib/Transforms/IPO/SampleProfile.cpp:1513
 
-  SampleProfileLoader SampleLoader(
-      ProfileFileName.empty() ? SampleProfileFile : ProfileFileName);
+  std::function<AssumptionCache &(Function &)> GetAssumptionCache =
+      [&](Function &F) -> AssumptionCache & {
----------------
you don't need the std::function here. Just use auto. 


https://reviews.llvm.org/D37773