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

BlockFrequencyInfo: Add PrintBlockFreq helper #67512

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Conversation

MatzeB
Copy link
Contributor

@MatzeB MatzeB commented Sep 27, 2023

  • 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.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 27, 2023

@llvm/pr-subscribers-llvm-analysis

Changes
  • 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() &lt;&lt; PrintBlockFreq(MBFI, Freq) &lt;&lt; '\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() &lt;&lt; Freq.getFrequency() with the new function.

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

11 Files Affected:

  • (modified) llvm/include/llvm/Analysis/BlockFrequencyInfo.h (+4-8)
  • (modified) llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h (+4-10)
  • (modified) llvm/include/llvm/CodeGen/MBFIWrapper.h (+1-6)
  • (modified) llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h (+11-9)
  • (modified) llvm/lib/Analysis/BlockFrequencyInfo.cpp (+17-18)
  • (modified) llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp (+6-12)
  • (modified) llvm/lib/CodeGen/MBFIWrapper.cpp (-10)
  • (modified) llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp (+12-13)
  • (modified) llvm/lib/CodeGen/MachineBlockPlacement.cpp (+9-9)
  • (modified) llvm/lib/CodeGen/RegAllocGreedy.cpp (+8-9)
  • (modified) llvm/lib/CodeGen/ShrinkWrap.cpp (+2-2)
diff --git a/llvm/include/llvm/Analysis/BlockFrequencyInfo.h b/llvm/include/llvm/Analysis/BlockFrequencyInfo.h
index 39507570a1b2c3f..7823177f7b98baf 100644
--- a/llvm/include/llvm/Analysis/BlockFrequencyInfo.h
+++ b/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, 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 BasicBlock *BB) const;
-
   uint64_t getEntryFreq() const;
   void releaseMemory();
   void print(raw_ostream &OS) const;
@@ -108,6 +101,9 @@ class BlockFrequencyInfo {
   void verifyMatch(BlockFrequencyInfo &Other) const;
 };
 
+Printable PrintBlockFreq(const BlockFrequencyInfo &BFI, BlockFrequency Freq);
+Printable PrintBlockFreq(const BlockFrequencyInfo &BFI, const BasicBlock &BB);
+
 /// Analysis pass which computes \c BlockFrequencyInfo.
 class BlockFrequencyAnalysis
     : public AnalysisInfoMixin<BlockFrequencyAnalysis> {
diff --git a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
index 54d56f8472c2bcc..9354015e6c15bf4 100644
--- a/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
+++ b/llvm/include/llvm/Analysis/BlockFrequencyInfoImpl.h
@@ -533,16 +533,15 @@ class BlockFrequencyInfoImplBase {
 
   void setBlockFreq(const BlockNode &Node, uint64_t Freq);
 
-  raw_ostream &printBlockFreq(raw_ostream &OS, const BlockNode &Node) const;
-  raw_ostream &printBlockFreq(raw_ostream &OS,
-                              const BlockFrequency &Freq) const;
-
   uint64_t getEntryFreq() const {
     assert(!Freqs.empty());
     return Freqs[0].Integer;
   }
 };
 
+void PrintBlockFreqImpl(raw_ostream &OS, uint64_t EntryFreq,
+                        BlockFrequency Freq);
+
 namespace bfi_detail {
 
 template <class BlockT> struct TypeMap {};
@@ -1068,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 +1856,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();
diff --git a/llvm/include/llvm/CodeGen/MBFIWrapper.h b/llvm/include/llvm/CodeGen/MBFIWrapper.h
index 714ecc5d4334e40..1b00deb6fac7b63 100644
--- a/llvm/include/llvm/CodeGen/MBFIWrapper.h
+++ b/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,13 +32,9 @@ 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,
-                              const BlockFrequency Freq) const;
   void view(const Twine &Name, bool isSimple = true);
   uint64_t getEntryFreq() const;
-  const MachineBlockFrequencyInfo &getMBFI() { return MBFI; }
+  const MachineBlockFrequencyInfo &getMBFI() const { return MBFI; }
 
  private:
   const MachineBlockFrequencyInfo &MBFI;
diff --git a/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h b/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
index 6d58c7a14fb95ac..3f8555e756a5287 100644
--- a/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
@@ -89,20 +89,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.
   uint64_t getEntryFreq() const;
 };
 
+/// Print the block frequency 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
diff --git a/llvm/lib/Analysis/BlockFrequencyInfo.cpp b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
index b18d04cc73dbca0..6686e4c7f2d954f 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfo.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfo.cpp
@@ -78,12 +78,11 @@ 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,
+static cl::opt<bool> PrintBFI("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::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
@@ -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());
   }
 }
@@ -266,17 +264,6 @@ const BranchProbabilityInfo *BlockFrequencyInfo::getBPI() const {
   return BFI ? &BFI->getBPI() : nullptr;
 }
 
-raw_ostream &BlockFrequencyInfo::
-printBlockFreq(raw_ostream &OS, const 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;
-}
-
 uint64_t BlockFrequencyInfo::getEntryFreq() const {
   return BFI ? BFI->getEntryFreq() : 0;
 }
@@ -293,6 +280,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)
diff --git a/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp b/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
index 82b1e3b9eede709..7bb05de08506a6d 100644
--- a/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
+++ b/llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp
@@ -643,19 +643,13 @@ 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,
-                                           const BlockFrequency &Freq) const {
+void llvm::PrintBlockFreqImpl(raw_ostream &OS, uint64_t EntryFreq,
+                              BlockFrequency Freq) {
+  if (EntryFreq == 0)
+    return;
   Scaled64 Block(Freq.getFrequency(), 0);
-  Scaled64 Entry(getEntryFreq(), 0);
-
-  return OS << Block / Entry;
+  Scaled64 Entry(EntryFreq, 0);
+  OS << Block / Entry;
 }
 
 void IrreducibleGraph::addNodesInLoop(const BFIBase::LoopData &OuterLoop) {
diff --git a/llvm/lib/CodeGen/MBFIWrapper.cpp b/llvm/lib/CodeGen/MBFIWrapper.cpp
index 5b388be27839464..6fe821e086babc1 100644
--- a/llvm/lib/CodeGen/MBFIWrapper.cpp
+++ b/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,
-                                          const BlockFrequency Freq) const {
-  return MBFI.printBlockFreq(OS, Freq);
-}
-
 void MBFIWrapper::view(const Twine &Name, bool isSimple) {
   MBFI.view(Name, isSimple);
 }
diff --git a/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp b/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
index b1cbe525d7e6c16..e3a26e245059a5e 100644
--- a/llvm/lib/CodeGen/MachineBlockFrequencyInfo.cpp
+++ b/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;
+uint64_t MachineBlockFrequencyInfo::getEntryFreq() const {
+  return MBFI ? MBFI->getEntryFreq() : 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);
+  });
 }
 
-uint64_t MachineBlockFrequencyInfo::getEntryFreq() const {
-  return MBFI ? MBFI->getEntryFreq() : 0;
+Printable llvm::PrintBlockFreq(const MachineBlockFrequencyInfo &MBFI,
+                               const MachineBasicBlock &MBB) {
+  return PrintBlockFreq(MBFI, MBFI.getBlockFreq(&MBB));
 }
diff --git a/llvm/lib/CodeGen/MachineBlockPlacement.cpp b/llvm/lib/CodeGen/MachineBlockPlacement.cpp
index 603c0e9600afc32..f38b6d7a579e9da 100644
--- a/llvm/lib/CodeGen/MachineBlockPlacement.cpp
+++ b/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,9 +2241,8 @@ 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");
+                        << 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 +2537,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;
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 36625c4848c5349..3e3d148820a96ee 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/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";
@@ -2302,9 +2301,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;
diff --git a/llvm/lib/CodeGen/ShrinkWrap.cpp b/llvm/lib/CodeGen/ShrinkWrap.cpp
index 4b1d3637a7462ec..ad0598c254a25c4 100644
--- a/llvm/lib/CodeGen/ShrinkWrap.cpp
+++ b/llvm/lib/CodeGen/ShrinkWrap.cpp
@@ -886,9 +886,9 @@ bool ShrinkWrap::performShrinkWrapping(
   do {
     LLVM_DEBUG(dbgs() << "Shrink wrap candidates (#, Name, Freq):\nSave: "
                       << printMBBReference(*Save) << ' '
-                      << MBFI->getBlockFreq(Save).getFrequency()
+                      << PrintBlockFreq(*MBFI, *Save)
                       << "\nRestore: " << printMBBReference(*Restore) << ' '
-                      << MBFI->getBlockFreq(Restore).getFrequency() << '\n');
+                      << PrintBlockFreq(*MBFI, *Restore) << '\n');
 
     bool IsSaveCheap, TargetCanUseSaveAsPrologue = false;
     if (((IsSaveCheap = EntryFreq >= MBFI->getBlockFreq(Save).getFrequency()) &&

@github-actions
Copy link

github-actions bot commented Sep 27, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@WenleiHe WenleiHe left a comment

Choose a reason for hiding this comment

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

LGTM.

llvm/lib/Analysis/BlockFrequencyInfoImpl.cpp Show resolved Hide resolved
llvm/include/llvm/Analysis/BlockFrequencyInfo.h Outdated Show resolved Hide resolved
@MatzeB
Copy link
Contributor Author

MatzeB commented Oct 2, 2023

@MaskRay is this good to go now?

@MatzeB
Copy link
Contributor Author

MatzeB commented Oct 5, 2023

I believe I have addressed the feedback from @MaskRay and it's been a week. Will rebase and land now.

- 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.
@MatzeB MatzeB merged commit 2e26d09 into llvm:main Oct 6, 2023
3 checks passed
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Oct 12, 2023
Local branch amd-gfx 22f2ec2 Merged main:8f397e04e5ce into amd-gfx:61f415de20dd
Remote branch main 2e26d09 BlockFrequencyInfo: Add PrintBlockFreq helper (llvm#67512)
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

5 participants