[PATCH] D81800: Add --hot-func-list to llvm-profdata show for sample profiles

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1092-1127
+  const int MaxSampleCol = 24; // column offset for output formatting
+  const int EntrySampleCol = 42;
+  const int FuncNameCol = 58;
+  formatted_raw_ostream FOS(OS);
+  FOS << HotFuncCount << " out of " << Profiles.size()
+      << " functions with profile ("
+      << format("%.2f%%", (((double)HotFuncCount) / Profiles.size() * 100))
----------------
weihe wrote:
> wmi wrote:
> > Can you wrap the output part in a function? I feel it can be useful as well for Instr profile. It is also better to be flexible about which metric is used to decide whether the function is hot (currently it is MaxCount). That could be different between PGO and SampleFDO.
> Thank you for the suggestions! I would like to ask the following questions to make sure I understand them correctly:
> 
> For wrapping the output part into a function, is this function supposed to be reusable for other output format or only for the current format?
> 
> For making the metric flexible, is the purpose to make the showHotFunctionList() reusable for PGO? I'm asking this question because this function is currently coupled with FunctionSample objects and interfaces that may not work with other types of profiles. 
> 
> Thanks!
It is to make the output part usable for PGO profile. I see that showHotFunctionList is coupled with FunctionSample, but after you decide which functions to output based on Function Sample, the output content and format are independent from profile kind and can be shared with PGO.  



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81800/new/

https://reviews.llvm.org/D81800