From 716abbeb43824edfde37505f98303d148cbb7a01 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Wed, 6 Feb 2019 04:29:39 +0000 Subject: [PATCH] [HotColdSplit] Move splitting after instrumented PGO use Summary: Follow up to D57082 which moved splitting earlier in the pipeline, in order to perform it before inlining. However, it was moved too early, before the IR is annotated with instrumented PGO data. This caused the splitting to incorrectly determine cold functions. Move it to just after PGO annotation (still before inlining), in both pass managers. Reviewers: vsk, hiraditya, sebpop Subscribers: mehdi_amini, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D57805 llvm-svn: 353270 --- llvm/lib/Passes/PassBuilder.cpp | 16 ++++++++-------- llvm/lib/Transforms/IPO/PassManagerBuilder.cpp | 10 +++++----- llvm/test/Other/Inputs/pass-pipelines.proftext | 1 + llvm/test/Other/new-pm-pgo.ll | 2 ++ llvm/test/Other/pass-pipelines.ll | 16 ++++++++++++++++ 5 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 llvm/test/Other/Inputs/pass-pipelines.proftext diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index aa82f268338ac..8e62c1d0b690a 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -680,14 +680,6 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, // globals. MPM.addPass(DeadArgumentEliminationPass()); - // 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()); - // Create a small function pass pipeline to cleanup after all the global // optimizations. FunctionPassManager GlobalCleanupPM(DebugLogging); @@ -710,6 +702,14 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, 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()); diff --git a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp index 03d7088eab4ec..8f2860ba51b0e 100644 --- a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp +++ b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp @@ -517,11 +517,6 @@ void PassManagerBuilder::populateModulePassManager( MPM.add(createDeadArgEliminationPass()); // Dead argument elimination - // Split out cold code before inlining. See comment in the new PM - // (\ref buildModuleSimplificationPipeline). - if (EnableHotColdSplit && DefaultOrPreLinkPipeline) - MPM.add(createHotColdSplittingPass()); - addInstructionCombiningPass(MPM); // Clean up after IPCP & DAE addExtensionsToPM(EP_Peephole, MPM); MPM.add(createCFGSimplificationPass()); // Clean up after IPCP & DAE @@ -534,6 +529,11 @@ void PassManagerBuilder::populateModulePassManager( 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. diff --git a/llvm/test/Other/Inputs/pass-pipelines.proftext b/llvm/test/Other/Inputs/pass-pipelines.proftext new file mode 100644 index 0000000000000..04a7c1c1a35aa --- /dev/null +++ b/llvm/test/Other/Inputs/pass-pipelines.proftext @@ -0,0 +1 @@ +:ir diff --git a/llvm/test/Other/new-pm-pgo.ll b/llvm/test/Other/new-pm-pgo.ll index c1a26b449c113..916309e625b2c 100644 --- a/llvm/test/Other/new-pm-pgo.ll +++ b/llvm/test/Other/new-pm-pgo.ll @@ -1,6 +1,7 @@ ; RUN: opt -debug-pass-manager -passes='default' -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' -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' -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' -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' -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-pgo.prof' %s 2>&1 \ @@ -12,6 +13,7 @@ ; 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{{.*}}> diff --git a/llvm/test/Other/pass-pipelines.ll b/llvm/test/Other/pass-pipelines.ll index dddf1338a4d01..dfddcee55e833 100644 --- a/llvm/test/Other/pass-pipelines.ll +++ b/llvm/test/Other/pass-pipelines.ll @@ -6,6 +6,16 @@ ; RUN: opt -disable-output -disable-verify -debug-pass=Structure \ ; RUN: -O2 %s 2>&1 \ ; RUN: | FileCheck %s --check-prefix=CHECK-O2 +; RUN: llvm-profdata merge %S/Inputs/pass-pipelines.proftext -o %t.profdata +; RUN: opt -disable-output -disable-verify -debug-pass=Structure \ +; RUN: -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' \ +; RUN: -O2 %s 2>&1 \ +; RUN: | FileCheck %s --check-prefix=CHECK-O2 --check-prefix=PGOUSE +; RUN: opt -disable-output -disable-verify -debug-pass=Structure \ +; RUN: -pgo-kind=pgo-instr-use-pipeline -profile-file='%t.profdata' \ +; RUN: -hot-cold-split \ +; RUN: -O2 %s 2>&1 \ +; RUN: | FileCheck %s --check-prefix=CHECK-O2 --check-prefix=PGOUSE --check-prefix=SPLIT ; ; In the first pipeline there should just be a function pass manager, no other ; pass managers. @@ -27,6 +37,12 @@ ; 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