[PATCH] D146452: [AutoFDO] Use flattened profiles for profile staleness metrics

[Wenlei He via Phabricator via llvm-commits]

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