[llvm] [nfc][llvm-profdata]Refactor llvm-profdata showInstrProfile (PR #71328)

Mingming Liu via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 6 14:21:27 PST 2023


https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/71328

>From 8dd6a71d78373cfaf8869358ae27990b7e083ad0 Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Sat, 4 Nov 2023 23:30:57 -0700
Subject: [PATCH 1/2] [nfc][llvm-profdata]refactor

---
 llvm/tools/llvm-profdata/llvm-profdata.cpp | 213 ++++++++++++---------
 1 file changed, 125 insertions(+), 88 deletions(-)

diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 7d665a8005b0d62..4d0e1549f7adc80 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -2372,9 +2372,10 @@ struct ValueSitesStats {
 };
 } // namespace
 
-static void traverseAllValueSites(const InstrProfRecord &Func, uint32_t VK,
-                                  ValueSitesStats &Stats, raw_fd_ostream &OS,
-                                  InstrProfSymtab *Symtab) {
+static void traverseAndShowAllValueSites(const InstrProfRecord &Func,
+                                         uint32_t VK, ValueSitesStats &Stats,
+                                         raw_fd_ostream &OS,
+                                         InstrProfSymtab *Symtab) {
   uint32_t NS = Func.getNumValueSites(VK);
   Stats.TotalNumValueSites += NS;
   for (size_t I = 0; I < NS; ++I) {
@@ -2406,6 +2407,14 @@ static void traverseAllValueSites(const InstrProfRecord &Func, uint32_t VK,
   }
 }
 
+namespace {
+struct InstrProfilePerFuncOptions {
+  bool ShowCounts;
+  bool ShowIndirectCallTargets;
+  bool ShowMemOPSizes;
+};
+}; // namespace
+
 static void showValueSitesStats(raw_fd_ostream &OS, uint32_t VK,
                                 ValueSitesStats &Stats) {
   OS << "  Total number of sites: " << Stats.TotalNumValueSites << "\n";
@@ -2420,14 +2429,97 @@ static void showValueSitesStats(raw_fd_ostream &OS, uint32_t VK,
   }
 }
 
-static int showInstrProfile(
-    const std::string &Filename, bool ShowCounts, uint32_t TopN,
-    bool ShowIndirectCallTargets, bool ShowMemOPSizes, bool ShowDetailedSummary,
-    std::vector<uint32_t> DetailedSummaryCutoffs, bool ShowAllFunctions,
-    bool ShowCS, uint64_t ValueCutoff, bool OnlyListBelow,
-    const std::string &ShowFunction, bool TextFormat, bool ShowBinaryIds,
-    bool ShowCovered, bool ShowProfileVersion, bool ShowTemporalProfTraces,
-    ShowFormat SFormat, raw_fd_ostream &OS) {
+static void
+showFuncPseudoCounters(const NamedInstrProfRecord &FuncRecord,
+                       const InstrProfRecord::CountPseudoKind PseudoKind,
+                       size_t &ShownFunctions, raw_fd_ostream &OS) {
+  if (!ShownFunctions)
+    OS << "Counters:\n";
+  ++ShownFunctions;
+  OS << "  " << FuncRecord.Name << ":\n"
+     << "    Hash: " << format("0x%016" PRIx64, FuncRecord.Hash) << "\n"
+     << "    Counters: " << FuncRecord.Counts.size();
+  if (PseudoKind == InstrProfRecord::PseudoHot)
+    OS << "    <PseudoHot>\n";
+  else if (PseudoKind == InstrProfRecord::PseudoWarm)
+    OS << "    <PseudoWarm>\n";
+  else
+    llvm_unreachable("Unknown PseudoKind");
+}
+
+static void showFuncInstrProfile(const NamedInstrProfRecord &Func,
+                                 const bool IsIRInstr,
+                                 const InstrProfilePerFuncOptions &Options,
+                                 InstrProfSymtab *Symtab,
+                                 std::vector<ValueSitesStats> &VPStats,
+                                 size_t &ShownFunctions, raw_fd_ostream &OS) {
+  if (!ShownFunctions)
+    OS << "Counters:\n";
+
+  ++ShownFunctions;
+
+  OS << "  " << Func.Name << ":\n"
+     << "    Hash: " << format("0x%016" PRIx64, Func.Hash) << "\n"
+     << "    Counters: " << Func.Counts.size() << "\n";
+  if (!IsIRInstr)
+    OS << "    Function count: " << Func.Counts[0] << "\n";
+
+  if (Options.ShowIndirectCallTargets)
+    OS << "    Indirect Call Site Count: "
+       << Func.getNumValueSites(IPVK_IndirectCallTarget) << "\n";
+
+  uint32_t NumMemOPCalls = Func.getNumValueSites(IPVK_MemOPSize);
+  if (Options.ShowMemOPSizes && NumMemOPCalls > 0)
+    OS << "    Number of Memory Intrinsics Calls: " << NumMemOPCalls << "\n";
+
+  if (Options.ShowCounts) {
+    OS << "    Block counts: [";
+    size_t Start = (IsIRInstr ? 0 : 1);
+    for (size_t I = Start, E = Func.Counts.size(); I < E; ++I) {
+      OS << (I == Start ? "" : ", ") << Func.Counts[I];
+    }
+    OS << "]\n";
+  }
+
+  if (Options.ShowIndirectCallTargets) {
+    OS << "    Indirect Target Results:\n";
+    traverseAndShowAllValueSites(Func, IPVK_IndirectCallTarget,
+                                 VPStats[IPVK_IndirectCallTarget], OS, Symtab);
+  }
+
+  if (Options.ShowMemOPSizes && NumMemOPCalls > 0) {
+    OS << "    Memory Intrinsic Size Results:\n";
+    traverseAndShowAllValueSites(Func, IPVK_MemOPSize, VPStats[IPVK_MemOPSize],
+                                 OS, nullptr);
+  }
+}
+
+static void showValueProfileStats(size_t ShownFunctions,
+                                  bool ShowIndirectCallTargets,
+                                  bool ShowMemOPSizes,
+                                  std::vector<ValueSitesStats> &VPStats,
+                                  raw_fd_ostream &OS) {
+  if (ShownFunctions && ShowIndirectCallTargets) {
+    OS << "Statistics for indirect call sites profile:\n";
+    showValueSitesStats(OS, IPVK_IndirectCallTarget,
+                        VPStats[IPVK_IndirectCallTarget]);
+  }
+
+  if (ShownFunctions && ShowMemOPSizes) {
+    OS << "Statistics for memory intrinsic calls sizes profile:\n";
+    showValueSitesStats(OS, IPVK_MemOPSize, VPStats[IPVK_MemOPSize]);
+  }
+}
+
+static int
+showInstrProfile(const std::string &Filename, uint32_t TopN,
+                 InstrProfilePerFuncOptions Options, bool ShowDetailedSummary,
+                 std::vector<uint32_t> DetailedSummaryCutoffs,
+                 bool ShowAllFunctions, bool ShowCS, uint64_t ValueCutoff,
+                 bool OnlyListBelow, const std::string &ShowFunction,
+                 bool TextFormat, bool ShowBinaryIds, bool ShowCovered,
+                 bool ShowProfileVersion, bool ShowTemporalProfTraces,
+                 ShowFormat SFormat, raw_fd_ostream &OS) {
   if (SFormat == ShowFormat::Json)
     exitWithError("JSON output is not supported for instr profiles");
   if (SFormat == ShowFormat::Yaml)
@@ -2468,21 +2560,21 @@ static int showInstrProfile(
   if (TextFormat && IsIRInstr)
     OS << ":ir\n";
 
+  const bool IsIRLevelProfile = Reader->isIRLevelProfile();
+
   for (const auto &Func : *Reader) {
-    if (Reader->isIRLevelProfile()) {
-      bool FuncIsCS = NamedInstrProfRecord::hasCSFlagInHash(Func.Hash);
-      if (FuncIsCS != ShowCS)
+    if (IsIRLevelProfile) {
+      if (NamedInstrProfRecord::hasCSFlagInHash(Func.Hash) != ShowCS)
         continue;
     }
     bool Show = ShowAllFunctions ||
                 (!ShowFunction.empty() && Func.Name.contains(ShowFunction));
 
-    bool doTextFormatDump = (Show && TextFormat);
+    const bool doTextFormatDump = (Show && TextFormat);
 
     if (doTextFormatDump) {
-      InstrProfSymtab &Symtab = Reader->getSymtab();
-      InstrProfWriter::writeRecordInText(Func.Name, Func.Hash, Func, Symtab,
-                                         OS);
+      InstrProfWriter::writeRecordInText(Func.Name, Func.Hash, Func,
+                                         Reader->getSymtab(), OS);
       continue;
     }
 
@@ -2500,20 +2592,9 @@ static int showInstrProfile(
 
     auto PseudoKind = Func.getCountPseudoKind();
     if (PseudoKind != InstrProfRecord::NotPseudo) {
-      if (Show) {
-        if (!ShownFunctions)
-          OS << "Counters:\n";
-        ++ShownFunctions;
-        OS << "  " << Func.Name << ":\n"
-           << "    Hash: " << format("0x%016" PRIx64, Func.Hash) << "\n"
-           << "    Counters: " << Func.Counts.size();
-        if (PseudoKind == InstrProfRecord::PseudoHot)
-          OS << "    <PseudoHot>\n";
-        else if (PseudoKind == InstrProfRecord::PseudoWarm)
-          OS << "    <PseudoWarm>\n";
-        else
-          llvm_unreachable("Unknown PseudoKind");
-      }
+      if (Show)
+        showFuncPseudoCounters(Func, PseudoKind, ShownFunctions, OS);
+
       continue;
     }
 
@@ -2543,47 +2624,8 @@ static int showInstrProfile(
     }
 
     if (Show) {
-      if (!ShownFunctions)
-        OS << "Counters:\n";
-
-      ++ShownFunctions;
-
-      OS << "  " << Func.Name << ":\n"
-         << "    Hash: " << format("0x%016" PRIx64, Func.Hash) << "\n"
-         << "    Counters: " << Func.Counts.size() << "\n";
-      if (!IsIRInstr)
-        OS << "    Function count: " << Func.Counts[0] << "\n";
-
-      if (ShowIndirectCallTargets)
-        OS << "    Indirect Call Site Count: "
-           << Func.getNumValueSites(IPVK_IndirectCallTarget) << "\n";
-
-      uint32_t NumMemOPCalls = Func.getNumValueSites(IPVK_MemOPSize);
-      if (ShowMemOPSizes && NumMemOPCalls > 0)
-        OS << "    Number of Memory Intrinsics Calls: " << NumMemOPCalls
-           << "\n";
-
-      if (ShowCounts) {
-        OS << "    Block counts: [";
-        size_t Start = (IsIRInstr ? 0 : 1);
-        for (size_t I = Start, E = Func.Counts.size(); I < E; ++I) {
-          OS << (I == Start ? "" : ", ") << Func.Counts[I];
-        }
-        OS << "]\n";
-      }
-
-      if (ShowIndirectCallTargets) {
-        OS << "    Indirect Target Results:\n";
-        traverseAllValueSites(Func, IPVK_IndirectCallTarget,
-                              VPStats[IPVK_IndirectCallTarget], OS,
-                              &(Reader->getSymtab()));
-      }
-
-      if (ShowMemOPSizes && NumMemOPCalls > 0) {
-        OS << "    Memory Intrinsic Size Results:\n";
-        traverseAllValueSites(Func, IPVK_MemOPSize, VPStats[IPVK_MemOPSize], OS,
-                              nullptr);
-      }
+      showFuncInstrProfile(Func, IsIRInstr, Options, , &(Reader->getSymtab()),
+                           VPStats, ShownFunctions, OS);
     }
   }
   if (Reader->hasError())
@@ -2621,16 +2663,8 @@ static int showInstrProfile(
       OS << "  " << hotfunc.first << ", max count = " << hotfunc.second << "\n";
   }
 
-  if (ShownFunctions && ShowIndirectCallTargets) {
-    OS << "Statistics for indirect call sites profile:\n";
-    showValueSitesStats(OS, IPVK_IndirectCallTarget,
-                        VPStats[IPVK_IndirectCallTarget]);
-  }
-
-  if (ShownFunctions && ShowMemOPSizes) {
-    OS << "Statistics for memory intrinsic calls sizes profile:\n";
-    showValueSitesStats(OS, IPVK_MemOPSize, VPStats[IPVK_MemOPSize]);
-  }
+  showValueProfileStats(ShownFunctions, ShowIndirectCallTargets, ShowMemOPSizes,
+                        VPStats, OS);
 
   if (ShowDetailedSummary) {
     OS << "Total number of blocks: " << PS->getNumCounts() << "\n";
@@ -3044,11 +3078,14 @@ static int show_main(int argc, const char *argv[]) {
 
   if (ProfileKind == instr)
     return showInstrProfile(
-        Filename, ShowCounts, TopNFunctions, ShowIndirectCallTargets,
-        ShowMemOPSizes, ShowDetailedSummary, DetailedSummaryCutoffs,
-        ShowAllFunctions, ShowCS, ValueCutoff, OnlyListBelow, ShowFunction,
-        TextFormat, ShowBinaryIds, ShowCovered, ShowProfileVersion,
-        ShowTemporalProfTraces, SFormat, OS);
+        Filename, TopNFunctions,
+        InstrProfilePerFuncOptions{.ShowCounts = ShowCounts,
+                                   .ShowIndirectCallTargets =
+                                       ShowIndirectCallTargets,
+                                   .ShowMemOPSizes = ShowMemOPSizes},
+        ShowDetailedSummary, DetailedSummaryCutoffs, ShowAllFunctions, ShowCS,
+        ValueCutoff, OnlyListBelow, ShowFunction, TextFormat, ShowBinaryIds,
+        ShowCovered, ShowProfileVersion, ShowTemporalProfTraces, SFormat, OS);
   if (ProfileKind == sample)
     return showSampleProfile(Filename, ShowCounts, TopNFunctions,
                              ShowAllFunctions, ShowDetailedSummary,

>From f4261ef249e9f2fb75ba08bfce3c2c2c33597f1e Mon Sep 17 00:00:00 2001
From: Mingming Liu <mingmingl at google.com>
Date: Mon, 6 Nov 2023 14:11:44 -0800
Subject: [PATCH 2/2] resolve comments

---
 llvm/tools/llvm-profdata/llvm-profdata.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 4d0e1549f7adc80..5e44c8b311adb52 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -2409,14 +2409,14 @@ static void traverseAndShowAllValueSites(const InstrProfRecord &Func,
 
 namespace {
 struct InstrProfilePerFuncOptions {
-  bool ShowCounts;
-  bool ShowIndirectCallTargets;
-  bool ShowMemOPSizes;
+  bool ShowCounts = false;
+  bool ShowIndirectCallTargets = false;
+  bool ShowMemOPSizes = false;
 };
 }; // namespace
 
 static void showValueSitesStats(raw_fd_ostream &OS, uint32_t VK,
-                                ValueSitesStats &Stats) {
+                                const ValueSitesStats &Stats) {
   OS << "  Total number of sites: " << Stats.TotalNumValueSites << "\n";
   OS << "  Total number of sites with values: "
      << Stats.TotalNumValueSitesWithValueProfile << "\n";
@@ -2497,7 +2497,7 @@ static void showFuncInstrProfile(const NamedInstrProfRecord &Func,
 static void showValueProfileStats(size_t ShownFunctions,
                                   bool ShowIndirectCallTargets,
                                   bool ShowMemOPSizes,
-                                  std::vector<ValueSitesStats> &VPStats,
+                                  const std::vector<ValueSitesStats> &VPStats,
                                   raw_fd_ostream &OS) {
   if (ShownFunctions && ShowIndirectCallTargets) {
     OS << "Statistics for indirect call sites profile:\n";
@@ -2624,7 +2624,7 @@ showInstrProfile(const std::string &Filename, uint32_t TopN,
     }
 
     if (Show) {
-      showFuncInstrProfile(Func, IsIRInstr, Options, , &(Reader->getSymtab()),
+      showFuncInstrProfile(Func, IsIRInstr, Options, &(Reader->getSymtab()),
                            VPStats, ShownFunctions, OS);
     }
   }
@@ -2663,8 +2663,8 @@ showInstrProfile(const std::string &Filename, uint32_t TopN,
       OS << "  " << hotfunc.first << ", max count = " << hotfunc.second << "\n";
   }
 
-  showValueProfileStats(ShownFunctions, ShowIndirectCallTargets, ShowMemOPSizes,
-                        VPStats, OS);
+  showValueProfileStats(ShownFunctions, Options.ShowIndirectCallTargets,
+                        Options.ShowMemOPSizes, VPStats, OS);
 
   if (ShowDetailedSummary) {
     OS << "Total number of blocks: " << PS->getNumCounts() << "\n";



More information about the llvm-commits mailing list