[HotColdSplit] Schedule splitting late to fix perf regression

[vsk]

Summary

With or without PGO data applied, splitting early in the pipeline
(either before the inliner or shortly after it) regresses performance
across SPEC variants. The cause appears to be that splitting hides
context for subsequent optimizations.

Schedule splitting late again, in effect reversing r352080, which
scheduled the splitting pass early for code size benefits (documented in
https://reviews.llvm.org/D57082).

rdar://47622429

[tejohnson]

llvm/lib/Passes/PassBuilder.cpp

764 ↗

(On Diff #186928)

I see in the old PM that splitting is being moved post link for full LTO as well. IMO it should be in roughly the same place for the new PM. I guess the issue here is that there is no indication at this point that we are doing full LTO pre-link vs non-LTO. I would say either keep it pre-link in both PMs for full LTO or maybe better to pass down some indication that we are in the full LTO pre-link and not do it here in that case either (and then add it to buildLTODefaultPipeline as well to get it enabled for full LTO post-link). I'm reviewing another patch that needs to know this as well (D54175) and is adding a "bool LTOPreLink" parameter to this method. So maybe use the same thing?

llvm/test/Other/opt-hot-cold-split.ll

4 ↗

(On Diff #186928)

Can you add a regular LTO postlink invocation as well? That would be --passes='lto<Os'

[vsk]

llvm/lib/Passes/PassBuilder.cpp

764 ↗

(On Diff #186928)

I'm not sure whether there's an advantage to scheduling splitting twice (as in, I haven't tested this out), but there's at least a compile-time argument for only doing it once. @xur would you be ok with committing your bool LTOPreLink change from D54175 early? Happy to send in patch for this if you'd prefer.

[tejohnson]

llvm/lib/Passes/PassBuilder.cpp

764 ↗

(On Diff #186928)

I don't think we should do it twice, it should only be done once. The issue is that it is being added in different places in each PM with this patch. So my suggestion is to either move it post link here or pre link in the other PM.

@xur is out for a couple weeks, so if you want to add that part in the meantime that would be great!

[vsk]

• Add a NewPM LTO pre-link test.
• Schedule splitting at the same point in the old & new PM's.

[tejohnson]

LGTM

llvm/trunk/include/llvm/Passes/PassBuilder.h

public:
   /// without potentially seriously regressing either runtime performance of
   /// the code or serious code size growth.
   ///
   /// Note that \p Level cannot be `O0` here. The pipelines produced are
   /// only intended for use when attempting to optimize code. If frontends
   /// require some transformations for semantic reasons, they should explicitly
   /// build them.
   ModulePassManager buildModuleOptimizationPipeline(OptimizationLevel Level,
-                                                    bool DebugLogging = false);
+                                                    bool DebugLogging = false,
+                                                    bool LTOPreLink = false);
 
   /// Build a per-module default optimization pipeline.
   ///
   /// This provides a good default optimization pipeline for per-module
   /// optimization and code generation without any link-time optimization. It
   /// typically correspond to frontend "-O[123]" options for optimization
   /// levels \c O1, \c O2 and \c O3 resp.
   ///
   /// Note that \p Level cannot be `O0` here. The pipelines produced are
   /// only intended for use when attempting to optimize code. If frontends
   /// require some transformations for semantic reasons, they should explicitly
   /// build them.
   ModulePassManager buildPerModuleDefaultPipeline(OptimizationLevel Level,
-                                                  bool DebugLogging = false);
+                                                  bool DebugLogging = false,
+                                                  bool LTOPreLink = false);
 
   /// Build a pre-link, ThinLTO-targeting default optimization pipeline to
   /// a pass manager.
   ///
   /// This adds the pre-link optimizations tuned to prepare a module for
   /// a ThinLTO run. It works to minimize the IR which needs to be analyzed
   /// without making irreversible decisions which could be made better during
   /// the LTO run.

llvm/trunk/lib/Passes/PassBuilder.cpp

addPGOInstrPasses(MPM, DebugLogging, Level, PGOOpt->RunProfileGen,
                       PGOOpt->ProfileRemappingFile);
     MPM.addPass(PGOIndirectCallPromotion(false, false));
   }
 
   // Synthesize function entry counts for non-PGO compilation.
   if (EnableSyntheticCounts && !PGOOpt)
     MPM.addPass(SyntheticCountsPropagation());
 
-  // Split out cold code. Splitting is done before inlining because 1) the most
-  // common kinds of cold regions can (a) be found before inlining and (b) do
-  // not grow after inlining, and 2) inhibiting inlining of cold code improves
-  // code size & compile time. Split after Mem2Reg to make code model estimates
-  // more accurate, but before InstCombine to allow it to clean things up.
-  if (EnableHotColdSplit && Phase != ThinLTOPhase::PostLink)
-    MPM.addPass(HotColdSplittingPass());
-
   // Require the GlobalsAA analysis for the module so we can query it within
   // the CGSCC pipeline.
   MPM.addPass(RequireAnalysisPass<GlobalsAA, Module>());
 
   // Require the ProfileSummaryAnalysis for the module so we can query it within
   // the inliner pass.
   MPM.addPass(RequireAnalysisPass<ProfileSummaryAnalysis, Module>());
 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // in postorder (or bottom-up).
   MPM.addPass(
       createModuleToPostOrderCGSCCPassAdaptor(createDevirtSCCRepeatedPass(
           std::move(MainCGPipeline), MaxDevirtIterations)));
 
   return MPM;
 }
 
-ModulePassManager
-PassBuilder::buildModuleOptimizationPipeline(OptimizationLevel Level,
-                                             bool DebugLogging) {
+ModulePassManager PassBuilder::buildModuleOptimizationPipeline(
+    OptimizationLevel Level, bool DebugLogging, bool LTOPreLink) {
   ModulePassManager MPM(DebugLogging);
 
   // Optimize globals now that the module is fully simplified.
   MPM.addPass(GlobalOptPass());
   MPM.addPass(GlobalDCEPass());
 
   // Run partial inlining pass to partially inline functions that have
   // large bodies.
ModulePassManager PassBuilder::buildModuleOptimizationPipeline(
   OptimizePM.addPass(InstCombinePass());
   OptimizePM.addPass(RequireAnalysisPass<OptimizationRemarkEmitterAnalysis, Function>());
   OptimizePM.addPass(createFunctionToLoopPassAdaptor(LICMPass(), DebugLogging));
 
   // Now that we've vectorized and unrolled loops, we may have more refined
   // alignment information, try to re-derive it here.
   OptimizePM.addPass(AlignmentFromAssumptionsPass());
 
+  // Split out cold code. Splitting is done late to avoid hiding context from
+  // other optimizations and inadvertently regressing performance. The tradeoff
+  // is that this has a higher code size cost than splitting early.
+  if (EnableHotColdSplit && !LTOPreLink)
+    MPM.addPass(HotColdSplittingPass());
+
   // LoopSink pass sinks instructions hoisted by LICM, which serves as a
   // canonicalization pass that enables other optimizations. As a result,
   // LoopSink pass needs to be a very late IR pass to avoid undoing LICM
   // result too early.
   OptimizePM.addPass(LoopSinkPass());
 
   // And finally clean up LCSSA form before generating code.
   OptimizePM.addPass(InstSimplifyPass());
ModulePassManager PassBuilder::buildModuleOptimizationPipeline(
   MPM.addPass(GlobalDCEPass());
   MPM.addPass(ConstantMergePass());
 
   return MPM;
 }
 
 ModulePassManager
 PassBuilder::buildPerModuleDefaultPipeline(OptimizationLevel Level,
-                                           bool DebugLogging) {
+                                           bool DebugLogging, bool LTOPreLink) {
   assert(Level != O0 && "Must request optimizations for the default pipeline!");
 
   ModulePassManager MPM(DebugLogging);
 
   // Force any function attributes we want the rest of the pipeline to observe.
   MPM.addPass(ForceFunctionAttrsPass());
 
   // Apply module pipeline start EP callback.
   for (auto &C : PipelineStartEPCallbacks)
     C(MPM);
 
   if (PGOOpt && PGOOpt->SamplePGOSupport)
     MPM.addPass(createModuleToFunctionPassAdaptor(AddDiscriminatorsPass()));
 
   // Add the core simplification pipeline.
   MPM.addPass(buildModuleSimplificationPipeline(Level, ThinLTOPhase::None,
                                                 DebugLogging));
 
   // Now add the optimization pipeline.
-  MPM.addPass(buildModuleOptimizationPipeline(Level, DebugLogging));
+  MPM.addPass(buildModuleOptimizationPipeline(Level, DebugLogging, LTOPreLink));
 
   return MPM;
 }
 
 ModulePassManager
 PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level,
                                                 bool DebugLogging) {
   assert(Level != O0 && "Must request optimizations for the default pipeline!");
ModulePassManager PassBuilder::buildThinLTODefaultPipeline(
   return MPM;
 }
 
 ModulePassManager
 PassBuilder::buildLTOPreLinkDefaultPipeline(OptimizationLevel Level,
                                             bool DebugLogging) {
   assert(Level != O0 && "Must request optimizations for the default pipeline!");
   // FIXME: We should use a customized pre-link pipeline!
-  return buildPerModuleDefaultPipeline(Level, DebugLogging);
+  return buildPerModuleDefaultPipeline(Level, DebugLogging,
+                                       /*LTOPreLink=*/true);
 }
 
 ModulePassManager
 PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level, bool DebugLogging,
                                      ModuleSummaryIndex *ExportSummary) {
   assert(Level != O0 && "Must request optimizations for the default pipeline!");
   ModulePassManager MPM(DebugLogging);
 
#endif
   MPM.addPass(CrossDSOCFIPass());
 
   // Lower type metadata and the type.test intrinsic. This pass supports
   // clang's control flow integrity mechanisms (-fsanitize=cfi*) and needs
   // to be run at link time if CFI is enabled. This pass does nothing if
   // CFI is disabled.
   MPM.addPass(LowerTypeTestsPass(ExportSummary, nullptr));
 
+  // Enable splitting late in the FullLTO post-link pipeline. This is done in
+  // the same stage in the old pass manager (\ref addLateLTOOptimizationPasses).
+  if (EnableHotColdSplit)
+    MPM.addPass(HotColdSplittingPass());
+
   // Add late LTO optimization passes.
   // Delete basic blocks, which optimization passes may have killed.
   MPM.addPass(createModuleToFunctionPassAdaptor(SimplifyCFGPass()));
 
   // Drop bodies of available eternally objects to improve GlobalDCE.
   MPM.addPass(EliminateAvailableExternallyPass());
 
   // Now that we have optimized the program, discard unreachable functions.

llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp

void PassManagerBuilder::populateModulePassManager(
   // 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.
   // PGO instrumentation is added during the compile phase for ThinLTO, do
   // not run it a second time
   if (DefaultOrPreLinkPipeline && !PrepareForThinLTOUsingPGOSampleProfile)
     addPGOInstrPasses(MPM);
 
-  // Split out cold code before inlining. See comment in the new PM
-  // (\ref buildModuleSimplificationPipeline).
-  if (EnableHotColdSplit && DefaultOrPreLinkPipeline)
-    MPM.add(createHotColdSplittingPass());
-
   // We add a module alias analysis pass here. In part due to bugs in the
   // analysis infrastructure this "works" in that the analysis stays alive
   // for the entire SCC pass run below.
   MPM.add(createGlobalsAAWrapperPass());
 
   // Start of CallGraph SCC passes.
   MPM.add(createPruneEHPass()); // Remove dead EH info
   bool RunInliner = false;
void PassManagerBuilder::populateModulePassManager(
 
   // GlobalOpt already deletes dead functions and globals, at -O2 try a
   // late pass of GlobalDCE.  It is capable of deleting dead cycles.
   if (OptLevel > 1) {
     MPM.add(createGlobalDCEPass());         // Remove dead fns and globals.
     MPM.add(createConstantMergePass());     // Merge dup global constants
   }
 
+  // See comment in the new PM for justification of scheduling splitting at
+  // this stage (\ref buildModuleSimplificationPipeline).
+  if (EnableHotColdSplit && !(PrepareForLTO || PrepareForThinLTO))
+    MPM.add(createHotColdSplittingPass());
+
   if (MergeFunctions)
     MPM.add(createMergeFunctionsPass());
 
   // LoopSink pass sinks instructions hoisted by LICM, which serves as a
   // canonicalization pass that enables other optimizations. As a result,
   // LoopSink pass needs to be a very late IR pass to avoid undoing LICM
   // result too early.
   MPM.add(createLoopSinkPass());
void PassManagerBuilder::addLTOOptimizationPasses(legacy::PassManagerBase &PM) {
   addInstructionCombiningPass(PM);
   addExtensionsToPM(EP_Peephole, PM);
 
   PM.add(createJumpThreadingPass());
 }
 
 void PassManagerBuilder::addLateLTOOptimizationPasses(
     legacy::PassManagerBase &PM) {
+  // See comment in the new PM for justification of scheduling splitting at
+  // this stage (\ref buildLTODefaultPipeline).
+  if (EnableHotColdSplit)
+    PM.add(createHotColdSplittingPass());
+
   // Delete basic blocks, which optimization passes may have killed.
   PM.add(createCFGSimplificationPass());
 
   // Drop bodies of available externally objects to improve GlobalDCE.
   PM.add(createEliminateAvailableExternallyPass());
 
   // Now that we have optimized the program, discard unreachable functions.
   PM.add(createGlobalDCEPass());

llvm/trunk/test/Other/X86/lto-hot-cold-split.ll

 ; RUN: opt -module-summary %s -o %t.bc
-; RUN: llvm-lto -hot-cold-split=true -thinlto-action=run %t.bc -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=OLDPM-THINLTO-POSTLINK-Os
+; RUN: llvm-lto -hot-cold-split=true -thinlto-action=run %t.bc -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=OLDPM-ANYLTO-POSTLINK-Os
+; RUN: llvm-lto -hot-cold-split=true %t.bc -debug-pass=Structure 2>&1 | FileCheck %s -check-prefix=OLDPM-ANYLTO-POSTLINK-Os
 
 ; REQUIRES: asserts
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-; OLDPM-THINLTO-POSTLINK-Os-LABEL: Pass Arguments
-; OLDPM-THINLTO-POSTLINK-Os-NOT: Hot Cold Splitting
+; OLDPM-ANYLTO-POSTLINK-Os: Hot Cold Splitting

llvm/trunk/test/Other/new-pm-pgo.ll

 ; RUN: opt -debug-pass-manager -passes='default<O2>' -pgo-kind=pgo-instr-gen-pipeline -profile-file='temp' %s 2>&1 |FileCheck %s --check-prefixes=GEN
 ; RUN: llvm-profdata merge %S/Inputs/new-pm-pgo.proftext -o %t.profdata
 ; RUN: opt -debug-pass-manager -passes='default<O2>' -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' %s 2>&1 |FileCheck %s --check-prefixes=USE
 ; RUN: opt -debug-pass-manager -passes='default<O2>' -hot-cold-split -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' %s 2>&1 |FileCheck %s --check-prefixes=USE --check-prefixes=SPLIT
 ; RUN: opt -debug-pass-manager -passes='default<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-pgo.prof' %s 2>&1 \
 ; RUN:     |FileCheck %s --check-prefixes=SAMPLE_USE,SAMPLE_USE_O
 ; RUN: opt -debug-pass-manager -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-pgo.prof' %s 2>&1 \
 ; RUN:     |FileCheck %s --check-prefixes=SAMPLE_USE,SAMPLE_USE_PRE_LINK
 ; RUN: opt -debug-pass-manager -passes='thinlto<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-pgo.prof' %s 2>&1 \
 ; RUN:     |FileCheck %s --check-prefixes=SAMPLE_USE,SAMPLE_USE_POST_LINK
 ; RUN: opt -debug-pass-manager -passes='default<O2>' -new-pm-debug-info-for-profiling %s 2>&1 |FileCheck %s --check-prefixes=SAMPLE_GEN
 ;
 ; GEN: Running pass: PGOInstrumentationGen
 ; USE: Running pass: PGOInstrumentationUse
 ; USE: Running pass: PGOIndirectCallPromotion
-; SPLIT: Running pass: HotColdSplittingPass
 ; USE: Running pass: PGOMemOPSizeOpt
 ; SAMPLE_USE_O: Running pass: ModuleToFunctionPassAdaptor<{{.*}}AddDiscriminatorsPass{{.*}}>
 ; SAMPLE_USE_PRE_LINK: Running pass: ModuleToFunctionPassAdaptor<{{.*}}AddDiscriminatorsPass{{.*}}>
 ; SAMPLE_USE: Running pass: SimplifyCFGPass
 ; SAMPLE_USE: Running pass: SROA
 ; SAMPLE_USE: Running pass: EarlyCSEPass
 ; SAMPLE_USE: Running pass: LowerExpectIntrinsicPass
 ; SAMPLE_USE_POST_LINK: Running pass: InstCombinePass
 ; SAMPLE_USE: Running pass: SampleProfileLoaderPass
 ; SAMPLE_USE_O: Running pass: PGOIndirectCallPromotion
 ; SAMPLE_USE_POST_LINK-NOT: Running pass: GlobalOptPass
 ; SAMPLE_USE_POST_LINK: Running pass: PGOIndirectCallPromotion
 ; SAMPLE_GEN: Running pass: ModuleToFunctionPassAdaptor<{{.*}}AddDiscriminatorsPass{{.*}}>
+; SPLIT: Running pass: HotColdSplittingPass
 
 define void @foo() {
   ret void
 }

llvm/trunk/test/Other/opt-hot-cold-split.ll

 ; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -debug-pass=Structure < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=DEFAULT-Os
 ; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -passes='lto-pre-link<Os>' -debug-pass-manager < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=LTO-PRELINK-Os
 ; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -passes='thinlto-pre-link<Os>' -debug-pass-manager < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=THINLTO-PRELINK-Os
+; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -passes='lto<Os>' -debug-pass-manager < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=LTO-POSTLINK-Os
 ; RUN: opt -mtriple=x86_64-- -Os -hot-cold-split=true -passes='thinlto<Os>' -debug-pass-manager < %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=THINLTO-POSTLINK-Os
 
 ; REQUIRES: asserts
 
-; Splitting should occur after Mem2Reg and should be followed by InstCombine.
+; Splitting should occur late.
 
-; DEFAULT-Os: Promote Memory to Register
 ; DEFAULT-Os: Hot Cold Splitting
-; DEFAULT-Os: Combine redundant instructions
+; DEFAULT-Os: Simplify the CFG
 
-; LTO-PRELINK-Os-LABEL: Starting llvm::Module pass manager run.
-; LTO-PRELINK-Os: Running pass: {{.*}}PromotePass
-; LTO-PRELINK-Os: Running pass: HotColdSplittingPass
+; LTO-PRELINK-Os-NOT: pass: HotColdSplittingPass
 
-; THINLTO-PRELINK-Os-LABEL: Running analysis: PassInstrumentationAnalysis
-; THINLTO-PRELINK-Os: Running pass: {{.*}}PromotePass
-; THINLTO-PRELINK-Os: Running pass: HotColdSplittingPass
+; THINLTO-PRELINK-Os-NOT: Running pass: HotColdSplittingPass
 
-; THINLTO-POSTLINK-Os-NOT: HotColdSplitting
+; LTO-POSTLINK-Os: HotColdSplitting
+; THINLTO-POSTLINK-Os: HotColdSplitting

llvm/trunk/test/Other/pass-pipelines.ll

 ; CHECK-O2-NOT: Manager
 ; Very carefully assert the CGSCC pass pipeline as it is fragile and unusually
 ; susceptible to phase ordering issues.
 ; CHECK-O2: CallGraph Construction
 ; PGOUSE: Call Graph SCC Pass Manager
 ; PGOUSE:      Function Integration/Inlining
 ; PGOUSE: PGOInstrumentationUsePass
 ; PGOUSE: PGOIndirectCallPromotion
-; SPLIT: Hot Cold Splitting
 ; PGOUSE: CallGraph Construction
 ; CHECK-O2-NEXT: Globals Alias Analysis
 ; CHECK-O2-NEXT: Call Graph SCC Pass Manager
 ; CHECK-O2-NEXT: Remove unused exception handling info
 ; CHECK-O2-NEXT: Function Integration/Inlining
 ; CHECK-O2-NEXT: Deduce function attributes
 ; Next up is the main function pass pipeline. It shouldn't be split up and
 ; should contain the main loop pass pipeline as well.

 ; CHECK-O2: Loop Pass Manager
 ; CHECK-O2-NEXT: Unroll loops
 ; CHECK-O2-NOT: Manager
 ; After vectorization and unrolling we try to do any cleanup of inserted code,
 ; including a run of LICM. This shouldn't run in the same loop pass manager as
 ; the runtime unrolling though.
 ; CHECK-O2: Loop Pass Manager
 ; CHECK-O2-NEXT: Loop Invariant Code Motion
+; SPLIT: Hot Cold Splitting
 ; CHECK-O2: FunctionPass Manager
 ; CHECK-O2: Loop Pass Manager
 ; CHECK-O2-NEXT: Loop Sink
 ; CHECK-O2: Simplify the CFG
 ; CHECK-O2-NOT: Manager
 ;
 ; FIXME: There really shouldn't be another pass manager, especially one that
 ; just builds the domtree. It doesn't even run the verifier.
 ; CHECK-O2: Pass Arguments:
 ; CHECK-O2: FunctionPass Manager
 ; CHECK-O2-NEXT: Dominator Tree Construction
 
 define void @foo() {
   ret void
 }