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

[David Li via Phabricator via llvm-commits]

davidxl 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));
----------------
chandlerc wrote:
> 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.)
Should probably add a reference to the autofdo CGO 2016 paper which has detailed description of how autofdo's context senstive profile data is organized and the profile driven inline and branch profile annotation design.


https://reviews.llvm.org/D36333