Skip to content

Commit 02bb76c

Browse files
committed
addressing comments
1 parent f0be485 commit 02bb76c

File tree

2 files changed

+20
-23
lines changed

2 files changed

+20
-23
lines changed

llvm/lib/Transforms/IPO/SampleProfile.cpp

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -240,17 +240,15 @@ static cl::opt<unsigned> ChecksumMismatchFuncHotBlockSkip(
240240
"whose num of hot(on average) blocks is smaller than the "
241241
"given number."));
242242

243-
static cl::opt<unsigned> ChecksumMismatchNumFuncSkip(
244-
"checksum-mismatch-num-func-skip", cl::Hidden, cl::init(50),
245-
cl::desc("For checksum-mismatch error check, skip the check if the total "
246-
"number of selected function is smaller than the given number."));
243+
static cl::opt<unsigned> MinfuncsForStalenessError(
244+
"min-functions-for-staleness-error", cl::Hidden, cl::init(50),
245+
cl::desc("Skip the check if the number of hot functions is smaller than "
246+
"the given number."));
247247

248-
static cl::opt<unsigned> ChecksumMismatchErrorThreshold(
249-
"checksum-mismatch-error-threshold", cl::Hidden, cl::init(80),
250-
cl::desc(
251-
"For checksum-mismatch error check, error out if the percentage of "
252-
"function mismatched-checksum is higher than the given percentage "
253-
"threshold"));
248+
static cl::opt<unsigned> PrecentMismatchForStalenessError(
249+
"precent-mismatch-for-staleness-error", cl::Hidden, cl::init(80),
250+
cl::desc("Reject the profile if the mismatch percent is higher than the "
251+
"given number"));
254252

255253
static cl::opt<bool> CallsitePrioritizedInline(
256254
"sample-profile-prioritized-inline", cl::Hidden,
@@ -648,7 +646,7 @@ class SampleProfileLoader final : public SampleProfileLoaderBaseImpl<Function> {
648646
std::vector<Function *> buildFunctionOrder(Module &M, LazyCallGraph &CG);
649647
std::unique_ptr<ProfiledCallGraph> buildProfiledCallGraph(Module &M);
650648
void generateMDProfMetadata(Function &F);
651-
bool errorIfHighChecksumMismatch(Module &M, ProfileSummaryInfo *PSI,
649+
bool rejectHighStalenessProfile(Module &M, ProfileSummaryInfo *PSI,
652650
const SampleProfileMap &Profiles);
653651

654652
/// Map from function name to Function *. Used to find the function from
@@ -2218,12 +2216,12 @@ bool SampleProfileLoader::doInitialization(Module &M,
22182216
// function selection is based on two criteria: 1) The function is "hot" enough,
22192217
// which is tuned by a hotness-based flag(ChecksumMismatchFuncHotBlockSkip). 2)
22202218
// The num of function is large enough which is tuned by the
2221-
// ChecksumMismatchNumFuncSkip flag.
2222-
bool SampleProfileLoader::errorIfHighChecksumMismatch(
2219+
// MinfuncsForStalenessError flag.
2220+
bool SampleProfileLoader::rejectHighStalenessProfile(
22232221
Module &M, ProfileSummaryInfo *PSI, const SampleProfileMap &Profiles) {
22242222
assert(FunctionSamples::ProfileIsProbeBased &&
22252223
"Only support for probe-based profile");
2226-
uint64_t TotalSelectedFunc = 0;
2224+
uint64_t TotalHotFunc = 0;
22272225
uint64_t NumMismatchedFunc = 0;
22282226
for (const auto &I : Profiles) {
22292227
const auto &FS = I.second;
@@ -2240,22 +2238,22 @@ bool SampleProfileLoader::errorIfHighChecksumMismatch(
22402238
ChecksumMismatchFuncHotBlockSkip * PSI->getOrCompHotCountThreshold())
22412239
continue;
22422240

2243-
TotalSelectedFunc++;
2241+
TotalHotFunc++;
22442242
if (ProbeManager->profileIsHashMismatched(*FuncDesc, FS))
22452243
NumMismatchedFunc++;
22462244
}
22472245
// Make sure that the num of selected function is not too small to distinguish
22482246
// from the user's benign changes.
2249-
if (TotalSelectedFunc < ChecksumMismatchNumFuncSkip)
2247+
if (TotalHotFunc < MinfuncsForStalenessError)
22502248
return false;
22512249

22522250
// Finally check the mismatch percentage against the threshold.
22532251
if (NumMismatchedFunc * 100 >=
2254-
TotalSelectedFunc * ChecksumMismatchErrorThreshold) {
2252+
TotalHotFunc * PrecentMismatchForStalenessError) {
22552253
auto &Ctx = M.getContext();
22562254
const char *Msg =
2257-
"The FDO profile is too old and will cause big performance regression, "
2258-
"please drop the profile and collect a new one.";
2255+
"The input profile significantly mismatches current source code. "
2256+
"Please recollect profile to avoid performance regression.";
22592257
Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg));
22602258
return true;
22612259
}
@@ -2796,7 +2794,7 @@ bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM,
27962794

27972795
// Error out if the profile checksum mismatch is too high.
27982796
if (FunctionSamples::ProfileIsProbeBased &&
2799-
errorIfHighChecksumMismatch(M, PSI, Reader->getProfiles()))
2797+
rejectHighStalenessProfile(M, PSI, Reader->getProfiles()))
28002798
return false;
28012799

28022800
// Compute the total number of samples collected in this profile.
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
; REQUIRES: x86_64-linux
2-
; RUN: opt < %S/pseudo-probe-profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch.prof -checksum-mismatch-func-hot-block-skip=0 -checksum-mismatch-num-func-skip=1 -checksum-mismatch-error-threshold=1 -S 2>%t -o %t.ll
3-
; RUN: FileCheck %s --input-file %t
2+
; RUN: not opt < %S/pseudo-probe-profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch.prof -checksum-mismatch-func-hot-block-skip=0 -min-functions-for-staleness-error=1 -precent-mismatch-for-staleness-error=1 -S 2>&1 | FileCheck %s
43

5-
; CHECK: error: [[*]]: The FDO profile is too old and will cause big performance regression, please drop the profile and collect a new one.
4+
; CHECK: error: {{.*}}: The input profile significantly mismatches current source code. Please recollect profile to avoid performance regression.

0 commit comments

Comments
 (0)