[PATCH] D146452: [AutoFDO] Use flattened profiles for profile staleness metrics
Wenlei He via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 22 18:01:37 PDT 2023
wenlei added inline comments.
================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:1333
+
+ static void flattenNestedProfile(SampleProfileMap &OutputProfiles,
+ const FunctionSamples &FS) {
----------------
This function doesn't have check on whether input is CS or profile or not, and it relies on caller to pass in the right kind of profile. Should we make it private and only expose flattenProfile as API which can take any kind of profile as input.
================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1039
+ ProfileConverter::flattenProfile(ProfileMap, FunctionSamples::ProfileIsCS);
+ if (FunctionSamples::ProfileIsCS)
+ ProfileIsCS = FunctionSamples::ProfileIsCS = false;
----------------
this check seems unnecessary
================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1254
cl::desc("Generate nested function profiles for CSSPGO"));
+ cl::opt<bool> GenFlattenedProfile(
+ "gen-flattened-profile", cl::init(false),
----------------
do we need to make sure `gen-flattened-profile` and `gen-cs-nested-profile` won't/can't be used at the same time?
also wondering if we should consolidate them into one flag, something like `profile-structure=<cs-nested|flat>`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146452/new/
https://reviews.llvm.org/D146452
More information about the llvm-commits
mailing list