Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SHT_LLVM_BB_ADDR_MAP] Adds pretty printing of BFI and BPI for PGO Analysis Map in tools. #82292

Merged
merged 13 commits into from
Feb 27, 2024

Conversation

red1bluelost
Copy link
Member

Primary change is to add a flag --pretty-pgo-analysis-map to llvm-readobj and llvm-objdump that prints block frequencies and branch probabilities in the same manner as BFI and BPI respectively. This can be helpful if you are manually inspecting the outputs from the tools.

In order to print, I moved the printBlockFreqImpl function from Analysis to Support and renamed it to printRelativeBlockFreq.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 20, 2024

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-support

Author: Micah Weston (red1bluelost)

Changes

Primary change is to add a flag --pretty-pgo-analysis-map to llvm-readobj and llvm-objdump that prints block frequencies and branch probabilities in the same manner as BFI and BPI respectively. This can be helpful if you are manually inspecting the outputs from the tools.

In order to print, I moved the printBlockFreqImpl function from Analysis to Support and renamed it to printRelativeBlockFreq.


Patch is 21.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82292.diff

13 Files Affected:

  • (modified) llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h (-3)
  • (modified) llvm/include/llvm/Support/BlockFrequency.h (+4)
  • (modified) llvm/lib/Analysis/BlockFrequencyInfo.cpp (+1-1)
  • (modified) llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp (-15)
  • (modified) llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp (+1-1)
  • (modified) llvm/lib/Support/BlockFrequency.cpp (+17)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml (+25-9)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test (+15-7)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+21-7)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+21-13)
  • (modified) llvm/tools/llvm-readobj/ObjDumper.h (+3-1)
  • (modified) llvm/tools/llvm-readobj/Opts.td (+1)
  • (modified) llvm/tools/llvm-readobj/llvm-readobj.cpp (+8-1)
diff --git a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
index 8acb75e8725410..4aa922635c374e 100644
--- a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
+++ b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
@@ -539,9 +539,6 @@ class BlockFrequencyInfoImplBase {
   }
 };
 
-void printBlockFreqImpl(raw_ostream &OS, BlockFrequency EntryFreq,
-                        BlockFrequency Freq);
-
 namespace bfi_detail {
 
 template <class BlockT> struct TypeMap {};
diff --git a/llvm/include/llvm/Support/BlockFrequency.h b/llvm/include/llvm/Support/BlockFrequency.h
index 8b172ee486aab8..aeab99615a951a 100644
--- a/llvm/include/llvm/Support/BlockFrequency.h
+++ b/llvm/include/llvm/Support/BlockFrequency.h
@@ -19,6 +19,7 @@
 
 namespace llvm {
 
+class raw_ostream;
 class BranchProbability;
 
 // This class represents Block Frequency as a 64-bit value.
@@ -119,6 +120,9 @@ class BlockFrequency {
   }
 };
 
+void printRelativeBlockFreq(raw_ostream &OS, BlockFrequency EntryFreq,
+                            BlockFrequency Freq);
+
 } // namespace llvm
 
 #endif
diff --git a/llvm/lib/Analysis/BlockFrequencyInfo.cpp b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
index 96c9bfa0e372ce..ebad8388cbe410 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfo.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
@@ -284,7 +284,7 @@ void BlockFrequencyInfo::verifyMatch(BlockFrequencyInfo &Other) const {
 Printable llvm::printBlockFreq(const BlockFrequencyInfo &BFI,
                                BlockFrequency Freq) {
   return Printable([&BFI, Freq](raw_ostream &OS) {
-    printBlockFreqImpl(OS, BFI.getEntryFreq(), Freq);
+    printRelativeBlockFreq(OS, BFI.getEntryFreq(), Freq);
   });
 }
 
diff --git a/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp b/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
index ae08d56ef098a7..9f6e53ba15b6a6 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
@@ -634,21 +634,6 @@ BlockFrequencyInfoImplBase::getLoopName(const LoopData &Loop) const {
   return getBlockName(Loop.getHeader()) + (Loop.isIrreducible() ? "**" : "*");
 }
 
-void llvm::printBlockFreqImpl(raw_ostream &OS, BlockFrequency EntryFreq,
-                              BlockFrequency Freq) {
-  if (Freq == BlockFrequency(0)) {
-    OS << "0";
-    return;
-  }
-  if (EntryFreq == BlockFrequency(0)) {
-    OS << "<invalid BFI>";
-    return;
-  }
-  Scaled64 Block(Freq.getFrequency(), 0);
-  Scaled64 Entry(EntryFreq.getFrequency(), 0);
-  OS << Block / Entry;
-}
-
 void IrreducibleGraph::addNodesInLoop(const BFIBase::LoopData &OuterLoop) {
   Start = OuterLoop.getHeader();
   Nodes.reserve(OuterLoop.Nodes.size());
diff --git a/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp b/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
index 7ee72e21442630..cbebdd87398e4b 100644
--- a/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
+++ b/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
@@ -280,7 +280,7 @@ BlockFrequency MachineBlockFrequencyInfo::getEntryFreq() const {
 Printable llvm::printBlockFreq(const MachineBlockFrequencyInfo &MBFI,
                                BlockFrequency Freq) {
   return Printable([&MBFI, Freq](raw_ostream &OS) {
-    printBlockFreqImpl(OS, MBFI.getEntryFreq(), Freq);
+    printRelativeBlockFreq(OS, MBFI.getEntryFreq(), Freq);
   });
 }
 
diff --git a/llvm/lib/Support/BlockFrequency.cpp b/llvm/lib/Support/BlockFrequency.cpp
index 329f1e12cdc29b..7d5498e7cb9974 100644
--- a/llvm/lib/Support/BlockFrequency.cpp
+++ b/llvm/lib/Support/BlockFrequency.cpp
@@ -13,6 +13,8 @@
 #include "llvm/Support/BlockFrequency.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/ScaledNumber.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
@@ -45,3 +47,18 @@ std::optional<BlockFrequency> BlockFrequency::mul(uint64_t Factor) const {
     return {};
   return BlockFrequency(ResultFrequency);
 }
+
+void llvm::printRelativeBlockFreq(raw_ostream &OS, BlockFrequency EntryFreq,
+                                  BlockFrequency Freq) {
+  if (Freq == BlockFrequency(0)) {
+    OS << "0";
+    return;
+  }
+  if (EntryFreq == BlockFrequency(0)) {
+    OS << "<invalid BFI>";
+    return;
+  }
+  ScaledNumber<uint64_t> Block(Freq.getFrequency(), 0);
+  ScaledNumber<uint64_t> Entry(EntryFreq.getFrequency(), 0);
+  OS << Block / Entry;
+}
diff --git a/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml b/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
index 732fab3e2a378c..8ac9b1434aaef8 100644
--- a/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
+++ b/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
@@ -48,6 +48,8 @@ Symbols:
 # RUN: yaml2obj %s --docnum=2 -o %t2
 # RUN: llvm-objdump %t2 -d --symbolize-operands --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --check-prefix=ENTRYCOUNT-BLOCKFREQ
+# RUN: llvm-objdump %t2 -d --symbolize-operands --pretty-pgo-analysis-map --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s --check-prefix=ENTRYCOUNT-BLOCKFREQ-PRETTY
 
 --- !ELF
 FileHeader:
@@ -104,12 +106,20 @@ Symbols:
 # ENTRYCOUNT-BLOCKFREQ: <BB2> (Frequency: 18):
 # ENTRYCOUNT-BLOCKFREQ: <BB5> (Frequency: 1000):
 
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <foo>:
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB3> (Entry count: 1000, Frequency: 1.0):
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB1> (Frequency: 0.133):
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB2> (Frequency: 0.018):
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB5> (Frequency: 1.0):
+
 ## Check the case where we have entry points, block frequency, and branch
 ## proabability information.
 
 # RUN: yaml2obj %s --docnum=3 -o %t3
 # RUN: llvm-objdump %t3 -d --symbolize-operands --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --check-prefix=ENTRY-FREQ-PROB
+# RUN: llvm-objdump %t3 -d --symbolize-operands --pretty-pgo-analysis-map --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s --check-prefix=ENTRY-FREQ-PROB-PRETTY
 
 --- !ELF
 FileHeader:
@@ -154,21 +164,21 @@ Sections:
           - BBFreq: 1000
             Successors:
             - ID:          1
-              BrProb:      0x22222222
+              BrProb:      0x10000000
             - ID:          2
-              BrProb:      0x33333333
+              BrProb:      0x15000000
             - ID:          3
-              BrProb:      0xaaaaaaaa
+              BrProb:      0x50000000
           - BBFreq: 133
             Successors:
             - ID:          2
-              BrProb:      0x11111111
+              BrProb:      0x10000000
             - ID:          3
-              BrProb:      0xeeeeeeee
+              BrProb:      0x70000000
           - BBFreq: 18
             Successors:
             - ID:          3
-              BrProb:      0xffffffff
+              BrProb:      0x80000000
           - BBFreq: 1000
             Successors:    []
 Symbols:
@@ -177,7 +187,13 @@ Symbols:
     Value:   0x0
 
 # ENTRY-FREQ-PROB: <foo>:
-# ENTRY-FREQ-PROB: <BB3> (Entry count: 1000, Frequency: 1000, Successors: BB1:22222222, BB2:33333333, BB3:aaaaaaaa):
-# ENTRY-FREQ-PROB: <BB1> (Frequency: 133, Successors: BB2:11111111, BB3:eeeeeeee):
-# ENTRY-FREQ-PROB: <BB2> (Frequency: 18, Successors: BB3:ffffffff):
+# ENTRY-FREQ-PROB: <BB3> (Entry count: 1000, Frequency: 1000, Successors: BB1:10000000, BB2:15000000, BB3:50000000):
+# ENTRY-FREQ-PROB: <BB1> (Frequency: 133, Successors: BB2:10000000, BB3:70000000):
+# ENTRY-FREQ-PROB: <BB2> (Frequency: 18, Successors: BB3:80000000):
 # ENTRY-FREQ-PROB: <BB5> (Frequency: 1000):
+
+# ENTRY-FREQ-PROB-PRETTY: <foo>:
+# ENTRY-FREQ-PROB-PRETTY: <BB3> (Entry count: 1000, Frequency: 1.0, Successors: BB1:[0x10000000 / 0x80000000 = 12.50%], BB2:[0x15000000 / 0x80000000 = 16.41%], BB3:[0x50000000 / 0x80000000 = 62.50%]):
+# ENTRY-FREQ-PROB-PRETTY: <BB1> (Frequency: 0.133, Successors: BB2:[0x10000000 / 0x80000000 = 12.50%], BB3:[0x70000000 / 0x80000000 = 87.50%]):
+# ENTRY-FREQ-PROB-PRETTY: <BB2> (Frequency: 0.018, Successors: BB3:[0x80000000 / 0x80000000 = 100.00%]):
+# ENTRY-FREQ-PROB-PRETTY: <BB5> (Frequency: 1.0):
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
index e5a9400c670c00..1ad5d5f7611268 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
@@ -3,12 +3,14 @@
 
 ## Check 64-bit:
 # RUN: yaml2obj %s -DBITS=64 -DADDR=0x999999999 -o %t1.x64.o
-# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefix=CHECK
+# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,RAW
+# RUN: llvm-readobj %t1.x64.o --bb-addr-map --pretty-pgo-analysis-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,PRETTY
 # RUN: llvm-readelf %t1.x64.o --bb-addr-map | FileCheck %s --check-prefix=GNU
+# RUN: llvm-readobj %t1.x64.o --pretty-pgo-analysis-map 2>&1 | FileCheck %s --check-prefix=PRETTY-NO-BAM
 
 ## Check 32-bit:
 # RUN: yaml2obj %s -DBITS=32 -o %t1.x32.o
-# RUN: llvm-readobj %t1.x32.o --bb-addr-map 2>&1 | FileCheck -DADDR=0x11111 %s -DFILE=%t1.x32.o --check-prefix=CHECK
+# RUN: llvm-readobj %t1.x32.o --bb-addr-map 2>&1 | FileCheck -DADDR=0x11111 %s -DFILE=%t1.x32.o --check-prefixes=CHECK,RAW
 # RUN: llvm-readelf %t1.x32.o --bb-addr-map | FileCheck %s --check-prefix=GNU
 
 ## Check that a malformed section can be handled.
@@ -55,16 +57,19 @@
 # CHECK-NEXT:       FuncEntryCount: 100
 # CHECK-NEXT:       PGO BB entries [
 # CHECK-NEXT:         {
-# CHECK-NEXT:           Frequency: 100
+# RAW-NEXT:             Frequency: 100
+# PRETTY-NEXT:          Frequency: 1.0
 # CHECK-NEXT:           Successors [
 # CHECK-NEXT:             {
 # CHECK-NEXT:               ID: 2
-# CHECK-NEXT:               Probability: 0xFFFFFFFF
+# RAW-NEXT:                 Probability: 0x80000000
+# PRETTY-NEXT:              Probability: 0x80000000 / 0x80000000 = 100.00%
 # CHECK-NEXT:             }
 # CHECK-NEXT:           ]
 # CHECK-NEXT:         }
 # CHECK-NEXT:         {
-# CHECK-NEXT:           Frequency: 100
+# RAW-NEXT:             Frequency: 100
+# PRETTY-NEXT:          Frequency: 1.0
 # CHECK-NEXT:           Successors [
 # CHECK-NEXT:           ]
 # CHECK-NEXT:         }
@@ -95,7 +100,8 @@
 # CHECK-NEXT:       FuncEntryCount: 8888
 # CHECK-NEXT:       PGO BB entries [
 # CHECK-NEXT:         {
-# CHECK-NEXT:           Frequency: 9000
+# RAW-NEXT:             Frequency: 9000
+# PRETTY-NEXT:          Frequency: 1.0
 # CHECK-NEXT:         }
 # CHECK-NEXT:       ]
 # CHECK-NEXT:     }
@@ -104,6 +110,8 @@
 
 # GNU: GNUStyle::printBBAddrMaps not implemented
 
+# PRETTY-NO-BAM: warning: --bb-addr-map must be enabled for --pretty-pgo-analysis-map to have an effect
+
 # TRUNCATED:      BBAddrMap [
 # TRUNCATED-NEXT:   warning: '[[FILE]]': unable to dump SHT_LLVM_BB_ADDR_MAP section with index 3: unable to decode LEB128 at offset [[OFFSET]]: malformed uleb128, extends past end
 # TRUNCATED-NEXT: ]
@@ -192,7 +200,7 @@ Sections:
           - BBFreq:        100
             Successors:
               - ID:        2
-                BrProb:    0xFFFFFFFF
+                BrProb:    0x80000000
           - BBFreq:        100
             Successors:    []
       - FuncEntryCount: 8888
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 948a5d74e1ab2b..7a398e35a8eb53 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -188,8 +188,10 @@ class BBAddrMapFunctionEntry {
   const BBAddrMap &getAddrMap() const { return AddrMap; }
 
   // Returns the PGO string associated with the entry of index `PGOBBEntryIndex`
-  // in `PGOMap`.
-  std::string constructPGOLabelString(size_t PGOBBEntryIndex) const {
+  // in `PGOMap`. If PrettyPGOAnalysis is true, prints BFI as relative frequency
+  // and BPI as percentage. Otherwise raw values are displayed.
+  std::string constructPGOLabelString(size_t PGOBBEntryIndex,
+                                      bool PrettyPGOAnalysis) const {
     if (!PGOMap.FeatEnable.hasPGOAnalysis())
       return "";
     std::string PGOString;
@@ -211,7 +213,13 @@ class BBAddrMapFunctionEntry {
           PGOMap.BBEntries[PGOBBEntryIndex];
 
       if (PGOMap.FeatEnable.BBFreq) {
-        PGOSS << "Frequency: " << Twine(PGOBBEntry.BlockFreq.getFrequency());
+        PGOSS << "Frequency: ";
+        if (PrettyPGOAnalysis) {
+          printRelativeBlockFreq(PGOSS, PGOMap.BBEntries.front().BlockFreq,
+                                 PGOBBEntry.BlockFreq);
+        } else {
+          PGOSS << Twine(PGOBBEntry.BlockFreq.getFrequency());
+        }
         if (PGOMap.FeatEnable.BrProb && PGOBBEntry.Successors.size() > 0) {
           PGOSS << ", ";
         }
@@ -220,9 +228,13 @@ class BBAddrMapFunctionEntry {
         PGOSS << "Successors: ";
         interleaveComma(
             PGOBBEntry.Successors, PGOSS,
-            [&PGOSS](const PGOAnalysisMap::PGOBBEntry::SuccessorEntry &SE) {
+            [&](const PGOAnalysisMap::PGOBBEntry::SuccessorEntry &SE) {
               PGOSS << "BB" << SE.ID << ":";
-              PGOSS.write_hex(SE.Prob.getNumerator());
+              if (PrettyPGOAnalysis) {
+                PGOSS << "[" << SE.Prob << "]";
+              } else {
+                PGOSS.write_hex(SE.Prob.getNumerator());
+              }
             });
       }
     }
@@ -331,6 +343,7 @@ static bool HasStopAddressFlag;
 
 bool objdump::SymbolTable;
 static bool SymbolizeOperands;
+static bool PrettyPGOAnalysisMap;
 static bool DynamicSymbolTable;
 std::string objdump::TripleName;
 bool objdump::UnwindInfo;
@@ -1410,8 +1423,8 @@ static void collectBBAddrMapLabels(
 
     std::string LabelString = ("BB" + Twine(BBEntry.ID)).str();
     Labels[BBAddress].push_back(
-        {LabelString,
-         FunctionMap->constructPGOLabelString(NumBBEntriesBeforeRange + I)});
+        {LabelString, FunctionMap->constructPGOLabelString(
+                          NumBBEntriesBeforeRange + I, PrettyPGOAnalysisMap)});
   }
 }
 
@@ -3473,6 +3486,7 @@ static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   HasStopAddressFlag = InputArgs.hasArg(OBJDUMP_stop_address_EQ);
   SymbolTable = InputArgs.hasArg(OBJDUMP_syms);
   SymbolizeOperands = InputArgs.hasArg(OBJDUMP_symbolize_operands);
+  PrettyPGOAnalysisMap = InputArgs.hasArg(OBJDUMP_pretty_pgo_analysis_map);
   DynamicSymbolTable = InputArgs.hasArg(OBJDUMP_dynamic_syms);
   TripleName = InputArgs.getLastArgValue(OBJDUMP_triple_EQ).str();
   UnwindInfo = InputArgs.hasArg(OBJDUMP_unwind_info);
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 50ea63e87a43be..9710fcf14430e7 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -593,7 +593,7 @@ template <typename ELFT> class GNUELFDumper : public ELFDumper<ELFT> {
   void printVersionDefinitionSection(const Elf_Shdr *Sec) override;
   void printVersionDependencySection(const Elf_Shdr *Sec) override;
   void printCGProfile() override;
-  void printBBAddrMaps() override;
+  void printBBAddrMaps(bool PrettyPGOAnalysis) override;
   void printAddrsig() override;
   void printNotes() override;
   void printELFLinkerOptions() override;
@@ -704,7 +704,7 @@ template <typename ELFT> class LLVMELFDumper : public ELFDumper<ELFT> {
   void printVersionDefinitionSection(const Elf_Shdr *Sec) override;
   void printVersionDependencySection(const Elf_Shdr *Sec) override;
   void printCGProfile() override;
-  void printBBAddrMaps() override;
+  void printBBAddrMaps(bool PrettyPGOAnalysis) override;
   void printAddrsig() override;
   void printNotes() override;
   void printELFLinkerOptions() override;
@@ -5031,7 +5031,7 @@ template <class ELFT> void GNUELFDumper<ELFT>::printCGProfile() {
   OS << "GNUStyle::printCGProfile not implemented\n";
 }
 
-template <class ELFT> void GNUELFDumper<ELFT>::printBBAddrMaps() {
+template <class ELFT> void GNUELFDumper<ELFT>::printBBAddrMaps(bool) {
   OS << "GNUStyle::printBBAddrMaps not implemented\n";
 }
 
@@ -7521,7 +7521,8 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printCGProfile() {
   }
 }
 
-template <class ELFT> void LLVMELFDumper<ELFT>::printBBAddrMaps() {
+template <class ELFT>
+void LLVMELFDumper<ELFT>::printBBAddrMaps(bool PrettyPGOAnalysis) {
   bool IsRelocatable = this->Obj.getHeader().e_type == ELF::ET_REL;
   using Elf_Shdr = typename ELFT::Shdr;
   auto IsMatch = [](const Elf_Shdr &Sec) -> bool {
@@ -7600,21 +7601,28 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printBBAddrMaps() {
           for (const PGOAnalysisMap::PGOBBEntry &PBBE : PAM.BBEntries) {
             DictScope L(W);
 
-            /// FIXME: currently we just emit the raw frequency, it may be
-            /// better to provide an option to scale it by the first entry
-            /// frequence using BlockFrequency::Scaled64 number
-            if (PAM.FeatEnable.BBFreq)
-              W.printNumber("Frequency", PBBE.BlockFreq.getFrequency());
+            if (PAM.FeatEnable.BBFreq) {
+              if (PrettyPGOAnalysis) {
+                std::string BlockFreqStr;
+                raw_string_ostream SS(BlockFreqStr);
+                printRelativeBlockFreq(SS, PAM.BBEntries.front().BlockFreq,
+                                       PBBE.BlockFreq);
+                W.printString("Frequency", BlockFreqStr);
+              } else {
+                W.printNumber("Frequency", PBBE.BlockFreq.getFrequency());
+              }
+            }
 
             if (PAM.FeatEnable.BrProb) {
               ListScope L(W, "Successors");
               for (const auto &Succ : PBBE.Successors) {
                 DictScope L(W);
                 W.printNumber("ID", Succ.ID);
-                /// FIXME: currently we just emit the raw numerator of the
-                /// probably, it may be better to provide an option to emit it
-                /// as a percentage or other prettied representation
-                W.printHex("Probability", Succ.Prob.getNumerator());
+                if (PrettyPGOAnalysis) {
+                  W.printObject("Probability", Succ.Prob);
+                } else {
+                  W.printHex("Probability", Succ.Prob.getNumerator());
+                }
               }
             }
           }
diff --git a/llvm/tools/llvm-readobj/ObjDumper.h b/llvm/tools/llvm-readobj/ObjDumper.h
index 1d679453581bc8..ae51019fa30bbb 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.h
+++ b/llvm/tools/llvm-readobj/ObjDumper.h
@@ -129,7 +129,9 @@ class ObjDumper {
   virtual void printGroupSections() {}
   virtual void printHashHistograms() {}
   virtual void printCGProfile() {}
-  virtual void printBBAddrMaps() {}
+  // If PrettyPGOAnalysis is true, prints BFI as relative frequency and BPI as
+  // percentage. Otherwise raw values are displayed.
+  virtual void printBBAddrMaps(bool PrettyPGOAnalysis) {}
   virtual void printAddrsig() {}
   virtual void printNotes() {}
   virtual void printELFLinkerOptions() {}
diff --git a/llvm/tools/llvm-readobj/Opts.td b/llvm/tools/llvm-readobj/Opts.td
index e2d93c6ec229e9..765a43fdef7cef 100644
--- a/llvm/tools/llvm-readobj/Opts.td
+++ b/llvm/tools/llvm-readobj/Opts.td
@@ -19,6 +19,7 @@ def all : FF<"all", "Equivalent to setting: --file-header, --program-headers, --
              "--section-groups and --histogram">;
 def arch_specific : FF<"arch-specific", "Display architecture-specific information">;
 def bb_addr_map : FF<"bb-addr-map", "Display the BB address map section">;
+def pretty_pgo_analysis_map : FF<"pretty-pgo-analysis-map", "Display PGO analysis values with formatting rather than raw numbers">;
 def cg_profile : FF<"cg-profile", "Display call graph profile section">;
 defm demangle : BB<"demangle", "Demangle symbol names", "Do not demangle symbol names (default)">;
 def dependent_libraries : FF<"dependent-libraries", "Display the dependent libraries section">;
diff --git a/llvm/tools/llvm-readobj/llvm-readobj.cpp b/llvm/tools/llvm-readobj/llvm-readobj.cpp
index f9d605d35244bf..ce2c332529581a 100644
--- a/llvm/tools/llvm-readobj/llvm-readobj.cpp
+++ b/llvm/tools/llvm-readobj/llvm-readobj.cpp
@@ -95,6 +95...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 20, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Micah Weston (red1bluelost)

Changes

Primary change is to add a flag --pretty-pgo-analysis-map to llvm-readobj and llvm-objdump that prints block frequencies and branch probabilities in the same manner as BFI and BPI respectively. This can be helpful if you are manually inspecting the outputs from the tools.

In order to print, I moved the printBlockFreqImpl function from Analysis to Support and renamed it to printRelativeBlockFreq.


Patch is 21.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82292.diff

13 Files Affected:

  • (modified) llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h (-3)
  • (modified) llvm/include/llvm/Support/BlockFrequency.h (+4)
  • (modified) llvm/lib/Analysis/BlockFrequencyInfo.cpp (+1-1)
  • (modified) llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp (-15)
  • (modified) llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp (+1-1)
  • (modified) llvm/lib/Support/BlockFrequency.cpp (+17)
  • (modified) llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml (+25-9)
  • (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test (+15-7)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+21-7)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+21-13)
  • (modified) llvm/tools/llvm-readobj/ObjDumper.h (+3-1)
  • (modified) llvm/tools/llvm-readobj/Opts.td (+1)
  • (modified) llvm/tools/llvm-readobj/llvm-readobj.cpp (+8-1)
diff --git a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
index 8acb75e8725410c..4aa922635c374e4 100644
--- a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
+++ b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
@@ -539,9 +539,6 @@ class BlockFrequencyInfoImplBase {
   }
 };
 
-void printBlockFreqImpl(raw_ostream &OS, BlockFrequency EntryFreq,
-                        BlockFrequency Freq);
-
 namespace bfi_detail {
 
 template <class BlockT> struct TypeMap {};
diff --git a/llvm/include/llvm/Support/BlockFrequency.h b/llvm/include/llvm/Support/BlockFrequency.h
index 8b172ee486aab85..aeab99615a951a2 100644
--- a/llvm/include/llvm/Support/BlockFrequency.h
+++ b/llvm/include/llvm/Support/BlockFrequency.h
@@ -19,6 +19,7 @@
 
 namespace llvm {
 
+class raw_ostream;
 class BranchProbability;
 
 // This class represents Block Frequency as a 64-bit value.
@@ -119,6 +120,9 @@ class BlockFrequency {
   }
 };
 
+void printRelativeBlockFreq(raw_ostream &OS, BlockFrequency EntryFreq,
+                            BlockFrequency Freq);
+
 } // namespace llvm
 
 #endif
diff --git a/llvm/lib/Analysis/BlockFrequencyInfo.cpp b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
index 96c9bfa0e372ced..ebad8388cbe4102 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfo.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
@@ -284,7 +284,7 @@ void BlockFrequencyInfo::verifyMatch(BlockFrequencyInfo &Other) const {
 Printable llvm::printBlockFreq(const BlockFrequencyInfo &BFI,
                                BlockFrequency Freq) {
   return Printable([&BFI, Freq](raw_ostream &OS) {
-    printBlockFreqImpl(OS, BFI.getEntryFreq(), Freq);
+    printRelativeBlockFreq(OS, BFI.getEntryFreq(), Freq);
   });
 }
 
diff --git a/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp b/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
index ae08d56ef098a75..9f6e53ba15b6a62 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
@@ -634,21 +634,6 @@ BlockFrequencyInfoImplBase::getLoopName(const LoopData &Loop) const {
   return getBlockName(Loop.getHeader()) + (Loop.isIrreducible() ? "**" : "*");
 }
 
-void llvm::printBlockFreqImpl(raw_ostream &OS, BlockFrequency EntryFreq,
-                              BlockFrequency Freq) {
-  if (Freq == BlockFrequency(0)) {
-    OS << "0";
-    return;
-  }
-  if (EntryFreq == BlockFrequency(0)) {
-    OS << "<invalid BFI>";
-    return;
-  }
-  Scaled64 Block(Freq.getFrequency(), 0);
-  Scaled64 Entry(EntryFreq.getFrequency(), 0);
-  OS << Block / Entry;
-}
-
 void IrreducibleGraph::addNodesInLoop(const BFIBase::LoopData &OuterLoop) {
   Start = OuterLoop.getHeader();
   Nodes.reserve(OuterLoop.Nodes.size());
diff --git a/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp b/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
index 7ee72e21442630f..cbebdd87398e4bb 100644
--- a/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
+++ b/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
@@ -280,7 +280,7 @@ BlockFrequency MachineBlockFrequencyInfo::getEntryFreq() const {
 Printable llvm::printBlockFreq(const MachineBlockFrequencyInfo &MBFI,
                                BlockFrequency Freq) {
   return Printable([&MBFI, Freq](raw_ostream &OS) {
-    printBlockFreqImpl(OS, MBFI.getEntryFreq(), Freq);
+    printRelativeBlockFreq(OS, MBFI.getEntryFreq(), Freq);
   });
 }
 
diff --git a/llvm/lib/Support/BlockFrequency.cpp b/llvm/lib/Support/BlockFrequency.cpp
index 329f1e12cdc29b2..7d5498e7cb9974b 100644
--- a/llvm/lib/Support/BlockFrequency.cpp
+++ b/llvm/lib/Support/BlockFrequency.cpp
@@ -13,6 +13,8 @@
 #include "llvm/Support/BlockFrequency.h"
 #include "llvm/Support/BranchProbability.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/ScaledNumber.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
@@ -45,3 +47,18 @@ std::optional<BlockFrequency> BlockFrequency::mul(uint64_t Factor) const {
     return {};
   return BlockFrequency(ResultFrequency);
 }
+
+void llvm::printRelativeBlockFreq(raw_ostream &OS, BlockFrequency EntryFreq,
+                                  BlockFrequency Freq) {
+  if (Freq == BlockFrequency(0)) {
+    OS << "0";
+    return;
+  }
+  if (EntryFreq == BlockFrequency(0)) {
+    OS << "<invalid BFI>";
+    return;
+  }
+  ScaledNumber<uint64_t> Block(Freq.getFrequency(), 0);
+  ScaledNumber<uint64_t> Entry(EntryFreq.getFrequency(), 0);
+  OS << Block / Entry;
+}
diff --git a/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml b/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
index 732fab3e2a378cd..8ac9b1434aaef85 100644
--- a/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
+++ b/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
@@ -48,6 +48,8 @@ Symbols:
 # RUN: yaml2obj %s --docnum=2 -o %t2
 # RUN: llvm-objdump %t2 -d --symbolize-operands --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --check-prefix=ENTRYCOUNT-BLOCKFREQ
+# RUN: llvm-objdump %t2 -d --symbolize-operands --pretty-pgo-analysis-map --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s --check-prefix=ENTRYCOUNT-BLOCKFREQ-PRETTY
 
 --- !ELF
 FileHeader:
@@ -104,12 +106,20 @@ Symbols:
 # ENTRYCOUNT-BLOCKFREQ: <BB2> (Frequency: 18):
 # ENTRYCOUNT-BLOCKFREQ: <BB5> (Frequency: 1000):
 
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <foo>:
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB3> (Entry count: 1000, Frequency: 1.0):
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB1> (Frequency: 0.133):
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB2> (Frequency: 0.018):
+# ENTRYCOUNT-BLOCKFREQ-PRETTY: <BB5> (Frequency: 1.0):
+
 ## Check the case where we have entry points, block frequency, and branch
 ## proabability information.
 
 # RUN: yaml2obj %s --docnum=3 -o %t3
 # RUN: llvm-objdump %t3 -d --symbolize-operands --no-show-raw-insn --no-leading-addr | \
 # RUN:   FileCheck %s --check-prefix=ENTRY-FREQ-PROB
+# RUN: llvm-objdump %t3 -d --symbolize-operands --pretty-pgo-analysis-map --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s --check-prefix=ENTRY-FREQ-PROB-PRETTY
 
 --- !ELF
 FileHeader:
@@ -154,21 +164,21 @@ Sections:
           - BBFreq: 1000
             Successors:
             - ID:          1
-              BrProb:      0x22222222
+              BrProb:      0x10000000
             - ID:          2
-              BrProb:      0x33333333
+              BrProb:      0x15000000
             - ID:          3
-              BrProb:      0xaaaaaaaa
+              BrProb:      0x50000000
           - BBFreq: 133
             Successors:
             - ID:          2
-              BrProb:      0x11111111
+              BrProb:      0x10000000
             - ID:          3
-              BrProb:      0xeeeeeeee
+              BrProb:      0x70000000
           - BBFreq: 18
             Successors:
             - ID:          3
-              BrProb:      0xffffffff
+              BrProb:      0x80000000
           - BBFreq: 1000
             Successors:    []
 Symbols:
@@ -177,7 +187,13 @@ Symbols:
     Value:   0x0
 
 # ENTRY-FREQ-PROB: <foo>:
-# ENTRY-FREQ-PROB: <BB3> (Entry count: 1000, Frequency: 1000, Successors: BB1:22222222, BB2:33333333, BB3:aaaaaaaa):
-# ENTRY-FREQ-PROB: <BB1> (Frequency: 133, Successors: BB2:11111111, BB3:eeeeeeee):
-# ENTRY-FREQ-PROB: <BB2> (Frequency: 18, Successors: BB3:ffffffff):
+# ENTRY-FREQ-PROB: <BB3> (Entry count: 1000, Frequency: 1000, Successors: BB1:10000000, BB2:15000000, BB3:50000000):
+# ENTRY-FREQ-PROB: <BB1> (Frequency: 133, Successors: BB2:10000000, BB3:70000000):
+# ENTRY-FREQ-PROB: <BB2> (Frequency: 18, Successors: BB3:80000000):
 # ENTRY-FREQ-PROB: <BB5> (Frequency: 1000):
+
+# ENTRY-FREQ-PROB-PRETTY: <foo>:
+# ENTRY-FREQ-PROB-PRETTY: <BB3> (Entry count: 1000, Frequency: 1.0, Successors: BB1:[0x10000000 / 0x80000000 = 12.50%], BB2:[0x15000000 / 0x80000000 = 16.41%], BB3:[0x50000000 / 0x80000000 = 62.50%]):
+# ENTRY-FREQ-PROB-PRETTY: <BB1> (Frequency: 0.133, Successors: BB2:[0x10000000 / 0x80000000 = 12.50%], BB3:[0x70000000 / 0x80000000 = 87.50%]):
+# ENTRY-FREQ-PROB-PRETTY: <BB2> (Frequency: 0.018, Successors: BB3:[0x80000000 / 0x80000000 = 100.00%]):
+# ENTRY-FREQ-PROB-PRETTY: <BB5> (Frequency: 1.0):
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
index e5a9400c670c006..1ad5d5f76112682 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-pgo-analysis-map.test
@@ -3,12 +3,14 @@
 
 ## Check 64-bit:
 # RUN: yaml2obj %s -DBITS=64 -DADDR=0x999999999 -o %t1.x64.o
-# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefix=CHECK
+# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,RAW
+# RUN: llvm-readobj %t1.x64.o --bb-addr-map --pretty-pgo-analysis-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,PRETTY
 # RUN: llvm-readelf %t1.x64.o --bb-addr-map | FileCheck %s --check-prefix=GNU
+# RUN: llvm-readobj %t1.x64.o --pretty-pgo-analysis-map 2>&1 | FileCheck %s --check-prefix=PRETTY-NO-BAM
 
 ## Check 32-bit:
 # RUN: yaml2obj %s -DBITS=32 -o %t1.x32.o
-# RUN: llvm-readobj %t1.x32.o --bb-addr-map 2>&1 | FileCheck -DADDR=0x11111 %s -DFILE=%t1.x32.o --check-prefix=CHECK
+# RUN: llvm-readobj %t1.x32.o --bb-addr-map 2>&1 | FileCheck -DADDR=0x11111 %s -DFILE=%t1.x32.o --check-prefixes=CHECK,RAW
 # RUN: llvm-readelf %t1.x32.o --bb-addr-map | FileCheck %s --check-prefix=GNU
 
 ## Check that a malformed section can be handled.
@@ -55,16 +57,19 @@
 # CHECK-NEXT:       FuncEntryCount: 100
 # CHECK-NEXT:       PGO BB entries [
 # CHECK-NEXT:         {
-# CHECK-NEXT:           Frequency: 100
+# RAW-NEXT:             Frequency: 100
+# PRETTY-NEXT:          Frequency: 1.0
 # CHECK-NEXT:           Successors [
 # CHECK-NEXT:             {
 # CHECK-NEXT:               ID: 2
-# CHECK-NEXT:               Probability: 0xFFFFFFFF
+# RAW-NEXT:                 Probability: 0x80000000
+# PRETTY-NEXT:              Probability: 0x80000000 / 0x80000000 = 100.00%
 # CHECK-NEXT:             }
 # CHECK-NEXT:           ]
 # CHECK-NEXT:         }
 # CHECK-NEXT:         {
-# CHECK-NEXT:           Frequency: 100
+# RAW-NEXT:             Frequency: 100
+# PRETTY-NEXT:          Frequency: 1.0
 # CHECK-NEXT:           Successors [
 # CHECK-NEXT:           ]
 # CHECK-NEXT:         }
@@ -95,7 +100,8 @@
 # CHECK-NEXT:       FuncEntryCount: 8888
 # CHECK-NEXT:       PGO BB entries [
 # CHECK-NEXT:         {
-# CHECK-NEXT:           Frequency: 9000
+# RAW-NEXT:             Frequency: 9000
+# PRETTY-NEXT:          Frequency: 1.0
 # CHECK-NEXT:         }
 # CHECK-NEXT:       ]
 # CHECK-NEXT:     }
@@ -104,6 +110,8 @@
 
 # GNU: GNUStyle::printBBAddrMaps not implemented
 
+# PRETTY-NO-BAM: warning: --bb-addr-map must be enabled for --pretty-pgo-analysis-map to have an effect
+
 # TRUNCATED:      BBAddrMap [
 # TRUNCATED-NEXT:   warning: '[[FILE]]': unable to dump SHT_LLVM_BB_ADDR_MAP section with index 3: unable to decode LEB128 at offset [[OFFSET]]: malformed uleb128, extends past end
 # TRUNCATED-NEXT: ]
@@ -192,7 +200,7 @@ Sections:
           - BBFreq:        100
             Successors:
               - ID:        2
-                BrProb:    0xFFFFFFFF
+                BrProb:    0x80000000
           - BBFreq:        100
             Successors:    []
       - FuncEntryCount: 8888
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 948a5d74e1ab2b9..7a398e35a8eb538 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -188,8 +188,10 @@ class BBAddrMapFunctionEntry {
   const BBAddrMap &getAddrMap() const { return AddrMap; }
 
   // Returns the PGO string associated with the entry of index `PGOBBEntryIndex`
-  // in `PGOMap`.
-  std::string constructPGOLabelString(size_t PGOBBEntryIndex) const {
+  // in `PGOMap`. If PrettyPGOAnalysis is true, prints BFI as relative frequency
+  // and BPI as percentage. Otherwise raw values are displayed.
+  std::string constructPGOLabelString(size_t PGOBBEntryIndex,
+                                      bool PrettyPGOAnalysis) const {
     if (!PGOMap.FeatEnable.hasPGOAnalysis())
       return "";
     std::string PGOString;
@@ -211,7 +213,13 @@ class BBAddrMapFunctionEntry {
           PGOMap.BBEntries[PGOBBEntryIndex];
 
       if (PGOMap.FeatEnable.BBFreq) {
-        PGOSS << "Frequency: " << Twine(PGOBBEntry.BlockFreq.getFrequency());
+        PGOSS << "Frequency: ";
+        if (PrettyPGOAnalysis) {
+          printRelativeBlockFreq(PGOSS, PGOMap.BBEntries.front().BlockFreq,
+                                 PGOBBEntry.BlockFreq);
+        } else {
+          PGOSS << Twine(PGOBBEntry.BlockFreq.getFrequency());
+        }
         if (PGOMap.FeatEnable.BrProb && PGOBBEntry.Successors.size() > 0) {
           PGOSS << ", ";
         }
@@ -220,9 +228,13 @@ class BBAddrMapFunctionEntry {
         PGOSS << "Successors: ";
         interleaveComma(
             PGOBBEntry.Successors, PGOSS,
-            [&PGOSS](const PGOAnalysisMap::PGOBBEntry::SuccessorEntry &SE) {
+            [&](const PGOAnalysisMap::PGOBBEntry::SuccessorEntry &SE) {
               PGOSS << "BB" << SE.ID << ":";
-              PGOSS.write_hex(SE.Prob.getNumerator());
+              if (PrettyPGOAnalysis) {
+                PGOSS << "[" << SE.Prob << "]";
+              } else {
+                PGOSS.write_hex(SE.Prob.getNumerator());
+              }
             });
       }
     }
@@ -331,6 +343,7 @@ static bool HasStopAddressFlag;
 
 bool objdump::SymbolTable;
 static bool SymbolizeOperands;
+static bool PrettyPGOAnalysisMap;
 static bool DynamicSymbolTable;
 std::string objdump::TripleName;
 bool objdump::UnwindInfo;
@@ -1410,8 +1423,8 @@ static void collectBBAddrMapLabels(
 
     std::string LabelString = ("BB" + Twine(BBEntry.ID)).str();
     Labels[BBAddress].push_back(
-        {LabelString,
-         FunctionMap->constructPGOLabelString(NumBBEntriesBeforeRange + I)});
+        {LabelString, FunctionMap->constructPGOLabelString(
+                          NumBBEntriesBeforeRange + I, PrettyPGOAnalysisMap)});
   }
 }
 
@@ -3473,6 +3486,7 @@ static void parseObjdumpOptions(const llvm::opt::InputArgList &InputArgs) {
   HasStopAddressFlag = InputArgs.hasArg(OBJDUMP_stop_address_EQ);
   SymbolTable = InputArgs.hasArg(OBJDUMP_syms);
   SymbolizeOperands = InputArgs.hasArg(OBJDUMP_symbolize_operands);
+  PrettyPGOAnalysisMap = InputArgs.hasArg(OBJDUMP_pretty_pgo_analysis_map);
   DynamicSymbolTable = InputArgs.hasArg(OBJDUMP_dynamic_syms);
   TripleName = InputArgs.getLastArgValue(OBJDUMP_triple_EQ).str();
   UnwindInfo = InputArgs.hasArg(OBJDUMP_unwind_info);
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 50ea63e87a43be5..9710fcf14430e77 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -593,7 +593,7 @@ template <typename ELFT> class GNUELFDumper : public ELFDumper<ELFT> {
   void printVersionDefinitionSection(const Elf_Shdr *Sec) override;
   void printVersionDependencySection(const Elf_Shdr *Sec) override;
   void printCGProfile() override;
-  void printBBAddrMaps() override;
+  void printBBAddrMaps(bool PrettyPGOAnalysis) override;
   void printAddrsig() override;
   void printNotes() override;
   void printELFLinkerOptions() override;
@@ -704,7 +704,7 @@ template <typename ELFT> class LLVMELFDumper : public ELFDumper<ELFT> {
   void printVersionDefinitionSection(const Elf_Shdr *Sec) override;
   void printVersionDependencySection(const Elf_Shdr *Sec) override;
   void printCGProfile() override;
-  void printBBAddrMaps() override;
+  void printBBAddrMaps(bool PrettyPGOAnalysis) override;
   void printAddrsig() override;
   void printNotes() override;
   void printELFLinkerOptions() override;
@@ -5031,7 +5031,7 @@ template <class ELFT> void GNUELFDumper<ELFT>::printCGProfile() {
   OS << "GNUStyle::printCGProfile not implemented\n";
 }
 
-template <class ELFT> void GNUELFDumper<ELFT>::printBBAddrMaps() {
+template <class ELFT> void GNUELFDumper<ELFT>::printBBAddrMaps(bool) {
   OS << "GNUStyle::printBBAddrMaps not implemented\n";
 }
 
@@ -7521,7 +7521,8 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printCGProfile() {
   }
 }
 
-template <class ELFT> void LLVMELFDumper<ELFT>::printBBAddrMaps() {
+template <class ELFT>
+void LLVMELFDumper<ELFT>::printBBAddrMaps(bool PrettyPGOAnalysis) {
   bool IsRelocatable = this->Obj.getHeader().e_type == ELF::ET_REL;
   using Elf_Shdr = typename ELFT::Shdr;
   auto IsMatch = [](const Elf_Shdr &Sec) -> bool {
@@ -7600,21 +7601,28 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printBBAddrMaps() {
           for (const PGOAnalysisMap::PGOBBEntry &PBBE : PAM.BBEntries) {
             DictScope L(W);
 
-            /// FIXME: currently we just emit the raw frequency, it may be
-            /// better to provide an option to scale it by the first entry
-            /// frequence using BlockFrequency::Scaled64 number
-            if (PAM.FeatEnable.BBFreq)
-              W.printNumber("Frequency", PBBE.BlockFreq.getFrequency());
+            if (PAM.FeatEnable.BBFreq) {
+              if (PrettyPGOAnalysis) {
+                std::string BlockFreqStr;
+                raw_string_ostream SS(BlockFreqStr);
+                printRelativeBlockFreq(SS, PAM.BBEntries.front().BlockFreq,
+                                       PBBE.BlockFreq);
+                W.printString("Frequency", BlockFreqStr);
+              } else {
+                W.printNumber("Frequency", PBBE.BlockFreq.getFrequency());
+              }
+            }
 
             if (PAM.FeatEnable.BrProb) {
               ListScope L(W, "Successors");
               for (const auto &Succ : PBBE.Successors) {
                 DictScope L(W);
                 W.printNumber("ID", Succ.ID);
-                /// FIXME: currently we just emit the raw numerator of the
-                /// probably, it may be better to provide an option to emit it
-                /// as a percentage or other prettied representation
-                W.printHex("Probability", Succ.Prob.getNumerator());
+                if (PrettyPGOAnalysis) {
+                  W.printObject("Probability", Succ.Prob);
+                } else {
+                  W.printHex("Probability", Succ.Prob.getNumerator());
+                }
               }
             }
           }
diff --git a/llvm/tools/llvm-readobj/ObjDumper.h b/llvm/tools/llvm-readobj/ObjDumper.h
index 1d679453581bc84..ae51019fa30bbb7 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.h
+++ b/llvm/tools/llvm-readobj/ObjDumper.h
@@ -129,7 +129,9 @@ class ObjDumper {
   virtual void printGroupSections() {}
   virtual void printHashHistograms() {}
   virtual void printCGProfile() {}
-  virtual void printBBAddrMaps() {}
+  // If PrettyPGOAnalysis is true, prints BFI as relative frequency and BPI as
+  // percentage. Otherwise raw values are displayed.
+  virtual void printBBAddrMaps(bool PrettyPGOAnalysis) {}
   virtual void printAddrsig() {}
   virtual void printNotes() {}
   virtual void printELFLinkerOptions() {}
diff --git a/llvm/tools/llvm-readobj/Opts.td b/llvm/tools/llvm-readobj/Opts.td
index e2d93c6ec229e9f..765a43fdef7cef8 100644
--- a/llvm/tools/llvm-readobj/Opts.td
+++ b/llvm/tools/llvm-readobj/Opts.td
@@ -19,6 +19,7 @@ def all : FF<"all", "Equivalent to setting: --file-header, --program-headers, --
              "--section-groups and --histogram">;
 def arch_specific : FF<"arch-specific", "Display architecture-specific information">;
 def bb_addr_map : FF<"bb-addr-map", "Display the BB address map section">;
+def pretty_pgo_analysis_map : FF<"pretty-pgo-analysis-map", "Display PGO analysis values with formatting rather than raw numbers">;
 def cg_profile : FF<"cg-profile", "Display call graph profile section">;
 defm demangle : BB<"demangle", "Demangle symbol names", "Do not demangle symbol names (default)">;
 def dependent_libraries : FF<"dependent-libraries", "Display the dependent libraries section">;
diff --git a/llvm/tools/llvm-readobj/llvm-readobj.cpp b/llvm/tools/llvm-readobj/llvm-readobj.cpp
index f9d605d35244bf3..ce2c332529581ac 100644
--- a/llvm/tools/llvm-readobj/llvm-readobj.cpp
+++ b/llvm/tools/llvm-readobj/llv...
[truncated]

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buildkite is failing to build this change.

Also, you need to add new options to the command-guide help in llvm/docs/CommandGuide for each tool.

llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml Outdated Show resolved Hide resolved
llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml Outdated Show resolved Hide resolved
@@ -3,12 +3,14 @@

## Check 64-bit:
# RUN: yaml2obj %s -DBITS=64 -DADDR=0x999999999 -o %t1.x64.o
# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefix=CHECK
# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,RAW
# RUN: llvm-readobj %t1.x64.o --bb-addr-map --pretty-pgo-analysis-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,PRETTY
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same coomment as above re. --strict-whitespace etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For readobj, full line matching and strict whitespace is less needed since the structure formatting is done by ScopedPrinters, rather than manual like objdump. The braces and bracket are good enough to validate the fields are printed in the right spot and then trust the spacing is accurate from ScopedPrinters tests. There is a warning that we test for which throws off a full line match since it contains the readobj absolute path which varies by host test machine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The braces and bracket are good enough to validate the fields are printed in the right spot and then trust the spacing is accurate from ScopedPrinters tests.

In this context, I'm actually more interested in checking that there isn't anything on the line that is unexpected (e.g. garbage data etc). --match-full-lines on its own would probably suffice in this case.

There is a warning that we test for which throws off a full line match since it contains the readobj absolute path which varies by host test machine.

FWIW, this is easily dealt with by adding {{.*}} to the start of the check for the warning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, I added match-full-lines to these tests.

llvm/tools/llvm-objdump/llvm-objdump.cpp Outdated Show resolved Hide resolved
llvm/tools/llvm-readobj/ELFDumper.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@red1bluelost, it looks like you missed my out-of-line comments too? Buildkite is still failing and the docs still haven't been updated?

@@ -3,12 +3,14 @@

## Check 64-bit:
# RUN: yaml2obj %s -DBITS=64 -DADDR=0x999999999 -o %t1.x64.o
# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefix=CHECK
# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,RAW
# RUN: llvm-readobj %t1.x64.o --bb-addr-map --pretty-pgo-analysis-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefixes=CHECK,PRETTY
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The braces and bracket are good enough to validate the fields are printed in the right spot and then trust the spacing is accurate from ScopedPrinters tests.

In this context, I'm actually more interested in checking that there isn't anything on the line that is unexpected (e.g. garbage data etc). --match-full-lines on its own would probably suffice in this case.

There is a warning that we test for which throws off a full line match since it contains the readobj absolute path which varies by host test machine.

FWIW, this is easily dealt with by adding {{.*}} to the start of the check for the warning.

@red1bluelost
Copy link
Member Author

@red1bluelost, it looks like you missed my out-of-line comments too? Buildkite is still failing and the docs still haven't been updated?

Woops, I did indeed forget after reading your comment. Thanks for reminding! Added documentation and a fix that should clear buildkite.

When a bb-address-map section is present (i.e., the object file is built with ``-fbasic-block-sections=labels``), labels are retrieved from that section instead.
When a bb-address-map section is present (i.e., the object file is built with
``-fbasic-block-sections=labels``), labels are retrieved from that section
instead. If a pgo-analysis-map present along side the bb-address-map, any
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
instead. If a pgo-analysis-map present along side the bb-address-map, any
instead. If a pgo-analysis-map is present alongside the bb-address-map, any

Two nits.

.. option:: --pretty-pgo-analysis-map

When using ``--symbolize-operands`` with bb-address-map and pgo-analysis-map,
print analyses using same format as there analysis passes would. An example
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
print analyses using same format as there analysis passes would. An example
print analyses using the same format as their analysis passes would. An example

Comment on lines 303 to 304
of pretty format, would be printing block frequencies relative to the entry
block, same as BFI.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
of pretty format, would be printing block frequencies relative to the entry
block, same as BFI.
of pretty format would be printing block frequencies relative to the entry
block, the same as BFI.

@@ -291,6 +296,15 @@ OPTIONS
cmp eax, dword ptr <g>
jge <L0>

.. option:: --pretty-pgo-analysis-map

When using ``--symbolize-operands`` with bb-address-map and pgo-analysis-map,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When using ``--symbolize-operands`` with bb-address-map and pgo-analysis-map,
When using :option:``--symbolize-operands`` with bb-address-map and pgo-analysis-map,

Also applies below - this will ensure the reference becomes a hyperlink to the actual option in the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion!

analyses with special formats (i.e. BlockFrequency, BranchProbability, etc)
are printed using the same format as there respective analysis pass.

Requires ``--bb-addr-map`` to have an effect.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Requires ``--bb-addr-map`` to have an effect.
Requires :option:``--bb-addr-map`` to have an effect.

@@ -159,6 +159,17 @@ The following options are implemented only for the ELF file format.
Display the contents of the basic block address map section(s), which contain the
address of each function, along with the relative offset of each basic block.

When pgo analysis maps are present, all analyses are printed as there raw
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When pgo analysis maps are present, all analyses are printed as there raw
When pgo analysis maps are present, all analyses are printed as their raw


When pgo analysis maps are present in the basic block address map section(s),
analyses with special formats (i.e. BlockFrequency, BranchProbability, etc)
are printed using the same format as there respective analysis pass.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
are printed using the same format as there respective analysis pass.
are printed using the same format as their respective analysis pass.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with one remaining nit.

of pretty format, would be printing block frequencies relative to the entry
block, same as BFI.
When using :option:`--symbolize-operands` with bb-address-map and
pgo-analysis-map, print analyses using same the format as their analysis
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pgo-analysis-map, print analyses using same the format as their analysis
pgo-analysis-map, print analyses using the same format as their analysis

@red1bluelost red1bluelost merged commit 9ca8db3 into llvm:main Feb 27, 2024
4 of 5 checks passed
@red1bluelost red1bluelost deleted the pgo-analysis-map-pretty-print branch March 20, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants