BlockFrequencyInfo: Add PrintBlockFreq helper (#67512)

- Refactor the (Machine)BlockFrequencyInfo::printBlockFreq functions
into a `PrintBlockFreq()` function returning a `Printable` object. This
simplifies usage as it can be directly piped to a `raw_ostream` like
`dbgs() << PrintBlockFreq(MBFI, Freq) << '\n';`.
- Previously there was an interesting behavior where
`BlockFrequencyInfoImpl` stores frequencies both as a `Scaled64` number
and as an `uint64_t`. Most algorithms use the `BlockFrequency`
abstraction with the integers, the print function for basic blocks
printed the `Scaled64` number potentially showing higher accuracy than
was used by the algorithm. This changes things to only print
`BlockFrequency` values.
- Replace some instances of `dbgs() << Freq.getFrequency()` with the new
function.

Author: MatzeB

llvm/include/llvm/Analysis/BlockFrequencyInfo.h

@@ -16,6 +16,7 @@
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/BlockFrequency.h"
+#include "llvm/Support/Printable.h"
 #include <cstdint>
 #include <memory>
 #include <optional>
@@ -92,14 +93,6 @@ class BlockFrequencyInfo {
   void calculate(const Function &F, const BranchProbabilityInfo &BPI,
                  const LoopInfo &LI);
 
-  // Print the block frequency Freq to OS using the current functions entry
-  // frequency to convert freq into a relative decimal form.
-  raw_ostream &printBlockFreq(raw_ostream &OS, BlockFrequency Freq) const;
-
-  // Convenience method that attempts to look up the frequency associated with
-  // BB and print it to OS.
-  raw_ostream &printBlockFreq(raw_ostream &OS, const BasicBlock *BB) const;
-
   BlockFrequency getEntryFreq() const;
   void releaseMemory();
   void print(raw_ostream &OS) const;
@@ -108,6 +101,15 @@ class BlockFrequencyInfo {
   void verifyMatch(BlockFrequencyInfo &Other) const;
 };
 
+/// Print the block frequency @p Freq relative to the current functions entry
+/// frequency. Returns a Printable object that can be piped via `<<` to a
+/// `raw_ostream`.
+Printable printBlockFreq(const BlockFrequencyInfo &BFI, BlockFrequency Freq);
+
+/// Convenience function equivalent to calling
+/// `printBlockFreq(BFI, BFI.getBlocakFreq(&BB))`.
+Printable printBlockFreq(const BlockFrequencyInfo &BFI, const BasicBlock &BB);
+
 /// Analysis pass which computes \c BlockFrequencyInfo.
 class BlockFrequencyAnalysis
     : public AnalysisInfoMixin<BlockFrequencyAnalysis> {

llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h

@@ -533,15 +533,15 @@ class BlockFrequencyInfoImplBase {
 
   void setBlockFreq(const BlockNode &Node, BlockFrequency Freq);
 
-  raw_ostream &printBlockFreq(raw_ostream &OS, const BlockNode &Node) const;
-  raw_ostream &printBlockFreq(raw_ostream &OS, BlockFrequency Freq) const;
-
   BlockFrequency getEntryFreq() const {
     assert(!Freqs.empty());
     return BlockFrequency(Freqs[0].Integer);
   }
 };
 
+void printBlockFreqImpl(raw_ostream &OS, BlockFrequency EntryFreq,
+                        BlockFrequency Freq);
+
 namespace bfi_detail {
 
 template <class BlockT> struct TypeMap {};
@@ -1067,11 +1067,6 @@ template <class BT> class BlockFrequencyInfoImpl : BlockFrequencyInfoImplBase {
   raw_ostream &print(raw_ostream &OS) const override;
 
   using BlockFrequencyInfoImplBase::dump;
-  using BlockFrequencyInfoImplBase::printBlockFreq;
-
-  raw_ostream &printBlockFreq(raw_ostream &OS, const BlockT *BB) const {
-    return BlockFrequencyInfoImplBase::printBlockFreq(OS, getNode(BB));
-  }
 
   void verifyMatch(BlockFrequencyInfoImpl<BT> &Other) const;
 };
@@ -1862,7 +1857,7 @@ struct BFIDOTGraphTraitsBase : public DefaultDOTGraphTraits {
       OS << Node->getName() << " : ";
     switch (GType) {
     case GVDT_Fraction:
-      Graph->printBlockFreq(OS, Node);
+      OS << printBlockFreq(*Graph, *Node);
       break;
     case GVDT_Integer:
       OS << Graph->getBlockFreq(Node).getFrequency();

llvm/include/llvm/CodeGen/MBFIWrapper.h

@@ -16,7 +16,6 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/BlockFrequency.h"
-#include "llvm/Support/raw_ostream.h"
 #include <optional>
 
 namespace llvm {
@@ -33,14 +32,11 @@ class MBFIWrapper {
   std::optional<uint64_t>
   getBlockProfileCount(const MachineBasicBlock *MBB) const;
 
-  raw_ostream &printBlockFreq(raw_ostream &OS,
-                              const MachineBasicBlock *MBB) const;
-  raw_ostream &printBlockFreq(raw_ostream &OS, BlockFrequency Freq) const;
   void view(const Twine &Name, bool isSimple = true);
   BlockFrequency getEntryFreq() const;
-  const MachineBlockFrequencyInfo &getMBFI() { return MBFI; }
+  const MachineBlockFrequencyInfo &getMBFI() const { return MBFI; }
 
- private:
+private:
   const MachineBlockFrequencyInfo &MBFI;
   DenseMap<const MachineBasicBlock *, BlockFrequency> MergedBBFreq;
 };

llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h

@@ -91,20 +91,22 @@ class MachineBlockFrequencyInfo : public MachineFunctionPass {
   /// rendered using dot.
   void view(const Twine &Name, bool isSimple = true) const;
 
-  // Print the block frequency Freq to OS using the current functions entry
-  // frequency to convert freq into a relative decimal form.
-  raw_ostream &printBlockFreq(raw_ostream &OS, const BlockFrequency Freq) const;
-
-  // Convenience method that attempts to look up the frequency associated with
-  // BB and print it to OS.
-  raw_ostream &printBlockFreq(raw_ostream &OS,
-                              const MachineBasicBlock *MBB) const;
-
   /// Divide a block's BlockFrequency::getFrequency() value by this value to
   /// obtain the entry block - relative frequency of said block.
   BlockFrequency getEntryFreq() const;
 };
 
+/// Print the block frequency @p Freq relative to the current functions entry
+/// frequency. Returns a Printable object that can be piped via `<<` to a
+/// `raw_ostream`.
+Printable printBlockFreq(const MachineBlockFrequencyInfo &MBFI,
+                         BlockFrequency Freq);
+
+/// Convenience function equivalent to calling
+/// `printBlockFreq(MBFI, MBFI.getBlockFreq(&MBB))`.
+Printable printBlockFreq(const MachineBlockFrequencyInfo &MBFI,
+                         const MachineBasicBlock &MBB);
+
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_MACHINEBLOCKFREQUENCYINFO_H

llvm/lib/Analysis/BlockFrequencyInfo.cpp

@@ -78,14 +78,13 @@ cl::opt<PGOViewCountsType> PGOViewCounts(
                clEnumValN(PGOVCT_Graph, "graph", "show a graph."),
                clEnumValN(PGOVCT_Text, "text", "show in text.")));
 
-static cl::opt<bool> PrintBlockFreq(
-    "print-bfi", cl::init(false), cl::Hidden,
-    cl::desc("Print the block frequency info."));
-
-cl::opt<std::string> PrintBlockFreqFuncName(
-    "print-bfi-func-name", cl::Hidden,
-    cl::desc("The option to specify the name of the function "
-             "whose block frequency info is printed."));
+static cl::opt<bool> PrintBFI("print-bfi", cl::init(false), cl::Hidden,
+                              cl::desc("Print the block frequency info."));
+
+cl::opt<std::string>
+    PrintBFIFuncName("print-bfi-func-name", cl::Hidden,
+                     cl::desc("The option to specify the name of the function "
+                              "whose block frequency info is printed."));
 } // namespace llvm
 
 namespace llvm {
@@ -193,9 +192,8 @@ void BlockFrequencyInfo::calculate(const Function &F,
        F.getName().equals(ViewBlockFreqFuncName))) {
     view();
   }
-  if (PrintBlockFreq &&
-      (PrintBlockFreqFuncName.empty() ||
-       F.getName().equals(PrintBlockFreqFuncName))) {
+  if (PrintBFI &&
+      (PrintBFIFuncName.empty() || F.getName().equals(PrintBFIFuncName))) {
     print(dbgs());
   }
 }
@@ -267,17 +265,6 @@ const BranchProbabilityInfo *BlockFrequencyInfo::getBPI() const {
   return BFI ? &BFI->getBPI() : nullptr;
 }
 
-raw_ostream &BlockFrequencyInfo::printBlockFreq(raw_ostream &OS,
-                                                BlockFrequency Freq) const {
-  return BFI ? BFI->printBlockFreq(OS, Freq) : OS;
-}
-
-raw_ostream &
-BlockFrequencyInfo::printBlockFreq(raw_ostream &OS,
-                                   const BasicBlock *BB) const {
-  return BFI ? BFI->printBlockFreq(OS, BB) : OS;
-}
-
 BlockFrequency BlockFrequencyInfo::getEntryFreq() const {
   return BFI ? BFI->getEntryFreq() : BlockFrequency(0);
 }
@@ -294,6 +281,18 @@ void BlockFrequencyInfo::verifyMatch(BlockFrequencyInfo &Other) const {
     BFI->verifyMatch(*Other.BFI);
 }
 
+Printable llvm::printBlockFreq(const BlockFrequencyInfo &BFI,
+                               BlockFrequency Freq) {
+  return Printable([&BFI, Freq](raw_ostream &OS) {
+    printBlockFreqImpl(OS, BFI.getEntryFreq(), Freq);
+  });
+}
+
+Printable llvm::printBlockFreq(const BlockFrequencyInfo &BFI,
+                               const BasicBlock &BB) {
+  return printBlockFreq(BFI, BFI.getBlockFreq(&BB));
+}
+
 INITIALIZE_PASS_BEGIN(BlockFrequencyInfoWrapperPass, "block-freq",
                       "Block Frequency Analysis", true, true)
 INITIALIZE_PASS_DEPENDENCY(BranchProbabilityInfoWrapperPass)

llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp

@@ -640,19 +640,19 @@ BlockFrequencyInfoImplBase::getLoopName(const LoopData &Loop) const {
   return getBlockName(Loop.getHeader()) + (Loop.isIrreducible() ? "**" : "*");
 }
 
-raw_ostream &
-BlockFrequencyInfoImplBase::printBlockFreq(raw_ostream &OS,
-                                           const BlockNode &Node) const {
-  return OS << getFloatingBlockFreq(Node);
-}
-
-raw_ostream &
-BlockFrequencyInfoImplBase::printBlockFreq(raw_ostream &OS,
-                                           BlockFrequency Freq) const {
+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(getEntryFreq().getFrequency(), 0);
-
-  return OS << Block / Entry;
+  Scaled64 Entry(EntryFreq.getFrequency(), 0);
+  OS << Block / Entry;
 }
 
 void IrreducibleGraph::addNodesInLoop(const BFIBase::LoopData &OuterLoop) {

llvm/lib/CodeGen/MBFIWrapper.cpp

@@ -43,16 +43,6 @@ MBFIWrapper::getBlockProfileCount(const MachineBasicBlock *MBB) const {
   return MBFI.getBlockProfileCount(MBB);
 }
 
-raw_ostream & MBFIWrapper::printBlockFreq(raw_ostream &OS,
-                                          const MachineBasicBlock *MBB) const {
-  return MBFI.printBlockFreq(OS, getBlockFreq(MBB));
-}
-
-raw_ostream & MBFIWrapper::printBlockFreq(raw_ostream &OS,
-                                         BlockFrequency Freq) const {
-  return MBFI.printBlockFreq(OS, Freq);
-}
-
 void MBFIWrapper::view(const Twine &Name, bool isSimple) {
   MBFI.view(Name, isSimple);
 }

llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp

@@ -75,7 +75,7 @@ static cl::opt<bool> PrintMachineBlockFreq(
 
 // Command line option to specify the name of the function for block frequency
 // dump. Defined in Analysis/BlockFrequencyInfo.cpp.
-extern cl::opt<std::string> PrintBlockFreqFuncName;
+extern cl::opt<std::string> PrintBFIFuncName;
 } // namespace llvm
 
 static GVDAGType getGVDT() {
@@ -203,8 +203,7 @@ void MachineBlockFrequencyInfo::calculate(
     view("MachineBlockFrequencyDAGS." + F.getName());
   }
   if (PrintMachineBlockFreq &&
-      (PrintBlockFreqFuncName.empty() ||
-       F.getName().equals(PrintBlockFreqFuncName))) {
+      (PrintBFIFuncName.empty() || F.getName().equals(PrintBFIFuncName))) {
     MBFI->print(dbgs());
   }
 }
@@ -274,18 +273,18 @@ const MachineBranchProbabilityInfo *MachineBlockFrequencyInfo::getMBPI() const {
   return MBFI ? &MBFI->getBPI() : nullptr;
 }
 
-raw_ostream &
-MachineBlockFrequencyInfo::printBlockFreq(raw_ostream &OS,
-                                          const BlockFrequency Freq) const {
-  return MBFI ? MBFI->printBlockFreq(OS, Freq) : OS;
+BlockFrequency MachineBlockFrequencyInfo::getEntryFreq() const {
+  return MBFI ? MBFI->getEntryFreq() : BlockFrequency(0);
 }
 
-raw_ostream &
-MachineBlockFrequencyInfo::printBlockFreq(raw_ostream &OS,
-                                          const MachineBasicBlock *MBB) const {
-  return MBFI ? MBFI->printBlockFreq(OS, MBB) : OS;
+Printable llvm::printBlockFreq(const MachineBlockFrequencyInfo &MBFI,
+                               BlockFrequency Freq) {
+  return Printable([&MBFI, Freq](raw_ostream &OS) {
+    printBlockFreqImpl(OS, MBFI.getEntryFreq(), Freq);
+  });
 }
 
-BlockFrequency MachineBlockFrequencyInfo::getEntryFreq() const {
-  return MBFI ? MBFI->getEntryFreq() : BlockFrequency(0);
+Printable llvm::printBlockFreq(const MachineBlockFrequencyInfo &MBFI,
+                               const MachineBasicBlock &MBB) {
+  return printBlockFreq(MBFI, MBFI.getBlockFreq(&MBB));
 }

llvm/lib/CodeGen/MachineBlockPlacement.cpp

@@ -1729,8 +1729,9 @@ MachineBasicBlock *MachineBlockPlacement::selectBestCandidateBlock(
            "Found CFG-violating block");
 
     BlockFrequency CandidateFreq = MBFI->getBlockFreq(MBB);
-    LLVM_DEBUG(dbgs() << "    " << getBlockName(MBB) << " -> ";
-               MBFI->printBlockFreq(dbgs(), CandidateFreq) << " (freq)\n");
+    LLVM_DEBUG(dbgs() << "    " << getBlockName(MBB) << " -> "
+                      << printBlockFreq(MBFI->getMBFI(), CandidateFreq)
+                      << " (freq)\n");
 
     // For ehpad, we layout the least probable first as to avoid jumping back
     // from least probable landingpads to more probable ones.
@@ -2095,8 +2096,8 @@ MachineBlockPlacement::findBestLoopTopHelper(
     if (Pred == L.getHeader())
       continue;
     LLVM_DEBUG(dbgs() << "   old top pred: " << getBlockName(Pred) << ", has "
-                      << Pred->succ_size() << " successors, ";
-               MBFI->printBlockFreq(dbgs(), Pred) << " freq\n");
+                      << Pred->succ_size() << " successors, "
+                      << printBlockFreq(MBFI->getMBFI(), *Pred) << " freq\n");
     if (Pred->succ_size() > 2)
       continue;
 
@@ -2240,10 +2241,10 @@ MachineBlockPlacement::findBestLoopExit(const MachineLoop &L,
       }
 
       BlockFrequency ExitEdgeFreq = MBFI->getBlockFreq(MBB) * SuccProb;
-      LLVM_DEBUG(dbgs() << "    exiting: " << getBlockName(MBB) << " -> "
-                        << getBlockName(Succ) << " [L:" << SuccLoopDepth
-                        << "] (";
-                 MBFI->printBlockFreq(dbgs(), ExitEdgeFreq) << ")\n");
+      LLVM_DEBUG(
+          dbgs() << "    exiting: " << getBlockName(MBB) << " -> "
+                 << getBlockName(Succ) << " [L:" << SuccLoopDepth << "] ("
+                 << printBlockFreq(MBFI->getMBFI(), ExitEdgeFreq) << ")\n");
       // Note that we bias this toward an existing layout successor to retain
       // incoming order in the absence of better information. The exit must have
       // a frequency higher than the current exit before we consider breaking
@@ -2537,8 +2538,8 @@ void MachineBlockPlacement::rotateLoopWithProfile(
     }
 
     LLVM_DEBUG(dbgs() << "The cost of loop rotation by making "
-                      << getBlockName(*Iter)
-                      << " to the top: " << Cost.getFrequency() << "\n");
+                      << getBlockName(*Iter) << " to the top: "
+                      << printBlockFreq(MBFI->getMBFI(), Cost) << "\n");
 
     if (Cost < SmallestRotationCost) {
       SmallestRotationCost = Cost;

llvm/lib/CodeGen/RegAllocGreedy.cpp

@@ -1061,8 +1061,8 @@ MCRegister RAGreedy::tryRegionSplit(const LiveInterval &VirtReg,
     // No benefit from the compact region, our fallback will be per-block
     // splitting. Make sure we find a solution that is cheaper than spilling.
     BestCost = SpillCost;
-    LLVM_DEBUG(dbgs() << "Cost of isolating all blocks = ";
-               MBFI->printBlockFreq(dbgs(), BestCost) << '\n');
+    LLVM_DEBUG(dbgs() << "Cost of isolating all blocks = "
+                      << printBlockFreq(*MBFI, BestCost) << '\n');
   }
 
   unsigned BestCand = calculateRegionSplitCost(VirtReg, Order, BestCost,
@@ -1112,8 +1112,8 @@ RAGreedy::calculateRegionSplitCostAroundReg(MCPhysReg PhysReg,
     LLVM_DEBUG(dbgs() << printReg(PhysReg, TRI) << "\tno positive bundles\n");
     return BestCand;
   }
-  LLVM_DEBUG(dbgs() << printReg(PhysReg, TRI) << "\tstatic = ";
-             MBFI->printBlockFreq(dbgs(), Cost));
+  LLVM_DEBUG(dbgs() << printReg(PhysReg, TRI)
+                    << "\tstatic = " << printBlockFreq(*MBFI, Cost));
   if (Cost >= BestCost) {
     LLVM_DEBUG({
       if (BestCand == NoCand)
@@ -1139,8 +1139,7 @@ RAGreedy::calculateRegionSplitCostAroundReg(MCPhysReg PhysReg,
 
   Cost += calcGlobalSplitCost(Cand, Order);
   LLVM_DEBUG({
-    dbgs() << ", total = ";
-    MBFI->printBlockFreq(dbgs(), Cost) << " with bundles";
+    dbgs() << ", total = " << printBlockFreq(*MBFI, Cost) << " with bundles";
     for (int I : Cand.LiveBundles.set_bits())
       dbgs() << " EB#" << I;
     dbgs() << ".\n";
@@ -2313,9 +2312,9 @@ void RAGreedy::tryHintRecoloring(const LiveInterval &VirtReg) {
       LLVM_DEBUG(dbgs() << "Checking profitability:\n");
       BlockFrequency OldCopiesCost = getBrokenHintFreq(Info, CurrPhys);
       BlockFrequency NewCopiesCost = getBrokenHintFreq(Info, PhysReg);
-      LLVM_DEBUG(dbgs() << "Old Cost: " << OldCopiesCost.getFrequency()
-                        << "\nNew Cost: " << NewCopiesCost.getFrequency()
-                        << '\n');
+      LLVM_DEBUG(dbgs() << "Old Cost: " << printBlockFreq(*MBFI, OldCopiesCost)
+                        << "\nNew Cost: "
+                        << printBlockFreq(*MBFI, NewCopiesCost) << '\n');
       if (OldCopiesCost < NewCopiesCost) {
         LLVM_DEBUG(dbgs() << "=> Not profitable.\n");
         continue;