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

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 24 10:04:42 PDT 2023


hoy accepted this revision.
hoy added inline comments.


================
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),
----------------
wlei wrote:
> hoy wrote:
> > wenlei wrote:
> > > 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>`.
> > > also wondering if we should consolidate them into one flag, something like profile-structure=<cs-nested|flat>.
> > 
> > This is a good point. There's a similar term `SampleProfileLayout` introduced https://reviews.llvm.org/D121651 . Consider unifying them like below?
> > 
> > 
> > ```
> > enum SampleProfileLayout {
> >   SPL_None = 0,
> >   SPL_Nest = 0x1,
> >   SPL_Flat = 0x2,
> >   SPL_CSFlat = 0x3,
> > };
> > ```
> > 
> Good idea! The implementation in https://reviews.llvm.org/D121651 looks good to me, so I copied  that (except the "CSFlat", will leave it to the original patch), but I have scratching my head with the term `nest`, `CS-nested`, `flat`, `cs-flat`...I will follow your decision. 
Thanks. `Nest` and `Flat` sound good to me for now.


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