[PATCH] D36040: Refine the PGOOpt and SamplePGOSupport handling.

[Dehao Chen via Phabricator via llvm-commits]

danielcdh marked an inline comment as done.
danielcdh added inline comments.


================
Comment at: lib/Passes/PassBuilder.cpp:589-590
     // it changes IR to makes profile annotation in back compile inaccurate.
-    if (!PrepareForThinLTO || PGOOpt->SampleProfileFile.empty())
+    if ((!PrepareForThinLTO && !PGOOpt->SampleProfileFile.empty())
+        || !PGOOpt->ProfileUseFile.empty())
       MPM.addPass(PGOIndirectCallPromotion(
----------------
chandlerc wrote:
> danielcdh wrote:
> > chandlerc wrote:
> > > Why do we want to do this *before* the thin link with instrumentation PGO but not with sample PGO?
> > As illustrated in the comment, we do not enable it in PrepareForThinLTO phase during sample PGO because it changes IR to makes profile annotation in backend compile inaccurate.
> > 
> > I'm not sure why ICP is invoked twice in both prelink and backend.
> I suspect we should follow-up on this and change sample PGO and instrumentation PGO to behave the same way here (or have a clear explanation of why they differ).
> 
> But that can happen later.
Acknowledged.

Teresa and David, do you remember why we invoke ICP twice in instrumentation based FDO?


https://reviews.llvm.org/D36040