[SampleProfile][Inline] Annotate sample profile inline remarks with link phase (prelink/postlink) information.

[mingmingl]

Summary

The annotation is gated by a bool option, and off by default.

[mingmingl]

This is a strict subset of https://reviews.llvm.org/D125495, to make that patch simpler.

[mingmingl]

• Fix a bug in InlineAdvisor::getAnnotatedInlinePassName (from https://reviews.llvm.org/D126824). Basically, to compute string only once, make it a constant member of class, and return its c_str() ever since.

• Let SampleProfileLoader class call llvm::AnnotateInlinePassName to get an annotated name.

[mingmingl]

Revert unnecessary change in llvm/include/llvm/Transforms/IPO/SampleProfile.h

[kazu]

The patch looks good in general, but I have some nit-picky suggestions.

llvm/include/llvm/Analysis/InlineAdvisor.h

211

Capitalize the variable name like AnnotatedInlinePassName.

llvm/lib/Analysis/InlineAdvisor.cpp

576–577

You should be able to remove this if statement. When IC.hasValue() is false, the constructor sets annotatedInlinePassName to DEBUG_TYPE.

Once the function has been simplified down to:

const char *InlineAdvisor::getAnnotatedInlinePassName() {
  return annotatedInlinePassName.c_str();
}

you might as well move it to InlineAdvisor.h.

llvm/lib/Transforms/IPO/SampleProfile.cpp

503

Capitalize the variable name like AnnotatedPassName.

545

Remove inline unless you absolutely need it.

598

Likewise, remove inline unless you absolutely need it.

599–602

Could you fully compute annotatedPassName in the constructor as you do in InlineAdvisor.cpp? I'm suggesting this partly for consistency with InlineAdvisor.cpp and partly for readability.

Again, you might consider moving the function to the class declaration.

[mingmingl]

Resolve comments, and add Differential Revision: <URL> in commit message.

[mingmingl]

thanks for reviews! PTAL.

llvm/lib/Analysis/InlineAdvisor.cpp

576–577

Done by 1) simplifying InlineAdvisor::getAnnotatedInlinePassName 2) move it to InlineAdvisor.h.

[kazu]

LGTM with a minor change to the constructor of InlineAdvisor. Thank you for your patience.

llvm/lib/Analysis/InlineAdvisor.cpp

512

Replace IC.hasValue() with TC.

513

Replace IC.getValue() with *IC.

[mingmingl]

Address comments, by using overloaded operator rather than hasValue() / getValue(), since the former is closer to std::optional in terms of style.

llvm/include/llvm/Analysis/InlineAdvisor.h

public:
   virtual void onPassExit(LazyCallGraph::SCC *SCC = nullptr) {}
 
   /// Support for printer pass
   virtual void print(raw_ostream &OS) const {
     OS << "Unimplemented InlineAdvisor print\n";
   }
 
   /// NOTE pass name is annotated only when inline advisor constructor provides InlineContext.
-  const char *getAnnotatedInlinePassName();
+  const char *getAnnotatedInlinePassName() const {
+    return AnnotatedInlinePassName.c_str();
+  }
 
 protected:
   InlineAdvisor(Module &M, FunctionAnalysisManager &FAM,
                 Optional<InlineContext> IC = NoneType::None);
   virtual std::unique_ptr<InlineAdvice> getAdviceImpl(CallBase &CB) = 0;
   virtual std::unique_ptr<InlineAdvice> getMandatoryAdvice(CallBase &CB,
                                                            bool Advice);
 
   Module &M;
   FunctionAnalysisManager &FAM;
   const Optional<InlineContext> IC;
+  const std::string AnnotatedInlinePassName;
   std::unique_ptr<ImportedFunctionsInliningStatistics> ImportedFunctionsStats;
 
   enum class MandatoryInliningKind { NotMandatory, Always, Never };
 
   static MandatoryInliningKind getMandatoryKind(CallBase &CB,
                                                 FunctionAnalysisManager &FAM,
                                                 OptimizationRemarkEmitter &ORE);
 

llvm/include/llvm/Transforms/IPO/SampleProfile.h

SampleProfileLoaderPass(
       : ProfileFileName(File), ProfileRemappingFileName(RemappingFile),
         LTOPhase(LTOPhase) {}
 
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
 private:
   std::string ProfileFileName;
   std::string ProfileRemappingFileName;
-  ThinOrFullLTOPhase LTOPhase;
+  const ThinOrFullLTOPhase LTOPhase;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_IPO_SAMPLEPROFILE_H

llvm/lib/Analysis/InlineAdvisor.cpp

llvm::emitInlinedInto(
           Remark << " to match profiling context";
         Remark << " with " << IC;
       },
       PassName);
 }
 
 InlineAdvisor::InlineAdvisor(Module &M, FunctionAnalysisManager &FAM,
                              Optional<InlineContext> IC)
-    : M(M), FAM(FAM), IC(IC) {
+    : M(M), FAM(FAM), IC(IC),
+      AnnotatedInlinePassName(IC ? llvm::AnnotateInlinePassName(*IC)
+                                 : DEBUG_TYPE) {
   if (InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No) {
     ImportedFunctionsStats =
         std::make_unique<ImportedFunctionsInliningStatistics>();
     ImportedFunctionsStats->setModuleInfo(M);
   }
 }
 
 InlineAdvisor::~InlineAdvisor() {
static inline const char *getInlineAdvisorContext(InlinePass IP) {
 
   llvm_unreachable("unreachable");
 }
 
 std::string llvm::AnnotateInlinePassName(InlineContext IC) {
   return std::string(getLTOPhase(IC.LTOPhase)) + "-" +
          std::string(getInlineAdvisorContext(IC.Pass));
 }
 
-const char *InlineAdvisor::getAnnotatedInlinePassName() {
-  if (!IC)
-    return DEBUG_TYPE;
-
-  // IC is constant and initialized in constructor, so compute the annotated
-  // name only once.
-  static const std::string PassName = llvm::AnnotateInlinePassName(*IC);
-
-  return PassName.c_str();
-}
-
 InlineAdvisor::MandatoryInliningKind
 InlineAdvisor::getMandatoryKind(CallBase &CB, FunctionAnalysisManager &FAM,
                                 OptimizationRemarkEmitter &ORE) {
   auto &Callee = *CB.getCalledFunction();
 
   auto GetTLI = [&](Function &F) -> const TargetLibraryInfo & {
     return FAM.getResult<TargetLibraryAnalysis>(F);
   };
 

llvm/lib/Transforms/IPO/SampleProfile.cpp

static cl::opt<unsigned>
     MaxNumPromotions("sample-profile-icp-max-prom", cl::init(3), cl::Hidden,
                      cl::desc("Max number of promotions for a single indirect "
                               "call callsite in sample profile loader"));
 
 static cl::opt<bool> OverwriteExistingWeights(
     "overwrite-existing-weights", cl::Hidden, cl::init(false),
     cl::desc("Ignore existing branch weights on IR and always overwrite."));
 
+static cl::opt<bool> AnnotateSampleProfileInlinePhase(
+    "annotate-sample-profile-inline-phase", cl::Hidden, cl::init(false),
+    cl::desc("Annotate LTO phase (prelink / postlink), or main (no LTO) for "
+             "sample-profile inline pass name."));
+
 extern cl::opt<bool> EnableExtTspBlockPlacement;
 
 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>;
public:
   SampleProfileLoader(
       StringRef Name, StringRef RemapName, ThinOrFullLTOPhase LTOPhase,
       std::function<AssumptionCache &(Function &)> GetAssumptionCache,
       std::function<TargetTransformInfo &(Function &)> GetTargetTransformInfo,
       std::function<const TargetLibraryInfo &(Function &)> GetTLI)
       : SampleProfileLoaderBaseImpl(std::string(Name), std::string(RemapName)),
         GetAC(std::move(GetAssumptionCache)),
         GetTTI(std::move(GetTargetTransformInfo)), GetTLI(std::move(GetTLI)),
-        LTOPhase(LTOPhase) {}
+        LTOPhase(LTOPhase),
+        AnnotatedPassName(AnnotateSampleProfileInlinePhase
+                              ? llvm::AnnotateInlinePassName(InlineContext{
+                                    LTOPhase, InlinePass::SampleProfileInliner})
+                              : CSINLINE_DEBUG) {}
 
   bool doInitialization(Module &M, FunctionAnalysisManager *FAM = nullptr);
   bool runOnModule(Module &M, ModuleAnalysisManager *AM,
                    ProfileSummaryInfo *_PSI, CallGraph *CG);
 
 protected:
   bool runOnFunction(Function &F, ModuleAnalysisManager *AM);
   bool emitAnnotations(Function &F);
protected:
   /// Profile tracker for different context.
   std::unique_ptr<SampleContextTracker> ContextTracker;
 
   /// Flag indicating which LTO/ThinLTO phase the pass is invoked in.
   ///
   /// We need to know the LTO phase because for example in ThinLTOPrelink
   /// phase, in annotation, we should not promote indirect calls. Instead,
   /// we will mark GUIDs that needs to be annotated to the function.
-  ThinOrFullLTOPhase LTOPhase;
+  const ThinOrFullLTOPhase LTOPhase;
+  const std::string AnnotatedPassName;
 
   /// Profle Symbol list tells whether a function name appears in the binary
   /// used to generate the current profile.
   std::unique_ptr<ProfileSymbolList> PSL;
 
   /// Total number of samples collected in this profile.
   ///
   /// This is the sum of all the samples collected in all the functions executed
protected:
   // attribute.
   bool ProfAccForSymsInList;
 
   // External inline advisor used to replay inline decision from remarks.
   std::unique_ptr<InlineAdvisor> ExternalInlineAdvisor;
 
   // A pseudo probe helper to correlate the imported sample counts.
   std::unique_ptr<PseudoProbeManager> ProbeManager;
+
+private:
+  const char *getAnnotatedRemarkPassName() const {
+    return AnnotatedPassName.c_str();
+  }
 };
 
 class SampleProfileLoaderLegacyPass : public ModulePass {
 public:
   // Class identification, replacement for typeinfo
   static char ID;
 
   SampleProfileLoaderLegacyPass(
private:
   SampleProfileLoader SampleLoader;
   AssumptionCacheTracker *ACT = nullptr;
   TargetTransformInfoWrapperPass *TTIWP = nullptr;
   TargetLibraryInfoWrapperPass *TLIWP = nullptr;
 };
 
 } // end anonymous namespace
 
 ErrorOr<uint64_t> SampleProfileLoader::getInstWeight(const Instruction &Inst) {
   if (FunctionSamples::ProfileIsProbeBased)
     return getProbeWeight(Inst);
 
   const DebugLoc &DLoc = Inst.getDebugLoc();
   if (!DLoc)
     return std::error_code();
 
   // Ignore all intrinsics, phinodes and branch instructions.
   // Branch and phinodes instruction usually contains debug info from sources
   // outside of the residing basic block, thus we ignore them during annotation.
   if (isa<BranchInst>(Inst) || isa<IntrinsicInst>(Inst) || isa<PHINode>(Inst))
     return std::error_code();

 }
 
 void SampleProfileLoader::emitOptimizationRemarksForInlineCandidates(
     const SmallVectorImpl<CallBase *> &Candidates, const Function &F,
     bool Hot) {
   for (auto I : Candidates) {
     Function *CalledFunction = I->getCalledFunction();
     if (CalledFunction) {
-      ORE->emit(OptimizationRemarkAnalysis(CSINLINE_DEBUG, "InlineAttempt",
-                                           I->getDebugLoc(), I->getParent())
+      ORE->emit(OptimizationRemarkAnalysis(getAnnotatedRemarkPassName(),
+                                           "InlineAttempt", I->getDebugLoc(),
+                                           I->getParent())
                 << "previous inlining reattempted for "
                 << (Hot ? "hotness: '" : "size: '")
                 << ore::NV("Callee", CalledFunction) << "' into '"
                 << ore::NV("Caller", &F) << "'");
     }
   }
 }
 
bool SampleProfileLoader::tryInlineCandidate(
   CallBase &CB = *Candidate.CallInstr;
   Function *CalledFunction = CB.getCalledFunction();
   assert(CalledFunction && "Expect a callee with definition");
   DebugLoc DLoc = CB.getDebugLoc();
   BasicBlock *BB = CB.getParent();
 
   InlineCost Cost = shouldInlineCandidate(Candidate);
   if (Cost.isNever()) {
-    ORE->emit(OptimizationRemarkAnalysis(CSINLINE_DEBUG, "InlineFail", DLoc, BB)
+    ORE->emit(OptimizationRemarkAnalysis(getAnnotatedRemarkPassName(),
+                                         "InlineFail", DLoc, BB)
               << "incompatible inlining");
     return false;
   }
 
   if (!Cost)
     return false;
 
   InlineFunctionInfo IFI(nullptr, GetAC);
   IFI.UpdateProfile = false;
   if (!InlineFunction(CB, IFI).isSuccess())
     return false;
 
   // Merge the attributes based on the inlining.
   AttributeFuncs::mergeAttributesForInlining(*BB->getParent(),
                                              *CalledFunction);
 
   // The call to InlineFunction erases I, so we can't pass it here.
-  emitInlinedIntoBasedOnCost(*ORE, DLoc, BB, *CalledFunction,
-                             *BB->getParent(), Cost, true, CSINLINE_DEBUG);
+  emitInlinedIntoBasedOnCost(*ORE, DLoc, BB, *CalledFunction, *BB->getParent(),
+                             Cost, true, getAnnotatedRemarkPassName());
 
   // Now populate the list of newly exposed call sites.
   if (InlinedCallSites) {
     InlinedCallSites->clear();
     for (auto &I : IFI.InlinedCallSites)
       InlinedCallSites->push_back(I);
   }
 
void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
     const Function &F) {
   // Accumulate not inlined callsite information into notInlinedSamples
   for (const auto &Pair : NonInlinedCallSites) {
     CallBase *I = Pair.getFirst();
     Function *Callee = I->getCalledFunction();
     if (!Callee || Callee->isDeclaration())
       continue;
 
-    ORE->emit(OptimizationRemarkAnalysis(CSINLINE_DEBUG, "NotInline",
-                                         I->getDebugLoc(), I->getParent())
-              << "previous inlining not repeated: '"
-              << ore::NV("Callee", Callee) << "' into '"
-              << ore::NV("Caller", &F) << "'");
+    ORE->emit(
+        OptimizationRemarkAnalysis(getAnnotatedRemarkPassName(), "NotInline",
+        << "previous inlining not repeated: '" << ore::NV("Callee", Callee)
+        << "' into '" << ore::NV("Caller", &F) << "'");
 
     ++NumCSNotInlined;
     const FunctionSamples *FS = Pair.getSecond();
     if (FS->getTotalSamples() == 0 && FS->getEntrySamples() == 0) {
       continue;
     }
 
     // Do not merge a context that is already duplicated into the base profile.

llvm/test/Transforms/SampleProfile/pseudo-probe-inline.ll

-; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-inline.prof -S -pass-remarks=sample-profile -sample-profile-prioritized-inline=0 -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
-; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml
+; RUN: opt < %s -passes=pseudo-probe,sample-profile  -sample-profile-file=%S/Inputs/pseudo-probe-inline.prof -S -pass-remarks=sample-profile -sample-profile-prioritized-inline=0 -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-NO-ANNOTATE < %t.opt.yaml
 
 ; RUN: llvm-profdata merge --sample --extbinary %S/Inputs/pseudo-probe-inline.prof -o %t2
-; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%t2 -S -pass-remarks=sample-profile -sample-profile-prioritized-inline=0 -pass-remarks-output=%t2.opt.yaml 2>&1 | FileCheck %s
-; RUN: FileCheck %s -check-prefix=YAML < %t2.opt.yaml
+; RUN: opt < %s -passes=pseudo-probe,sample-profile  -sample-profile-file=%t2 -S -pass-remarks=sample-profile -sample-profile-prioritized-inline=0 -pass-remarks-output=%t2.opt.yaml 2>&1 | FileCheck %s
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-NO-ANNOTATE < %t2.opt.yaml
+
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -annotate-sample-profile-inline-phase=true -sample-profile-file=%S/Inputs/pseudo-probe-inline.prof -S -pass-remarks=sample-profile -sample-profile-prioritized-inline=0 -pass-remarks-output=%t3.opt.yaml 2>&1 | FileCheck %s
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-ANNOTATE < %t3.opt.yaml
+
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -annotate-sample-profile-inline-phase=true -sample-profile-file=%t2 -S -pass-remarks=sample-profile -sample-profile-prioritized-inline=0 -pass-remarks-output=%t4.opt.yaml 2>&1 | FileCheck %s
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-ANNOTATE < %t4.opt.yaml
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @factor = dso_local global i32 3, align 4
 
 define dso_local i32 @foo(i32 %x) #0 !dbg !12 {
 entry:

 ; CHECK: !llvm.pseudo_probe_desc = !{![[#DESC0:]], ![[#DESC1:]]}
 ; CHECK: ![[#DESC0]] = !{i64 [[#GUID1]], i64 [[#HASH1:]], !"foo"}
 ; CHECK: ![[#DESC1]] = !{i64 [[#GUID2]], i64 [[#HASH2:]], !"zen"}
 ; CHECK: ![[PD1]] = !{!"branch_weights", i32 5, i32 0}
 ; CHECK: ![[PD2]] = !{!"branch_weights", i32 382915, i32 5}
 
 ; Checking to see if YAML file is generated and contains remarks
 ;YAML: --- !Passed
-;YAML-NEXT:  Pass:            sample-profile-inline
+;YAML-NO-ANNOTATE-NEXT:  Pass:            sample-profile-inline
+;YAML-ANNOTATE-NEXT: Pass:    main-sample-profile-inline
 ;YAML-NEXT:  Name:            Inlined
 ;YAML-NEXT:  DebugLoc:        { File: test.cpp, Line: 10, Column: 11 }
 ;YAML-NEXT:  Function:        foo
 ;YAML-NEXT:  Args:
 ;YAML-NEXT:    - String:          ''''
 ;YAML-NEXT:    - Callee:          zen
 ;YAML-NEXT:      DebugLoc:        { File: test.cpp, Line: 38, Column: 0 }
 ;YAML-NEXT:    - String:          ''' inlined into '''

llvm/test/Transforms/SampleProfile/remarks.ll

 ; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
-; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-NO-ANNOTATE < %t.opt.yaml
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
-; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-NO-ANNOTATE < %t.opt.yaml
+
+; RUN: opt < %s -sample-profile -annotate-sample-profile-inline-phase -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-ANNOTATE < %t.opt.yaml
+; RUN: opt < %s -passes=sample-profile -annotate-sample-profile-inline-phase -sample-profile-file=%S/Inputs/remarks.prof -S -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml 2>&1 | FileCheck %s
+; RUN: FileCheck %s -check-prefixes=YAML,YAML-ANNOTATE < %t.opt.yaml
 
 ; Original test case.
 ;
 ;     1    #include <stdlib.h>
 ;     2
 ;     3    long long foo() {
 ;     4      long long int sum = 0;
 ;     5      for (int i = 0; i < 500000000; i++)

 ; The back edge for the loop is the hottest edge in the loop subgraph.
 ; CHECK: remark: remarks.cc:6:9: most popular destination for conditional branches at remarks.cc:5:3
 
 ; The predicate almost always chooses the 'else' branch.
 ; CHECK: remark: remarks.cc:9:15: most popular destination for conditional branches at remarks.cc:6:9
 
 ; Checking to see if YAML file is generated and contains remarks
 ;YAML:       --- !Passed
-;YAML-NEXT:  Pass:            sample-profile-inline
+;YAML-NO-ANNOTATE-NEXT:  Pass:            sample-profile-inline
+;YAML-ANNOTATE-NEXT:  Pass:            main-sample-profile-inline
 ;YAML-NEXT:  Name:            Inlined
 ;YAML-NEXT:  DebugLoc:        { File: remarks.cc, Line: 13, Column: 21 }
 ;YAML-NEXT:  Function:        main
 ;YAML-NEXT:  Args:
 ;YAML-NEXT:    - String:          ''''
 ;YAML-NEXT:    - Callee:          _Z3foov
 ;YAML-NEXT:      DebugLoc:        { File: remarks.cc, Line: 3, Column: 0 }
 ;YAML-NEXT:    - String:          ''' inlined into '

 ;YAML-NEXT:    - String:          main
 ;YAML-NEXT:    - String:          ':'
 ;YAML-NEXT:    - Line:            '0'
 ;YAML-NEXT:    - String:          ':'
 ;YAML-NEXT:    - Column:          '21'
 ;YAML-NEXT:    - String:          ';'
 ;YAML-NEXT:  ...
 ;YAML:       --- !Passed
-;YAML-NEXT:  Pass:            sample-profile-inline
+;YAML-NO-ANNOTATE-NEXT:  Pass:            sample-profile-inline
+;YAML-ANNOTATE-NEXT:  Pass:            main-sample-profile-inline
 ;YAML-NEXT:  Name:            AlwaysInline
 ;YAML-NEXT:  DebugLoc:        { File: remarks.cc, Line: 9, Column: 19 }
 ;YAML-NEXT:  Function:        main
 ;YAML-NEXT:  Args:
 ;YAML-NEXT:    - String:          ''''
 ;YAML-NEXT:    - Callee:          rand
 ;YAML-NEXT:      DebugLoc:        { File: remarks.cc, Line: 90, Column: 0 }
 ;YAML-NEXT:    - String:          ''' inlined into '''