[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

612

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

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

310

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

612

Done.

lib/Transforms/IPO/SampleProfile.cpp

126

Fixed.

[wmi]

Address Teresa's comments.

[tejohnson]

lib/Passes/PassBuilder.cpp

613

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

438 ↗

(On Diff #181414)

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

438 ↗

(On Diff #181414)

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

include/llvm/Transforms/IPO/SampleProfile.h

 namespace llvm {
 
 class Module;
 
 /// The sample profiler data loader pass.
 class SampleProfileLoaderPass : public PassInfoMixin<SampleProfileLoaderPass> {
 public:
   SampleProfileLoaderPass(std::string File = "", std::string RemappingFile = "",
-                          bool IsThinLTOPreLink = false)
+                          bool IsThinLTOPreLink = false,
+                          bool IsThinLTOPostLink = false)
       : ProfileFileName(File), ProfileRemappingFileName(RemappingFile),
-        IsThinLTOPreLink(IsThinLTOPreLink) {}
+        IsThinLTOPreLink(IsThinLTOPreLink),
+        IsThinLTOPostLink(IsThinLTOPostLink) {}
 
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
 private:
   std::string ProfileFileName;
   std::string ProfileRemappingFileName;
   bool IsThinLTOPreLink;
+  bool IsThinLTOPostLink;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_SAMPLEPROFILE_H

lib/Passes/PassBuilder.cpp

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.
-    MPM.addPass(SampleProfileLoaderPass(PGOOpt->SampleProfileFile,
-                                        PGOOpt->ProfileRemappingFile,
-                                        Phase == ThinLTOPhase::PreLink));
+    MPM.addPass(SampleProfileLoaderPass(
+        PGOOpt->SampleProfileFile, PGOOpt->ProfileRemappingFile,
+        Phase == ThinLTOPhase::PreLink, Phase == ThinLTOPhase::PostLink));
     // 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,
                                            true));

lib/Transforms/IPO/SampleProfile.cpp

static cl::opt<unsigned> SampleProfileSampleCoverage(
     cl::desc("Emit a warning if less than N% of samples in the input profile "
              "are matched to the IR."));
 
 static cl::opt<bool> NoWarnSampleUnused(
     "no-warn-sample-unused", cl::init(false), cl::Hidden,
     cl::desc("Use this option to turn off/on warnings about function with "
              "samples but without debug information to use those samples. "));
 
+static cl::opt<bool> FlattenProfileUsed(
+    "flatten-profile-used", cl::init(false), cl::Hidden,
+    cl::desc("Indicate the sample profile being used is flatten, i.e., "
+             "no inline hierachy exists in the profile. "));
+
 namespace {
 
 using BlockWeightMap = DenseMap<const BasicBlock *, uint64_t>;
 using EquivalenceClassMap = DenseMap<const BasicBlock *, const BasicBlock *>;
 using Edge = std::pair<const BasicBlock *, const BasicBlock *>;
 using EdgeWeightMap = DenseMap<Edge, uint64_t>;
 using BlockEdgeMap =
     DenseMap<const BasicBlock *, SmallVector<const BasicBlock *, 8>>;

 ///
 /// This pass reads profile data from the file specified by
 /// -sample-profile-file and annotates every affected function with the
 /// profile information found in that file.
 class SampleProfileLoader {
 public:
   SampleProfileLoader(
       StringRef Name, StringRef RemapName, bool IsThinLTOPreLink,
+      bool IsThinLTOPostLink,
       std::function<AssumptionCache &(Function &)> GetAssumptionCache,
       std::function<TargetTransformInfo &(Function &)> GetTargetTransformInfo)
       : GetAC(std::move(GetAssumptionCache)),
         GetTTI(std::move(GetTargetTransformInfo)), Filename(Name),
-        RemappingFilename(RemapName), IsThinLTOPreLink(IsThinLTOPreLink) {}
+        RemappingFilename(RemapName), IsThinLTOPreLink(IsThinLTOPreLink),
+        IsThinLTOPostLink(IsThinLTOPostLink) {}
 
   bool doInitialization(Module &M);
   bool runOnModule(Module &M, ModuleAnalysisManager *AM,
                    ProfileSummaryInfo *_PSI);
 
   void dump() { Reader->dump(); }
 
 protected:
protected:
   /// Flag indicating whether the profile input loaded successfully.
   bool ProfileIsValid = false;
 
   /// Flag indicating if the pass is invoked in ThinLTO compile phase.
   ///
   /// In this phase, in annotation, we should not promote indirect calls.
   /// Instead, we will mark GUIDs that needs to be annotated to the function.
   bool IsThinLTOPreLink;
+  /// Flag indicating if the pass is invoked in ThinLTO backend compile phase.
+  bool IsThinLTOPostLink;
 
   /// Profile Summary Info computed from sample profile.
   ProfileSummaryInfo *PSI = nullptr;
 
   /// Total number of samples collected in this profile.
   ///
   /// This is the sum of all the samples collected in all the functions executed
   /// at runtime.
   uint64_t TotalCollectedSamples = 0;
 
   /// Optimization Remark Emitter used to emit diagnostic remarks.
   OptimizationRemarkEmitter *ORE = nullptr;
 };
 
 class SampleProfileLoaderLegacyPass : public ModulePass {
 public:
   // Class identification, replacement for typeinfo
   static char ID;
 
   SampleProfileLoaderLegacyPass(StringRef Name = SampleProfileFile,
-                                bool IsThinLTOPreLink = false)
-      : ModulePass(ID),
-        SampleLoader(Name, SampleProfileRemappingFile, IsThinLTOPreLink,
+                                bool IsThinLTOPreLink = false,
+                                bool IsThinLTOPostLink = false)
+      : ModulePass(ID), SampleLoader(
+                            Name, SampleProfileRemappingFile, IsThinLTOPreLink,
+                            IsThinLTOPostLink,
                             [&](Function &F) -> AssumptionCache & {
                               return ACT->getAssumptionCache(F);
                             },
                             [&](Function &F) -> TargetTransformInfo & {
                               return TTIWP->getTTI(F);
                             }) {
     initializeSampleProfileLoaderLegacyPassPass(
         *PassRegistry::getPassRegistry());
   }
 
   void dump() { SampleLoader.dump(); }
 
   bool doInitialization(Module &M) override {
     return SampleLoader.doInitialization(M);
INITIALIZE_PASS_BEGIN(SampleProfileLoaderLegacyPass, "sample-profile",
                       "Sample Profile loader", false, false)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
 INITIALIZE_PASS_END(SampleProfileLoaderLegacyPass, "sample-profile",
                     "Sample Profile loader", false, false)
 
 bool SampleProfileLoader::doInitialization(Module &M) {
+  // In ThinLTO mode, when flatten profile is used, all the available
+  // profile information will be annotated in Prelink phase so there is
+  // no need to read the profile again.
+  if (IsThinLTOPostLink && FlattenProfileUsed)
+    return false;
+
+  LLVM_DEBUG(dbgs() << "SampleProfileLoader initialize for Module: " << M.getName() << "\n");
+
   auto &Ctx = M.getContext();
   auto ReaderOrErr = SampleProfileReader::create(Filename, Ctx);
   if (std::error_code EC = ReaderOrErr.getError()) {
     std::string Msg = "Could not open profile: " + EC.message();
     Ctx.diagnose(DiagnosticInfoSampleProfile(Filename, Msg));
     return false;
   }
   Reader = std::move(ReaderOrErr.get());
PreservedAnalyses SampleProfileLoaderPass::run(Module &M,
   auto GetTTI = [&](Function &F) -> TargetTransformInfo & {
     return FAM.getResult<TargetIRAnalysis>(F);
   };
 
   SampleProfileLoader SampleLoader(
       ProfileFileName.empty() ? SampleProfileFile : ProfileFileName,
       ProfileRemappingFileName.empty() ? SampleProfileRemappingFile
                                        : ProfileRemappingFileName,
-      IsThinLTOPreLink, GetAssumptionCache, GetTTI);
+      IsThinLTOPreLink, IsThinLTOPostLink, GetAssumptionCache, GetTTI);
 
   SampleLoader.doInitialization(M);
 
   ProfileSummaryInfo *PSI = &AM.getResult<ProfileSummaryAnalysis>(M);
   if (!SampleLoader.runOnModule(M, &AM, PSI))
     return PreservedAnalyses::all();
 
   return PreservedAnalyses::none();
 }

test/Transforms/SampleProfile/Inputs/flatten.prof

+foo:100:100
+ 1: 100

test/Transforms/SampleProfile/flatten.ll

+; REQUIRES: asserts
+; RUN: opt < %s -passes='thinlto<O2>' -pgo-kind=new-pm-pgo-sample-use-pipeline -profile-file=%S/Inputs/flatten.prof -debug-only=sample-profile -flatten-profile-used -S 2>&1| FileCheck %s
+;
+; Check SampleProfileLoader doesn't read the profile in ThinLTO postlink stage
+; when flatten sample profile is used.
+; CHECK-NOT: SampleProfileLoader initialize for Module:
+
+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)