[SampleFDO] Skip profile reading when flatten profile is used in ThinLTO postlink phase

[wmi]

Summary

If the sample profile has no inlining hierachy information included, we call the sample profile is flatten. For flatten profile, in ThinLTO postlink phase, SampleProfileLoader's hot function inlining and profile annotation will do nothing, so it is better to save the effort to read in the profile (pass running will be skipped too because no profile is available). It is helpful for reducing compile time when the flatten profile is huge.

[tejohnson]

I just stumbled on this patch - sorry for the late review! I thought this was already in. A few comments and a question below. Also, presumably this should be done for the old PM as well?

lib/Passes/PassBuilder.cpp

622

Rather than passing this down and skipping the initialization in the pass, can we just skip adding the pass completely?

lib/Transforms/IPO/SampleProfile.cpp

126 ↗

(On Diff #174994)

s/flatten/flattened/ here and in name/description below (and in rest of patch)

310 ↗

(On Diff #174994)

Nit, add empty line above

[wmi]

Yes, the support is also helpful for old pass manager. Add some code to make it easier to enable sampleprofileloader pass in the pipeline of old pass manager through opt, mainly for testing purpose.

lib/Passes/PassBuilder.cpp

622

Done.

lib/Transforms/IPO/SampleProfile.cpp

126 ↗

(On Diff #174994)

Fixed.

[wmi]

Address Teresa's comments.

[tejohnson]

lib/Passes/PassBuilder.cpp

623

Minor nit: to me this would be more understandable if written as:

if (!(FlattenedProfileUsed && Phase == ThinLTOPhase::PostLink))

(ditto for the old PM check, although I have another note down there that is more fundamental).

lib/Transforms/IPO/PassManagerBuilder.cpp

429

By guarding this with PrepareForThinLTO, I think that means that with a flattened profile and a non-thinlto build we would not get the sample PGO loaded. Should PrepareForThinLTO instead be !PerformThinLTO (which is equivalent to your new PM check)?

tools/opt/opt.cpp

391 ↗

(On Diff #181414)

How have we been testing the PGO pipelines in the old PM without this change?

[wmi]

lib/Transforms/IPO/PassManagerBuilder.cpp

429

Good catch. Thanks. A new option to control PerformThinLTO is added for testing purpose.

tools/opt/opt.cpp

391 ↗

(On Diff #181414)

There are some opt options defined in PassManagerBuilder.cpp for PGO pipeline testing. I feel like it is better to use the mechanism in opt driver similar as what new pass manager does. I will send out a clean up patch first.

[wmi]

Address Teresa's comments.

[tejohnson]

LGTM

lib/Passes/PassBuilder.cpp

static Regex DefaultAliasRegex(
     "^(default|thinlto-pre-link|thinlto|lto-pre-link|lto)<(O[0123sz])>$");
 
 static cl::opt<bool>
     EnableCHR("enable-chr-npm", cl::init(true), cl::Hidden,
               cl::desc("Enable control height reduction optimization (CHR)"));
 
 extern cl::opt<bool> EnableHotColdSplit;
 
+extern cl::opt<bool> FlattenedProfileUsed;
+
 static bool isOptimizingForSize(PassBuilder::OptimizationLevel Level) {
   switch (Level) {
   case PassBuilder::O0:
   case PassBuilder::O1:
   case PassBuilder::O2:
   case PassBuilder::O3:
     return false;
 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   if (PGOOpt && !PGOOpt->SampleProfileFile.empty() &&
       Phase == ThinLTOPhase::PostLink)
     EarlyFPM.addPass(InstCombinePass());
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(EarlyFPM)));
 
   if (PGOOpt && !PGOOpt->SampleProfileFile.empty()) {
     // Annotate sample profile right after early FPM to ensure freshness of
     // the debug info.
+    // In ThinLTO mode, when flattened profile is used, all the available
+    // profile information will be annotated in PreLink phase so there is
+    // no need to load the profile again in PostLink.
+    if (!(FlattenedProfileUsed && Phase == ThinLTOPhase::PostLink))
       MPM.addPass(SampleProfileLoaderPass(PGOOpt->SampleProfileFile,
                                           PGOOpt->ProfileRemappingFile,
                                           Phase == ThinLTOPhase::PreLink));
     // Do not invoke ICP in the ThinLTOPrelink phase as it makes it hard
     // for the profile annotation to be accurate in the ThinLTO backend.
     if (Phase != ThinLTOPhase::PreLink)
       // We perform early indirect call promotion here, before globalopt.
       // This is important for the ThinLTO backend phase because otherwise
       // imported available_externally functions look unreferenced and are
       // removed.
       MPM.addPass(PGOIndirectCallPromotion(Phase == ThinLTOPhase::PostLink,

lib/Transforms/IPO/PassManagerBuilder.cpp

 static cl::opt<bool> EnableUnrollAndJam("enable-unroll-and-jam",
                                         cl::init(false), cl::Hidden,
                                         cl::desc("Enable Unroll And Jam Pass"));
 
 static cl::opt<bool>
     EnablePrepareForThinLTO("prepare-for-thinlto", cl::init(false), cl::Hidden,
                             cl::desc("Enable preparation for ThinLTO."));
 
+static cl::opt<bool>
+    EnablePerformThinLTO("perform-thinlto", cl::init(false), cl::Hidden,
+                         cl::desc("Enable performing ThinLTO."));
+
 cl::opt<bool> EnableHotColdSplit("hot-cold-split", cl::init(false), cl::Hidden,
     cl::desc("Enable hot-cold splitting pass"));
 
 static cl::opt<bool> UseLoopVersioningLICM(
     "enable-loop-versioning-licm", cl::init(false), cl::Hidden,
     cl::desc("Enable the experimental Loop Versioning LICM pass"));
 
 static cl::opt<bool>

 static cl::opt<bool> EnableGVNSink(
     "enable-gvn-sink", cl::init(false), cl::Hidden,
     cl::desc("Enable the GVN sinking pass (default = off)"));
 
 static cl::opt<bool>
     EnableCHR("enable-chr", cl::init(true), cl::Hidden,
               cl::desc("Enable control height reduction optimization (CHR)"));
 
+cl::opt<bool> FlattenedProfileUsed(
+    "flattened-profile-used", cl::init(false), cl::Hidden,
+    cl::desc("Indicate the sample profile being used is flattened, i.e., "
+             "no inline hierachy exists in the profile. "));
+
 PassManagerBuilder::PassManagerBuilder() {
     OptLevel = 2;
     SizeLevel = 0;
     LibraryInfo = nullptr;
     Inliner = nullptr;
     DisableUnrollLoops = false;
     SLPVectorize = RunSLPVectorization;
     LoopVectorize = RunLoopVectorization;
     RerollLoops = RunLoopRerolling;
     NewGVN = RunNewGVN;
     DisableGVNLoadPRE = false;
     VerifyInput = false;
     VerifyOutput = false;
     MergeFunctions = false;
     PrepareForLTO = false;
     EnablePGOInstrGen = false;
     PGOInstrGen = "";
     PGOInstrUse = "";
     PGOSampleUse = "";
     PrepareForThinLTO = EnablePrepareForThinLTO;
-    PerformThinLTO = false;
+    PerformThinLTO = EnablePerformThinLTO;
     DivergentTarget = false;
 }
 
 PassManagerBuilder::~PassManagerBuilder() {
   delete LibraryInfo;
   delete Inliner;
 }
 
if (EnableCHR && OptLevel >= 3 &&
       (!PGOInstrUse.empty() || !PGOSampleUse.empty()))
     MPM.add(createControlHeightReductionLegacyPass());
 }
 
 void PassManagerBuilder::populateModulePassManager(
     legacy::PassManagerBase &MPM) {
   if (!PGOSampleUse.empty()) {
     MPM.add(createPruneEHPass());
+    // In ThinLTO mode, when flattened profile is used, all the available
+    // profile information will be annotated in PreLink phase so there is
+    // no need to load the profile again in PostLink.
+    if (!(FlattenedProfileUsed && PerformThinLTO))
       MPM.add(createSampleProfileLoaderPass(PGOSampleUse));
   }
 
   // Allow forcing function attributes as a debugging and tuning aid.
   MPM.add(createForceFunctionAttrsLegacyPass());
 
   // If all optimizations are disabled, just run the always-inline pass and,
   // if enabled, the function merging pass.
   if (OptLevel == 0) {

test/Transforms/SampleProfile/Inputs/flattened.prof

+foo:100:100
+ 1: 100

test/Transforms/SampleProfile/flattened.ll

+; Check flattened profile will not be read in thinlto postlink.
+; RUN: opt < %s -O2 -flattened-profile-used -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/flattened.prof -enable-chr=false -perform-thinlto=true -S | FileCheck %s
+; RUN: opt < %s -passes='thinlto<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/flattened.prof -flattened-profile-used -S | FileCheck %s
+;
+; Check flattened profile will be read in thinlto prelink.
+; RUN: opt < %s -O2 -flattened-profile-used -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/flattened.prof -enable-chr=false -prepare-for-thinlto=true -S | FileCheck %s --check-prefix=PRELINK
+; RUN: opt < %s -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/flattened.prof -flattened-profile-used -S | FileCheck %s --check-prefix=PRELINK
+;
+; Check flattened profile will be read in non-thinlto mode.
+; RUN: opt < %s -O2 -flattened-profile-used -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/flattened.prof -enable-chr=false -S | FileCheck %s --check-prefix=NOTHINLTO
+; RUN: opt < %s -passes='default<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/flattened.prof -flattened-profile-used -S | FileCheck %s --check-prefix=NOTHINLTO
+;
+; CHECK-NOT: !{!"ProfileFormat", !"SampleProfile"}
+; PRELINK:   !{!"ProfileFormat", !"SampleProfile"}
+; NOTHINLTO: !{!"ProfileFormat", !"SampleProfile"}
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: norecurse nounwind readnone uwtable
+define dso_local i32 @foo() local_unnamed_addr !dbg !7 {
+entry:
+  ret i32 -1, !dbg !9
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 8.0.0 (trunk 345241)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "a.c", directory: "")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 8.0.0 (trunk 345241)"}
+!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, retainedNodes: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 2, column: 3, scope: !7)