[profdata] Use --hot-func-list to show all hot functions

[ellishg]

The --hot-func-list flag is used for sample profiles to dump the list of hot functions. Add support to dump hot functions for IRPGO profiles as well.

This also removes a priority_queue used for --topn. We can instead store all functions and sort at the end before dumping. Since we are storing StringRefs, I believe this won't consume too much memory.

[llvmbot]

@llvm/pr-subscribers-pgo

Author: Ellis Hoag (ellishg)

Changes

The --hot-func-list flag is used for sample profiles to dump the list of hot functions. Add support to dump hot functions for IRPGO profiles as well.

This also removes a priority_queue used for --topn. We can instead store all functions and sort at the end before dumping. Since we are storing hashed names instead of strings, I believe this won't consume too much memory.

---

Full diff: https://github.com/llvm/llvm-project/pull/149428.diff

3 Files Affected:

• (modified) llvm/test/tools/llvm-profdata/c-general.test (+3-3)
• (added) llvm/test/tools/llvm-profdata/show-hot.proftext (+35)
• (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+24-28)

diff --git a/llvm/test/tools/llvm-profdata/c-general.test b/llvm/test/tools/llvm-profdata/c-general.test
index 7c48f7b04a05c..ab4849fac034f 100644
--- a/llvm/test/tools/llvm-profdata/c-general.test
+++ b/llvm/test/tools/llvm-profdata/c-general.test
@@ -22,6 +22,6 @@ SWITCHES-LABEL: Functions shown: 1
 CHECK-LABEL: Total functions: 12
 CHECK-NEXT: Maximum function count: 1
 CHECK-NEXT: Maximum internal block count: 100
-TOPN: boolean_operators, max count = 100
-TOPN-NEXT: simple_loops, max count = 100
-TOPN-NEXT:  conditionals, max count = 100
+TOPN:         simple_loops, max count = 100
+TOPN-NEXT:    conditionals, max count = 100
+TOPN-NEXT:    boolean_operators, max count = 100
diff --git a/llvm/test/tools/llvm-profdata/show-hot.proftext b/llvm/test/tools/llvm-profdata/show-hot.proftext
new file mode 100644
index 0000000000000..5c9bd61c20d28
--- /dev/null
+++ b/llvm/test/tools/llvm-profdata/show-hot.proftext
@@ -0,0 +1,35 @@
+# RUN: llvm-profdata show %s --hot-func-list | FileCheck %s
+
+# CHECK: # Hot count threshold: 101
+# CHECK: hot_b
+# CHECK: hot_a
+# CHECK: hot_c
+
+:ir
+hot_a
+# Func Hash:
+0x1234
+# Num Counters:
+1
+# Counter Values:
+101
+
+hot_b
+0x5678
+1
+202
+
+hot_c
+0x5678
+1
+101
+
+cold_d
+0xabcd
+1
+1
+
+cold_e
+0xefff
+1
+0
diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 45eac90aef935..f6d5c0cf14914 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -2849,9 +2849,9 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
   auto FS = vfs::getRealFileSystem();
   auto ReaderOrErr = InstrProfReader::create(Filename, *FS);
   std::vector<uint32_t> Cutoffs = std::move(DetailedSummaryCutoffs);
-  if (ShowDetailedSummary && Cutoffs.empty()) {
-    Cutoffs = ProfileSummaryBuilder::DefaultCutoffs;
-  }
+  if (Cutoffs.empty())
+    if (ShowDetailedSummary || ShowHotFuncList)
+      Cutoffs = ProfileSummaryBuilder::DefaultCutoffs;
   InstrProfSummaryBuilder Builder(std::move(Cutoffs));
   if (Error E = ReaderOrErr.takeError())
     exitWithError(std::move(E), Filename);
@@ -2863,15 +2863,7 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
   int NumVPKind = IPVK_Last - IPVK_First + 1;
   std::vector<ValueSitesStats> VPStats(NumVPKind);
 
-  auto MinCmp = [](const std::pair<std::string, uint64_t> &v1,
-                   const std::pair<std::string, uint64_t> &v2) {
-    return v1.second > v2.second;
-  };
-
-  std::priority_queue<std::pair<std::string, uint64_t>,
-                      std::vector<std::pair<std::string, uint64_t>>,
-                      decltype(MinCmp)>
-      HottestFuncs(MinCmp);
+  std::vector<std::pair<uint64_t, uint64_t>> NameRefAndMaxCount;
 
   if (!TextFormat && OnlyListBelow) {
     OS << "The list of functions with the maximum counter less than "
@@ -2946,15 +2938,9 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
     } else if (OnlyListBelow)
       continue;
 
-    if (TopNFunctions) {
-      if (HottestFuncs.size() == TopNFunctions) {
-        if (HottestFuncs.top().second < FuncMax) {
-          HottestFuncs.pop();
-          HottestFuncs.emplace(std::make_pair(std::string(Func.Name), FuncMax));
-        }
-      } else
-        HottestFuncs.emplace(std::make_pair(std::string(Func.Name), FuncMax));
-    }
+    if (TopNFunctions || ShowHotFuncList)
+      NameRefAndMaxCount.emplace_back(IndexedInstrProf::ComputeHash(Func.Name),
+                                      FuncMax);
 
     if (Show) {
       if (!ShownFunctions)
@@ -3034,16 +3020,26 @@ static int showInstrProfile(ShowFormat SFormat, raw_fd_ostream &OS) {
        << "): " << PS->getNumFunctions() - BelowCutoffFunctions << "\n";
   }
 
+  // Sort by MaxCount in decreasing order
+  llvm::stable_sort(NameRefAndMaxCount, [](const auto &L, const auto &R) {
+    return L.second > R.second;
+  });
   if (TopNFunctions) {
-    std::vector<std::pair<std::string, uint64_t>> SortedHottestFuncs;
-    while (!HottestFuncs.empty()) {
-      SortedHottestFuncs.emplace_back(HottestFuncs.top());
-      HottestFuncs.pop();
-    }
     OS << "Top " << TopNFunctions
        << " functions with the largest internal block counts: \n";
-    for (auto &hotfunc : llvm::reverse(SortedHottestFuncs))
-      OS << "  " << hotfunc.first << ", max count = " << hotfunc.second << "\n";
+    auto TopFuncs = ArrayRef(NameRefAndMaxCount).take_front(TopNFunctions);
+    for (auto [NameRef, MaxCount] : TopFuncs)
+      OS << "  " << Reader->getSymtab().getFuncOrVarName(NameRef)
+         << ", max count = " << MaxCount << "\n";
+  }
+
+  if (ShowHotFuncList) {
+    auto HotCountThreshold =
+        ProfileSummaryBuilder::getHotCountThreshold(PS->getDetailedSummary());
+    OS << "# Hot count threshold: " << HotCountThreshold << "\n";
+    for (auto [NameRef, MaxCount] : NameRefAndMaxCount)
+      if (MaxCount >= HotCountThreshold)
+        OS << Reader->getSymtab().getFuncOrVarName(NameRef) << "\n";
   }
 
   if (ShownFunctions && ShowIndirectCallTargets) {

[mingmingl-llvm]

thanks for the change. It mostly lgtm, with one implementation question around function name representation (StringRef vs hash)

[mingmingl-llvm]

nit: remove nested if, something like

if (Cutoffs.empty() && (ShowDetailedSummary || ShowHotFuncList)) {
    Cutoffs = ProfileSummaryBuilder::DefaultCutoffs;
}

[mingmingl-llvm]

nit: exit the loop after seeing the first counter that's smaller than HotCountThreshold, given NameRefAndMaxCount is sorted by count descendingly.

[mingmingl-llvm]

What do you think of using std::vector<std::pair<StringRef, uint64>> NameRefAndMaxCount here? This can saves the hashing / re-hashing work.

[ellishg]

Thanks, that's a lot more simple