[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Warren Ristow via Phabricator via cfe-commits]

wristow added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:614
   // during ThinLTO and perform the rest of the optimizations afterward.
   if (PrepareForThinLTO) {
     // Ensure we perform any last passes, but do so before renaming anonymous
----------------
wenlei wrote:
> this also need to be `PrepareForThinLTO || PrepareForLTO` for oldPM?
I agree this is another instance where a balancing act question applies.  In this case, assuming the comment about the concern of code bloat is accurate, it's not so much about compile-time resources in the full LTO back-end, but rather about minimizing the ThinLTO bitcode write/read time.  So if as this WIP evolves, it ultimately is a win for SamplePGO to suppress some loop optimizations (unrolling/vectorization) here, then that will probably also be a //small // win in full LTO compile time.  That said, in addition to these loop-related optimizations, there are other transformations here that are done in the full LTO pipeline (but not in the ThinLTO pipeline).  So I suspect if some change to check for `PrepareForThinLTO || PrepareForLTO` (rather than only `PrepareForThinLTO`) makes sense here from a Sample PGO perspective, then the change will be more complicated than simply adding the small set of passes here followed by the early return (that is, I think there are probably things after the `return` on line 621 that still ought to be enabled for full LTO -- essentially continuing to do them in the pre-link stage for full LTO, to try to avoid needing to do too much work in the full LTO backend stage, since it's more of a problem for the full backend to absorb that compile time cost).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69732/new/

https://reviews.llvm.org/D69732