[PATCH] D36333: Move the SampleProfileLoader right after EarlyFPM.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/Passes/PassBuilder.cpp:548
+      Phase == ThinLTOPhase::PostLink)
+    EarlyFPM.addPass(InstCombinePass());
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(EarlyFPM)));
----------------
What is the downside of always doing InstCombine here?


================
Comment at: lib/Passes/PassBuilder.cpp:552
+  if (PGOOpt && !PGOOpt->SampleProfileFile.empty()) {
+    // Annotate sample profile right after early FPM to ensure freshness of
+    // the debug info.
----------------
Also needs comment about why before globalopt.


================
Comment at: lib/Passes/PassBuilder.cpp:851
   // look unreferenced and are removed.
-  MPM.addPass(PGOIndirectCallPromotion(
-      true /* InLTO */, PGOOpt && !PGOOpt->SampleProfileFile.empty()));
+  if (PGOOpt && !PGOOpt->ProfileUseFile.empty())
+    MPM.addPass(PGOIndirectCallPromotion(true /* InLTO */,
----------------
Can we simply invoke PGO ICP always from within buildModuleSimplificationPipeline if we're not in the ThinLTO PreLink phase? I.e. regardless of Sample or Instrumented PGO? It seems cleaner to do it in the same place if possible.


https://reviews.llvm.org/D36333