[CSSPGO] Skip reporting staleness metrics for imported functions

[wlei]

Summary

Accumulating the staleness metrics from per-link is less accurate than doing it from post-link time(assuming we use the offline profile mismatch as baseline), the reason is that there are some duplicated reports for the same functions, for example, one template function could be included in multiple TUs, but in post thin link time, only one function are kept(linkonce_odr) and others are marked as available-externally function. Hence, this change skips reporting the metrics for imported functions(available-externally).

I saw the post-link number is now very close to the offline number(dump the mismatched functions and count the metrics offline based on the entire profile), sightly smaller than offline number due to some missing inlined functions.

[wlei]

retire flatten-profile-for-matching switch, always use flattened profile for matching.

[wlei]

Updating D156725: [CSSPGO] Improve profile staleness report for post-link time

[wenlei]

For the current sample loader, if the root or parent function profile are lost or mismatched, all its profile and its children profile are dropped, we should use the non-flattened profile(the original nested profile) to count the total mismatched samples.

How significantly does this affect staleness metric?

Maybe what we should actually do is to reuse the nested profile from those mismatched parent functions, something like run promoteMergeNotInlinedContextSamples on top level functions samples not used? We do reuse nested profile for not inlined callees from parent function.

llvm/lib/Transforms/IPO/SampleProfile.cpp

2109

Maybe this function needs a refactoring - break it into two functions: 1) findAnchors that only uses IR, 2) matchCallsites that also uses profile in addition to anchors. We call always call #1, but only call #2 when profile is available and reporting is enabled.

Also if MatchedCallsiteLocs is only used for reporting, but not matching, can we move #2 into countProfileMismatches? Currently the different parts seem a bit inter-wined.. would be good have some separation.

Maybe #2 can also be a standalone functions that covers both call site matching for report and for actual matching (like populateProfileCallsites on flattened profile)

2383

nit: Skip to report -> Skip reporting, the imported functions -> imported functions

2390

remove?

2403

This function is growing bigger. Can we move line 2384-2399 and line 2405-2414 into countProfileMismatches? These are all related to mismatch statistics.

2421

nit: instead of FSForReporting vs FSForMatching, just say FS and FSFlattened instead? it applies to other instances too.

[wlei]

Updating D156725: [CSSPGO] Improve profile staleness report for post-link time

[wlei]

rebase

[wlei]

Split the original patch into four patches, https://reviews.llvm.org/D156725, https://reviews.llvm.org/D158891, https://reviews.llvm.org/D158900, https://reviews.llvm.org/D158817
Or you can simply find them in the patch stack.

[wenlei]

The stack with refactoring first makes the changes easier to review. Thanks!

LGTM

[wlei]

rebase

[wlei]

rebase

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

2383–2401

As discussed in the other patch https://reviews.llvm.org/D158891, skipping external functions may cause its nested non-external callee samples to be skipped too. This is not what we want?

2449

nit: remove the colon

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

2383–2401

As using nested profile to count mismatches is by-design, the problem here is not quite relevant to the other patch and the external function check here.

A better way to frame the question is, when and where will the nested callee samples be counted if they are not inlined but its checksum is mismatched? In this case, the callee samples can be returned during inlining, but it should be counted when processing the callee.

The problem seems not easy to solve as the matcher doesn't know if a callsite will be inlined or not, and due to a hack in our system inlined samples always skip the checksum check.

llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h

if (NamedMDNode *FuncInfo =
                         ->getZExtValue();
         auto Hash = mdconst::dyn_extract<ConstantInt>(MD->getOperand(1))
                         ->getZExtValue();
         GUIDToProbeDescMap.try_emplace(GUID, PseudoProbeDescriptor(GUID, Hash));
       }
     }
   }
 
-  const PseudoProbeDescriptor *getDesc(const Function &F) const {
-    auto I = GUIDToProbeDescMap.find(
-        Function::getGUID(FunctionSamples::getCanonicalFnName(F)));
+  const PseudoProbeDescriptor *getDesc(uint64_t GUID) const {
+    auto I = GUIDToProbeDescMap.find(GUID);
     return I == GUIDToProbeDescMap.end() ? nullptr : &I->second;
   }
 
+  const PseudoProbeDescriptor *getDesc(StringRef FProfileName) const {
+    return getDesc(Function::getGUID(FProfileName));
+  }
+
+  const PseudoProbeDescriptor *getDesc(const Function &F) const {
+    return getDesc(Function::getGUID(FunctionSamples::getCanonicalFnName(F)));
+  }
+
   bool moduleIsProbed(const Module &M) const {
     return M.getNamedMetadata(PseudoProbeDescMetadataName);
   }
 
   bool profileIsHashMismatched(const PseudoProbeDescriptor &FuncDesc,
                                const FunctionSamples &Samples) const {
     return FuncDesc.getFunctionHash() != Samples.getFunctionHash();
   }

llvm/lib/Transforms/IPO/SampleProfile.cpp

static cl::opt<bool> ReportProfileStaleness(
     "report-profile-staleness", cl::Hidden, cl::init(false),
     cl::desc("Compute and report stale profile statistical metrics."));
 
 static cl::opt<bool> PersistProfileStaleness(
     "persist-profile-staleness", cl::Hidden, cl::init(false),
     cl::desc("Compute stale profile statistical metrics and write it into the "
              "native object file(.llvm_stats section)."));
 
-static cl::opt<bool> FlattenProfileForMatching(
-    "flatten-profile-for-matching", cl::Hidden, cl::init(true),
-    cl::desc(
-        "Use flattened profile for stale profile detection and matching."));
-
 static cl::opt<bool> ProfileSampleAccurate(
     "profile-sample-accurate", cl::Hidden, cl::init(false),
     cl::desc("If the sample profile is accurate, we will mark all un-sampled "
              "callsite and function as having 0 samples. Otherwise, treat "
              "un-sampled callsites and functions conservatively as unknown. "));
 
 static cl::opt<bool> ProfileSampleBlockAccurate(
     "profile-sample-block-accurate", cl::Hidden, cl::init(false),
class SampleProfileMatcher {
   uint64_t TotalProfiledFunc = 0;
   uint64_t NumMismatchedFuncHash = 0;
   uint64_t MismatchedFuncHashSamples = 0;
   uint64_t TotalFuncHashSamples = 0;
 
 public:
   SampleProfileMatcher(Module &M, SampleProfileReader &Reader,
                        const PseudoProbeManager *ProbeManager)
-      : M(M), Reader(Reader), ProbeManager(ProbeManager) {
-    if (FlattenProfileForMatching) {
-      ProfileConverter::flattenProfile(Reader.getProfiles(), FlattenedProfiles,
-                                       FunctionSamples::ProfileIsCS);
-    }
-  }
+      : M(M), Reader(Reader), ProbeManager(ProbeManager){};
   void runOnModule();
 
 private:
   FunctionSamples *getFlattenedSamplesFor(const Function &F) {
     StringRef CanonFName = FunctionSamples::getCanonicalFnName(F);
     auto It = FlattenedProfiles.find(CanonFName);
     if (It != FlattenedProfiles.end())
       return &It->second;
     return nullptr;
   }
-  void runOnFunction(const Function &F, const FunctionSamples &FS);
+  void runOnFunction(const Function &F);
+  void computeHashMismatchSamples(const FunctionSamples &FS,
+                                  uint64_t &MismatchedSamples);
   void countProfileMismatches(
       const FunctionSamples &FS,
       const std::unordered_set<LineLocation, LineLocationHash>
           &MatchedCallsiteLocs,
       uint64_t &FuncMismatchedCallsites, uint64_t &FuncProfiledCallsites);
 
   LocToLocMap &getIRToProfileLocationMap(const Function &F) {
     auto Ret = FuncMappings.try_emplace(
         FunctionSamples::getCanonicalFnName(F.getName()), LocToLocMap());
     return Ret.first->second;
   }
   void distributeIRToProfileLocationMap();
   void distributeIRToProfileLocationMap(FunctionSamples &FS);
   void populateIRLocations(
-      const Function &F, const FunctionSamples &FS,
+      const Function &F, const FunctionSamples *FSForReporting,
       std::unordered_set<LineLocation, LineLocationHash> &MatchedCallsiteLocs,
       std::map<LineLocation, StringRef> &IRLocations);
   void populateProfileCallsites(
       const FunctionSamples &FS,
       StringMap<std::set<LineLocation>> &CalleeToCallsitesMap);
   void runStaleProfileMatching(
       const std::map<LineLocation, StringRef> &IRLocations,
       StringMap<std::set<LineLocation>> &CalleeToCallsitesMap,
if (ReportProfileStaleness || PersistProfileStaleness ||
       SalvageStaleProfile) {
     MatchingManager =
         std::make_unique<SampleProfileMatcher>(M, *Reader, ProbeManager.get());
   }
 
   return true;
 }
 
 void SampleProfileMatcher::populateIRLocations(
-    const Function &F, const FunctionSamples &FS,
+    const Function &F, const FunctionSamples *FSForReporting,
     std::unordered_set<LineLocation, LineLocationHash> &MatchedCallsiteLocs,
     std::map<LineLocation, StringRef> &IRLocations) {
   for (auto &BB : F) {
     for (auto &I : BB) {
       // Use flattened IR for the matching since we also use the flattened
       // profile.
       if (DILocation *DIL = I.getDebugLoc()) {
         if (DIL->getInlinedAt()) {
for (auto &I : BB) {
         // written before.
         auto R = IRLocations.emplace(IRCallsite, CalleeName);
         R.first->second = CalleeName;
         assert((!FunctionSamples::ProfileIsProbeBased || R.second ||
                 R.first->second == CalleeName) &&
                "Overwrite non-call or different callee name location for "
                "pseudo probe callsite");
 
+        if (!FSForReporting)
+          continue;
+
         // Go through all the callsites on the IR and flag the callsite if the
         // target name is the same as the one in the profile.
-        const auto CTM = FS.findCallTargetMapAt(IRCallsite);
-        const auto CallsiteFS = FS.findFunctionSamplesMapAt(IRCallsite);
+        const auto CTM = FSForReporting->findCallTargetMapAt(IRCallsite);
+        const auto CallsiteFS =
+            FSForReporting->findFunctionSamplesMapAt(IRCallsite);
 
         // Indirect call case.
         if (CalleeName.empty()) {
           // Since indirect call does not have the CalleeName, check
           // conservatively if callsite in the profile is a callsite location.
           // This is to avoid nums of false positive since otherwise all the
           // indirect call samples will be reported as mismatching.
           if ((CTM && !CTM->empty()) || (CallsiteFS && !CallsiteFS->empty()))
if (!IsMatchedAnchor) {
       InsertMatching(Loc, Candidate);
       LLVM_DEBUG(dbgs() << "Location is matched from " << Loc << " to "
                         << Candidate << "\n");
       LastMatchedNonAnchors.emplace_back(Loc);
     }
   }
 }
 
-void SampleProfileMatcher::runOnFunction(const Function &F,
-                                         const FunctionSamples &FS) {
-  bool IsFuncHashMismatch = false;
-  if (FunctionSamples::ProfileIsProbeBased) {
-    uint64_t Count = FS.getTotalSamples();
-    TotalFuncHashSamples += Count;
-    TotalProfiledFunc++;
-    const auto *FuncDesc = ProbeManager->getDesc(F);
-    if (FuncDesc && ProbeManager->profileIsHashMismatched(*FuncDesc, FS)) {
-      MismatchedFuncHashSamples += Count;
-      NumMismatchedFuncHash++;
-      IsFuncHashMismatch = true;
-    }
-  }
+void SampleProfileMatcher::computeHashMismatchSamples(
+    const FunctionSamples &FS, uint64_t &MismatchedSamples) {
+  const auto *FuncDesc = ProbeManager->getDesc(FS.getName());
+  // Skip the function that is external or renamed.
+  if (!FuncDesc)
+    return;
+
+  if (ProbeManager->profileIsHashMismatched(*FuncDesc, FS)) {
+    MismatchedSamples += FS.getTotalSamples();
+    return;
+
+  for (const auto &I : FS.getCallsiteSamples())
+    for (const auto &CS : I.second)
+      computeHashMismatchSamples(CS.second, MismatchedSamples);
 
+void SampleProfileMatcher::runOnFunction(const Function &F) {
+  FunctionSamples *FSForReporting = Reader.getSamplesFor(F);
   std::unordered_set<LineLocation, LineLocationHash> MatchedCallsiteLocs;
   // The value of the map is the name of direct callsite and use empty StringRef
   // for non-direct-call site.
   std::map<LineLocation, StringRef> IRLocations;
   // Extract profile matching anchors and profile mismatch metrics in the IR.
-  populateIRLocations(F, FS, MatchedCallsiteLocs, IRLocations);
+  populateIRLocations(F, FSForReporting, MatchedCallsiteLocs, IRLocations);
 
   // Detect profile mismatch for profile staleness metrics report.
-  if (ReportProfileStaleness || PersistProfileStaleness) {
+  // Skip to report the metrics for the imported functions.
+  if (FSForReporting &&
+      !GlobalValue::isAvailableExternallyLinkage(F.getLinkage()) &&
+      (ReportProfileStaleness || PersistProfileStaleness)) {
+    bool IsFuncHashMismatch = false;
+    if (FunctionSamples::ProfileIsProbeBased) {
+      TotalFuncHashSamples += FSForReporting->getTotalSamples();
+      ;
+      TotalProfiledFunc++;
+      const auto *FuncDesc = ProbeManager->getDesc(F);
+      if (FuncDesc &&
+          ProbeManager->profileIsHashMismatched(*FuncDesc, *FSForReporting)) {
+        computeHashMismatchSamples(*FSForReporting, MismatchedFuncHashSamples);
+        NumMismatchedFuncHash++;
+        IsFuncHashMismatch = true;
+      }
+    }
+
     uint64_t FuncMismatchedCallsites = 0;
     uint64_t FuncProfiledCallsites = 0;
-    countProfileMismatches(FS, MatchedCallsiteLocs, FuncMismatchedCallsites,
-                           FuncProfiledCallsites);
+    countProfileMismatches(*FSForReporting, MatchedCallsiteLocs,
+                           FuncMismatchedCallsites, FuncProfiledCallsites);
     TotalProfiledCallsites += FuncProfiledCallsites;
     NumMismatchedCallsites += FuncMismatchedCallsites;
     LLVM_DEBUG({
       if (FunctionSamples::ProfileIsProbeBased && !IsFuncHashMismatch &&
           FuncMismatchedCallsites)
         dbgs() << "Function checksum is matched but there are "
                << FuncMismatchedCallsites << "/" << FuncProfiledCallsites
                << " mismatched callsites.\n";
     });
   }
 
-  if (IsFuncHashMismatch && SalvageStaleProfile) {
+  // Run profile matching, currently only support for pseudo-probe.
+  if (SalvageStaleProfile && FunctionSamples::ProfileIsProbeBased) {
+    // Note that if a callsite isn't hit by any samples, it won't show up in the
+    // profile. We use flattened function samples so that callsites from
+    // different contexts are merged together for matching.
+    FunctionSamples *FSForMatching = getFlattenedSamplesFor(F);
+    assert((!FSForReporting || (FSForReporting && FSForMatching)) &&
+           "Flattened profile should not be null if nested profile exists");
+    // Run profile matching for checksum mismatched profile.
+    if (!FSForMatching || ProbeManager->profileIsValid(F, *FSForMatching))
+      return;
     LLVM_DEBUG(dbgs() << "Run stale profile matching for " << F.getName()
                       << "\n");
 
     StringMap<std::set<LineLocation>> CalleeToCallsitesMap;
-    populateProfileCallsites(FS, CalleeToCallsitesMap);
+    populateProfileCallsites(*FSForMatching, CalleeToCallsitesMap);
 
     // The matching result will be saved to IRToProfileLocationMap, create a new
     // map for each function.
     auto &IRToProfileLocationMap = getIRToProfileLocationMap(F);
 
     runStaleProfileMatching(IRLocations, CalleeToCallsitesMap,
                             IRToProfileLocationMap);
   }
 }
 
 void SampleProfileMatcher::runOnModule() {
+  if (SalvageStaleProfile)
+    ProfileConverter::flattenProfile(Reader.getProfiles(), FlattenedProfiles,
+                                     FunctionSamples::ProfileIsCS);
   for (auto &F : M) {
     if (F.isDeclaration() || !F.hasFnAttribute("use-sample-profile"))
       continue;
-    FunctionSamples *FS = nullptr;
-    if (FlattenProfileForMatching)
-      FS = getFlattenedSamplesFor(F);
-    else
-      FS = Reader.getSamplesFor(F);
-    if (!FS)
-      continue;
-    runOnFunction(F, *FS);
+    runOnFunction(F);
   }
   if (SalvageStaleProfile)
     distributeIRToProfileLocationMap();
 
   if (ReportProfileStaleness) {
     if (FunctionSamples::ProfileIsProbeBased) {
       errs() << "(" << NumMismatchedFuncHash << "/" << TotalProfiledFunc << ")"
              << " of functions' profile are invalid and "

llvm/test/Transforms/SampleProfile/Inputs/pseudo-probe-profile-mismatch-nested.prof

+main:30:0
+ 1: 0
+ 12: 10 matched:10
+ 20: 10 bar:10
+ 13: bar:10
+  1: 10
+  !CFGChecksum: 42949672950
+ !CFGChecksum: 844635331715433
+bar:11:11
+ 1: 11
+ !CFGChecksum: 42949672950

llvm/test/Transforms/SampleProfile/Inputs/pseudo-probe-stale-profile-matching-lto.prof

8: foo:1859
   2: 526
   3: 63
   4: 618
   6: 63 bar:65
   !CFGChecksum: 563022570642068
  !CFGChecksum: 1125988587804525
 bar:2401:2401
  1: 2401
- !CFGChecksum: 4294967295
+ !CFGChecksum: 123

llvm/test/Transforms/SampleProfile/profile-mismatch-flattened-profile.ll

-; REQUIRES: x86_64-linux
-; RUN: opt < %S/profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/profile-mismatch.prof -report-profile-staleness -persist-profile-staleness -flatten-profile-for-matching=1 -S 2>%t -o %t.ll
-; RUN: FileCheck %s --input-file %t
-; RUN: FileCheck %s --input-file %t.ll -check-prefix=CHECK-MD
-
-; RUN: opt < %S/profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/profile-mismatch-cs.prof -report-profile-staleness -persist-profile-staleness -flatten-profile-for-matching=1 -S 2>%t -o %t.ll
-; RUN: FileCheck %s --input-file %t
-; RUN: FileCheck %s --input-file %t.ll -check-prefix=CHECK-MD
-
-
-; CHECK: (3/4) of callsites' profile are invalid and (20/30) of samples are discarded due to callsite location mismatch.
-
-; CHECK-MD: ![[#]] = !{!"NumMismatchedCallsites", i64 3, !"TotalProfiledCallsites", i64 4, !"MismatchedCallsiteSamples", i64 20, !"TotalCallsiteSamples", i64 30}

llvm/test/Transforms/SampleProfile/profile-mismatch.ll

 ; REQUIRES: x86_64-linux
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-mismatch.prof -report-profile-staleness -persist-profile-staleness -flatten-profile-for-matching=0 -S 2>%t -o %t.ll
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-mismatch.prof -report-profile-staleness -persist-profile-staleness -S 2>%t -o %t.ll
 ; RUN: FileCheck %s --input-file %t
 ; RUN: FileCheck %s --input-file %t.ll -check-prefix=CHECK-MD
 ; RUN: llc < %t.ll -filetype=obj -o %t.obj
 ; RUN: llvm-objdump --section-headers %t.obj | FileCheck %s --check-prefix=CHECK-OBJ
 ; RUN: llc < %t.ll -filetype=asm -o - | FileCheck %s --check-prefix=CHECK-ASM
 
 ; CHECK: (2/3) of callsites' profile are invalid and (15/25) of samples are discarded due to callsite location mismatch.
 

llvm/test/Transforms/SampleProfile/pseudo-probe-profile-mismatch-thinlto.ll

+; REQUIRES: x86_64-linux
+; RUN: opt < %S/pseudo-probe-stale-profile-matching-lto.ll -passes='thinlto<O2>' -pgo-kind=pgo-sample-use-pipeline -sample-profile-file=%S/Inputs/pseudo-probe-stale-profile-matching-lto.prof -report-profile-staleness -persist-profile-staleness  -S 2>%t -o %t.ll
+; RUN: FileCheck %s --input-file %t
+; RUN: FileCheck %s --input-file %t.ll -check-prefix=CHECK-MD
+
+; CHECK: (1/1) of functions' profile are invalid and  (6822/6822) of samples are discarded due to function hash mismatch.
+; CHECK: (4/4) of callsites' profile are invalid and (2470/2470) of samples are discarded due to callsite location mismatch.
+
+
+; CHECK-MD: ![[#]] = !{!"NumMismatchedFuncHash", i64 1, !"TotalProfiledFunc", i64 1, !"MismatchedFuncHashSamples", i64 6822, !"TotalFuncHashSamples", i64 6822, !"NumMismatchedCallsites", i64 4, !"TotalProfiledCallsites", i64 4, !"MismatchedCallsiteSamples", i64 2470, !"TotalCallsiteSamples", i64 2470}

llvm/test/Transforms/SampleProfile/pseudo-probe-profile-mismatch.ll

 ; REQUIRES: x86_64-linux
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch.prof -report-profile-staleness -persist-profile-staleness -S 2>%t -o %t.ll
 ; RUN: FileCheck %s --input-file %t
 ; RUN: FileCheck %s --input-file %t.ll -check-prefix=CHECK-MD
 ; RUN: llc < %t.ll -filetype=obj -o %t.obj
 ; RUN: llvm-objdump --section-headers %t.obj | FileCheck %s --check-prefix=CHECK-OBJ
 ; RUN: llc < %t.ll -filetype=asm -o - | FileCheck %s --check-prefix=CHECK-ASM
 
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch-nested.prof -report-profile-staleness -persist-profile-staleness -S 2>&1 | FileCheck %s --check-prefix=CHECK-NESTED
+
 ; CHECK: (1/3) of functions' profile are invalid and (10/50) of samples are discarded due to function hash mismatch.
 ; CHECK: (2/3) of callsites' profile are invalid and (20/30) of samples are discarded due to callsite location mismatch.
 
 ; CHECK-MD: ![[#]] = !{!"NumMismatchedFuncHash", i64 1, !"TotalProfiledFunc", i64 3, !"MismatchedFuncHashSamples", i64 10, !"TotalFuncHashSamples", i64 50, !"NumMismatchedCallsites", i64 2, !"TotalProfiledCallsites", i64 3, !"MismatchedCallsiteSamples", i64 20, !"TotalCallsiteSamples", i64 30}
 
 ; CHECK-OBJ: .llvm_stats
 
 ; CHECK-ASM: .section  .llvm_stats,"",@progbits

 ; CHECK-ASM: .ascii  "MismatchedCallsiteSamples"
 ; CHECK-ASM: .byte 4
 ; CHECK-ASM: .ascii  "MjA="
 ; CHECK-ASM: .byte 20
 ; CHECK-ASM: .ascii  "TotalCallsiteSamples"
 ; CHECK-ASM: .byte 4
 ; CHECK-ASM: .ascii  "MzA="
 
+; CHECK-NESTED: (1/2) of functions' profile are invalid and (11/41) of samples are discarded due to function hash mismatch.
+
 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"
 
 @x = dso_local global i32 0, align 4, !dbg !0
 
 ; Function Attrs: nounwind uwtable
 define dso_local i32 @foo(i32 noundef %x) #0 !dbg !16 {
 entry: