[llvm] [CSSPGO] Error out if the checksum mismatch is high (PR #84097)
Lei Wang via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 15 13:16:44 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:
> 1. The multiplier doesn't really has to do with "block" of the function, so the name can be confusing?
> 2. What do you think of using `isFunctionHotInCallGraphNthPercentile` or `isHotCountNthPercentile` with a percentile cutoff? We can tun the percentile to get what we need.
Let me try to clarify my initial thought: IIUC, all the "HotCount" stuffs in PSI are "block-level"(or body-sample level) concept, the `isFunctionHotXXX` function are used to check the **entry_count** of the function which is also "block-level"(the first block), however what I used here is the `getTotalSamples()` which I intend to include all the callee/inlinee samples(it could be the entry count is cold, but it does a lot of hot inlinings), as total_samples is kind of sum of all the block counts of this function, so I used a multiplier which I want to mean how many blocks on average is hot in this function.
So if we pass a sum of blocks(`getTotalSamples`) to this `isFunctionHotInCallGraphNthPercentile ` or `isHotCountNthPercentile ` function, might still be confusing?
I feet we actually don't have a function-level thing for this hotness check. so I was also thinking to add a new function to PSI, like:(but no sure if it's generally useful)
```
int getNumHotBlocks(const FunctionSamples &FS) {
int num = 0;
for(samples in all body samples)
if(samples > isHotCount())
num++;
return num;
}
```
Any thought? thanks!
https://github.com/llvm/llvm-project/pull/84097
More information about the llvm-commits
mailing list