[llvm] [CSSPGO] Error out if the checksum mismatch is high (PR #84097)
Lei Wang via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 19 15:08:36 PDT 2024
================
@@ -2184,6 +2204,61 @@ bool SampleProfileLoader::doInitialization(Module &M,
return true;
}
+// Note that this is a module-level check. Even if one module is errored out,
+// the entire build will be errored out. However, the user could make big
+// changes to functions in single module but those changes might not be
+// performance significant to the whole binary. Therefore, we use a conservative
+// approach to make sure we only error out if it globally impacts the binary
+// performance. To achieve this, we use heuristics to select a reasonable
+// big set of functions that are supposed to be globally performance
+// significant, only compute and check the mismatch within those functions. The
+// function selection is based on two criteria: 1) The function is "hot" enough,
+// which is tuned by a hotness-based flag(ChecksumMismatchFuncHotBlockSkip). 2)
+// The num of function is large enough which is tuned by the
+// ChecksumMismatchNumFuncSkip flag.
+bool SampleProfileLoader::errorIfHighChecksumMismatch(
+ Module &M, ProfileSummaryInfo *PSI, const SampleProfileMap &Profiles) {
+ assert(FunctionSamples::ProfileIsProbeBased &&
+ "Only support for probe-based profile");
+ uint64_t TotalSelectedFunc = 0;
+ uint64_t NumMismatchedFunc = 0;
+ for (const auto &I : Profiles) {
+ const auto &FS = I.second;
+ const auto *FuncDesc = ProbeManager->getDesc(FS.getGUID());
+ if (!FuncDesc)
+ continue;
+
+ // We want to select a set of functions that are globally performance
+ // significant, in other words, if those functions profiles are
+ // checksum-mismatched and dropped, the whole binary will likely be
+ // impacted, so here we use a hotness-based threshold to control the
+ // selection.
+ if (FS.getTotalSamples() <
+ ChecksumMismatchFuncHotBlockSkip * PSI->getOrCompHotCountThreshold())
----------------
wlei-llvm wrote:
> Sounds good to just use `isHotCountNthPercentile` against total samples for now.
>
> > Maybe a different topic, but I'm surprised why it uses total_samples to make inlining decision..
>
> Yes, this is not ideal. We intentionally switched away from that for CSSPGO. So if you check, the path that uses `callsiteIsHot` should be AutoFDO only.
Good to know, checked that the usages of `callsiteIsHot` are indeed from AutoFDO path.
https://github.com/llvm/llvm-project/pull/84097
More information about the llvm-commits
mailing list