Disable loop unrolling and icp in SamplePGO ThinLTO compile phase

[danielcdh]

Summary

loop unrolling and icp will make the sample profile annotation much harder in the backend. So disable these 2 optimization in the ThinLTO compile phase.
Will add a test in cfe in a separate patch.

[mehdi_amini]

lib/Transforms/IPO/PassManagerBuilder.cpp

464

That makes me worried in some way: we're not expecting ICP and SamplePGO to be complementary from each other?

[danielcdh]

lib/Transforms/IPO/PassManagerBuilder.cpp

464

Let me try to clarify here.

• ICP is invoked twice in thinlto: once in compile phase to cover intra-module promotion, another in backend to handle inter-module promotion. The first icp is aiming at reduce callgraph size thus reduce the thinlink burden, but it's not required.
• SamplePGO do interacts a lot with ICP. If an indirect call is inlined in the profile, then before annotation, we need to promote it and inline it as well. This is done before the profile annotation. But after the profile annotation, when we find more ICP opportunities, it will simply use the legacy ICP pass.

So in this patch, instead of invoking legacy ICP pass twice, we only invoke it once in backend if it is samplepgo+thinlto build.

[mehdi_amini]

lib/Transforms/IPO/PassManagerBuilder.cpp

464

Oh OK I missed that it was just the early ICP. Nevermind then.

[tejohnson]

LGTM with one suggestion

lib/Transforms/IPO/PassManagerBuilder.cpp

465

All the "!" make this a little difficult to parse. Since you are doing the same check a couple places anyway, I suggest adding a boolean up above like:
bool PrepareForThinLTOUsingPGOSampleProfile = PrepareForThinLTO && !PGOSampleUse.empty();

It's a mouthful, but hopefully clearer.

lib/Transforms/IPO/PassManagerBuilder.cpp

void PassManagerBuilder::populateModulePassManager(
   // the ThinLTO backend when PerformThinLTO=true, when we promote imported
   // inter-module indirect calls. For that we perform indirect call promotion
   // earlier in the pass pipeline, here before globalopt. Otherwise imported
   // available_externally functions look unreferenced and are removed.
   if (PerformThinLTO)
     MPM.add(createPGOIndirectCallPromotionLegacyPass(/*InLTO = */ true,
                                                      !PGOSampleUse.empty()));
 
+  // For SamplePGO in ThinLTO compile phase, we do not want to unroll loops
+  // as it will change the CFG too much to make the 2nd profile annotation
+  // in backend more difficult.
+  if (PrepareForThinLTO && !PGOSampleUse.empty())
+    DisableUnrollLoops = true;
+
   if (!DisableUnitAtATime) {
     // Infer attributes about declarations if possible.
     MPM.add(createInferFunctionAttrsLegacyPass());
 
     addExtensionsToPM(EP_ModuleOptimizerEarly, MPM);
 
     MPM.add(createIPSCCPPass());          // IP SCCP
     MPM.add(createGlobalOptimizerPass()); // Optimize out global vars
     // Promote any localized global vars.
     MPM.add(createPromoteMemoryToRegisterPass());
 
     MPM.add(createDeadArgEliminationPass()); // Dead argument elimination
 
     addInstructionCombiningPass(MPM); // Clean up after IPCP & DAE
     addExtensionsToPM(EP_Peephole, MPM);
     MPM.add(createCFGSimplificationPass()); // Clean up after IPCP & DAE
   }
 
-  if (!PerformThinLTO) {
+  // For SamplePGO in ThinLTO compile phase, we do not want to do indirect
+  // call promotion as it will change the CFG too much to make the 2nd
+  // profile annotation in backend more difficult.
+  if (!PerformThinLTO && !(PrepareForThinLTO && !PGOSampleUse.empty())) {
     /// PGO instrumentation is added during the compile phase for ThinLTO, do
     /// not run it a second time
     addPGOInstrPasses(MPM);
     // Indirect call promotion that promotes intra-module targets only.
     // For ThinLTO this is done earlier due to interactions with globalopt
     // for imported functions.
     MPM.add(
         createPGOIndirectCallPromotionLegacyPass(false, !PGOSampleUse.empty()));