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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Passes/PassBuilder.cpp:552-553
+  if (PGOOpt && !PGOOpt->SampleProfileFile.empty()) {
+    // Annotate sample profile right after early FPM to ensure freshness of
+    // the debug info.
+    MPM.addPass(SampleProfileLoaderPass(PGOOpt->SampleProfileFile));
----------------
Above, you say that this is because SampleProfileLoaderPass needs to be here because it inlines calls. Here you talk about debug info freshness... Which is it?

The debug info freshness makes plenty of sense to me. If SampleProfileLoaderPass is *doing inlining*, I find that deeply surprising and confusing. I'm not suggesting that we need to change the design of this right now, but I think the comment needs to be *really* clear because lots of other people will be surprised by this and I'd suggest a FIXME to revisit how this is structured. (Regardless of whether inlining makes sense here, I think it should *definitely* be separated from what appears to be a pass that merely annotates the IR from a profile.)


https://reviews.llvm.org/D36333