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

[Wei He via Phabricator via llvm-commits]

weihe marked 3 inline comments as done.
weihe added inline comments.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1032-1048
+static uint64_t
+getInlineeMaxCount(const sampleprof::FunctionSamples &FuncProf) {
+  using namespace sampleprof;
+
+  uint64_t MaxCount = 0;
+  for (const auto &L : FuncProf.getBodySamples()) {
+    MaxCount = std::max(MaxCount, L.second.getSamples());
----------------
wmi wrote:
> Please make it a member function of FunctionSamples so it can be used elsewhere. 
> 
> The max count can either be in inlinee or in itself. Your comment explicitly says that and it is good. Just the name getInlineeMaxCount is a little misleading. Perhaps use another name like getMaxCountInside? 
Sounds good! I'll move and rename it. Thank you for your suggestions.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1058
+  for (const auto &SummaryEntry : SummaryVector) {
+    if (SummaryEntry.Cutoff == 990000) {
+      MinCountThreshold = SummaryEntry.MinCount;
----------------
wmi wrote:
> Uses a const variable instead of a raw number.
Yeah, I'll change it. Thanks you!


================
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))
----------------
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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81800