[PATCH] D57082: [HotColdSplit] Move splitting earlier in the pipeline

Wed Jan 23 11:46:33 PST 2019

vsk added inline comments.

================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:521
+  // accurate, but before InstCombine to allow it to clean things up.
+  if (EnableHotColdSplit && !PrepareForLTO && !PrepareForThinLTO)
+    MPM.add(createHotColdSplittingPass());
----------------
tejohnson wrote:
> I think you have the wrong LTO guards. This is the opposite of the ThinLTO guard you have in the new PM. Here you are preventing this from running during the ThinLTO (and regular LTO) pre-link steps. The following are equivalent:
>    PrepareForThinLTO (oldPM) == ThinLTOPhase::PreLink (newPM)
>    PerformThinLTO (oldPM) == ThinLTOPhase::PostLink (newPM)
> 
> Note that there is no PerformLTO, since in the old PM this function is not even called during the regular LTO post link (which has a different, specialized pipeline). So in a nutshell, I believe this should be:
> 
>    if (EnableHotColdSplit && !PerformThinLTO)
> 
> This will give you splitting during a non-LTO compile, and during the pre-link *LTO compiles.
> 
Thanks for explaining this.

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

https://reviews.llvm.org/D57082