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

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

Thanks for adding the feature.



================
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());
----------------
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? 


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:1058
+  for (const auto &SummaryEntry : SummaryVector) {
+    if (SummaryEntry.Cutoff == 990000) {
+      MinCountThreshold = SummaryEntry.MinCount;
----------------
Uses a const variable instead of a raw number.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81800