[llvm] f179486 - [AsmPrint] Correctly factor function entry count when dumping MBB frequencies (#67826)

[via llvm-commits]

Author: Mircea Trofin
Date: 2023-09-29T18:06:53-07:00
New Revision: f179486204135b67af53dcdd45c4b4f16dce2801

URL: https://github.com/llvm/llvm-project/commit/f179486204135b67af53dcdd45c4b4f16dce2801
DIFF: https://github.com/llvm/llvm-project/commit/f179486204135b67af53dcdd45c4b4f16dce2801.diff

LOG: [AsmPrint] Correctly factor function entry count when dumping MBB frequencies (#67826)

The goal in #66818 was to capture function entry counts, but those are not the same as the frequency of the entry (machine) basic block. This fixes that, and adds explicit profiles to the test.

We also increase the precision of `MachineBlockFrequencyInfo::getBlockFreqRelativeToEntryBlock` to double. Existing code uses it as float so should be unaffected.

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
    llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/test/CodeGen/Generic/bb-profile-dump.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h b/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
index 6d58c7a14fb95ac..1152eefed6e45c3 100644
--- a/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h
@@ -65,9 +65,10 @@ class MachineBlockFrequencyInfo : public MachineFunctionPass {
 
   /// Compute the frequency of the block, relative to the entry block.
   /// This API assumes getEntryFreq() is non-zero.
-  float getBlockFreqRelativeToEntryBlock(const MachineBasicBlock *MBB) const {
+  double getBlockFreqRelativeToEntryBlock(const MachineBasicBlock *MBB) const {
     assert(getEntryFreq() != 0 && "getEntryFreq() should not return 0 here!");
-    return getBlockFreq(MBB).getFrequency() * (1.0f / getEntryFreq());
+    return static_cast<double>(getBlockFreq(MBB).getFrequency()) /
+           static_cast<double>(getEntryFreq());
   }
 
   std::optional<uint64_t>

diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 0c4ea1b3d9f04d9..97d2fe3426406f3 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1929,7 +1929,8 @@ void AsmPrinter::emitFunctionBody() {
 
   // Output MBB ids, function names, and frequencies if the flag to dump
   // MBB profile information has been set
-  if (MBBProfileDumpFileOutput) {
+  if (MBBProfileDumpFileOutput && !MF->empty() &&
+      MF->getFunction().getEntryCount()) {
     if (!MF->hasBBLabels())
       MF->getContext().reportError(
           SMLoc(),
@@ -1937,10 +1938,23 @@ void AsmPrinter::emitFunctionBody() {
           "must be called with -basic-block-sections=labels");
     MachineBlockFrequencyInfo &MBFI =
         getAnalysis<LazyMachineBlockFrequencyInfoPass>().getBFI();
+    // The entry count and the entry basic block frequency aren't the same. We
+    // want to capture "absolute" frequencies, i.e. the frequency with which a
+    // MBB is executed when the program is executed. From there, we can derive
+    // Function-relative frequencies (divide by the value for the first MBB).
+    // We also have the information about frequency with which functions
+    // were called. This helps, for example, in a type of integration tests
+    // where we want to cross-validate the compiler's profile with a real
+    // profile.
+    // Using double precision because uint64 values used to encode mbb
+    // "frequencies" may be quite large.
+    const double EntryCount =
+        static_cast<double>(MF->getFunction().getEntryCount()->getCount());
     for (const auto &MBB : *MF) {
+      const double MBBRelFreq = MBFI.getBlockFreqRelativeToEntryBlock(&MBB);
+      const double AbsMBBFreq = MBBRelFreq * EntryCount;
       *MBBProfileDumpFileOutput.get()
-          << MF->getName() << "," << MBB.getBBID() << ","
-          << MBFI.getBlockFreq(&MBB).getFrequency() << "\n";
+          << MF->getName() << "," << MBB.getBBID() << "," << AbsMBBFreq << "\n";
     }
   }
 }

diff  --git a/llvm/test/CodeGen/Generic/bb-profile-dump.ll b/llvm/test/CodeGen/Generic/bb-profile-dump.ll
index 934c281219d4ae7..7391a6ee6f9128d 100644
--- a/llvm/test/CodeGen/Generic/bb-profile-dump.ll
+++ b/llvm/test/CodeGen/Generic/bb-profile-dump.ll
@@ -7,26 +7,48 @@
 
 ; Check that given a simple case, we can return the default MBFI
 
-define i64 @f2(i64 %a, i64 %b) {
+define i64 @f2(i64 %a, i64 %b) !prof !1{
     %sum = add i64 %a, %b
     ret i64 %sum
 }
 
-; CHECK: f2,0,8
+; CHECK: f2,0,1.000000e+03
 
-define i64 @f1() {
+define i64 @f1() !prof !2{
     %sum = call i64 @f2(i64 2, i64 2)
     %isEqual = icmp eq i64 %sum, 4
-    br i1 %isEqual, label %ifEqual, label %ifNotEqual
+    br i1 %isEqual, label %ifEqual, label %ifNotEqual, !prof !3
 ifEqual:
     ret i64 0
 ifNotEqual:
     ret i64 %sum
 }
 
-; CHECK-NEXT: f1,0,16
-; CHECK-NEXT: f1,1,8
-; CHECK-NEXT: f1,2,16
+; CHECK-NEXT: f1,0,1.000000e+01
+; CHECK-NEXT: f1,2,6.000000e+00
+; CHECK-NEXT: f1,1,4.000000e+00
+
+define void @f3(i32 %iter) !prof !4 {
+entry:
+    br label %loop
+loop:
+    %i = phi i32 [0, %entry], [%i_next, %loop]
+    %i_next = add i32 %i, 1
+    %exit_cond = icmp slt i32 %i_next, %iter
+    br i1 %exit_cond, label %loop, label %exit, !prof !5
+exit:
+    ret void
+}
+
+; CHECK-NEXT: f3,0,2.000000e+00
+; CHECK-NEXT: f3,1,2.002000e+03
+; CHECK-NEXT: f3,2,2.000000e+00
+
+!1 = !{!"function_entry_count", i64 1000}
+!2 = !{!"function_entry_count", i64 10}
+!3 = !{!"branch_weights", i32 2, i32 3}
+!4 = !{!"function_entry_count", i64 2}
+!5 = !{!"branch_weights", i32 1000, i32 1}
 
 ; Check that if we pass -mbb-profile-dump but don't set -basic-block-sections,
 ; we get an appropriate error message