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

[Wei He via Phabricator via llvm-commits]

weihe marked an inline comment as done.
weihe 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:
> > 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.  
> > 
> Thank you for the clarification! I'm working on it now.
Hello, I moved the output code into a function dumpHotFunctionList() in the current revision. I tried to make it more flexible by allowing callers to determine column titles and column offsets. But the attributes to print out is still hard-coded in this function. There needs to be some changes if PGO would like to reuse this code. Please let me know if you have any suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81800