[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

2111–2112

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)

2418–2420

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

2418–2421

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

2423

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

2425

remove?

[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

2425

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?

2445

nit: remove the colon

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

2425

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/lib/Transforms/IPO/SampleProfile.cpp

bool SampleProfileLoader::doInitialization(Module &M,
   if (ReportProfileStaleness || PersistProfileStaleness ||
       SalvageStaleProfile) {
     MatchingManager =
         std::make_unique<SampleProfileMatcher>(M, *Reader, ProbeManager.get());
   }
 
   return true;
 }
 
 void SampleProfileMatcher::findIRAnchors(
     const Function &F, std::map<LineLocation, StringRef> &IRAnchors) {
   // Extract profile matching anchors in the IR.
   for (auto &BB : F) {
     for (auto &I : BB) {
       // TODO: To support AutoFDO, we need to parse all the non-call
       // instructions to extract the line-number based locations. For
       // pseudo-probe mode, since each block is instrumented with one probe
       // inst, parsing probe inst is enough.
void SampleProfileMatcher::runOnFunction(const Function &F) {
   std::map<LineLocation, StringRef> IRAnchors;
   findIRAnchors(F, IRAnchors);
   // Anchors for profile. It's a map from callsite location to a set of callee
   // name.
   std::map<LineLocation, StringSet<>> ProfileAnchors;
   findProfileAnchors(*FSFlattened, ProfileAnchors);
 
   // Detect profile mismatch for profile staleness metrics report.
-  if (ReportProfileStaleness || PersistProfileStaleness) {
+  // Skip reporting the metrics for imported functions.
+  if (!GlobalValue::isAvailableExternallyLinkage(F.getLinkage()) &&
+      (ReportProfileStaleness || PersistProfileStaleness)) {
     // Use top-level nested FS for counting profile mismatch metrics since
     // currently once a callsite is mismatched, all its children profiles are
     // dropped.
     if (const auto *FS = Reader.getSamplesFor(F))
       countProfileMismatches(F, *FS, IRAnchors, ProfileAnchors);
   }
 
   // Run profile matching for checksum mismatched profile, currently only
   // support for pseudo-probe.
   if (SalvageStaleProfile && FunctionSamples::ProfileIsProbeBased &&
       !ProbeManager->profileIsValid(F, *FSFlattened)) {
     // The matching result will be saved to IRToProfileLocationMap, create a new
     // map for each function.
     runStaleProfileMatching(F, IRAnchors, ProfileAnchors,
                             getIRToProfileLocationMap(F));
   }
 }
 
 void SampleProfileMatcher::runOnModule() {
   ProfileConverter::flattenProfile(Reader.getProfiles(), FlattenedProfiles,
                                    FunctionSamples::ProfileIsCS);
   for (auto &F : M) {
     if (F.isDeclaration() || !F.hasFnAttribute("use-sample-profile"))
       continue;
     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-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
+# Orignal CFGChecksum is 4294967295
+ !CFGChecksum: 123

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 (5026/5026) 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 5026, !"TotalCallsiteSamples", i64 5026}