[llvm] [AsmPrint] Correctly factor function entry count when dumping MBB frequencies (PR #67826)
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 29 11:57:32 PDT 2023
https://github.com/mtrofin updated https://github.com/llvm/llvm-project/pull/67826
>From 4c11591a42d2132589b886ade415bc1ed2dc542b Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Fri, 29 Sep 2023 09:06:12 -0700
Subject: [PATCH 1/3] [AsmPrint] Correctly factor function entry count when
dumping MBB frequencies
The goal in PR #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.
---
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 23 +++++++++++--
llvm/test/CodeGen/Generic/bb-profile-dump.ll | 36 ++++++++++++++++----
2 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 0c4ea1b3d9f04d9..a54a5ab75797b4c 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1929,7 +1929,7 @@ 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()) {
if (!MF->hasBBLabels())
MF->getContext().reportError(
SMLoc(),
@@ -1937,10 +1937,27 @@ 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),
+ // and 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());
+ const double EntryFrequency =
+ static_cast<double>(MBFI.getBlockFreq(&*MF->begin()).getFrequency());
+ const double FreqAdjustmentFactor = EntryCount / EntryFrequency;
for (const auto &MBB : *MF) {
+ const double MBBFreq =
+ static_cast<double>(MBFI.getBlockFreq(&MBB).getFrequency());
+ const double AbsMBBFreq = MBBFreq * FreqAdjustmentFactor;
*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
>From 39bcca3f0336c8bfb850298b2c89f818c8d8b68a Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Fri, 29 Sep 2023 10:18:35 -0700
Subject: [PATCH 2/3] Split long sentences in comment.
---
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index a54a5ab75797b4c..5b1f7adf78eafa7 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1939,9 +1939,9 @@ void AsmPrinter::emitFunctionBody() {
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),
- // and we also have the information about frequency with which functions
+ // 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.
>From 05dc741c4959bc59b65725014425ba9b5fac74ae Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Fri, 29 Sep 2023 11:55:55 -0700
Subject: [PATCH 3/3] feedback
---
llvm/include/llvm/CodeGen/MachineBlockFrequencyInfo.h | 5 +++--
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | 11 ++++-------
2 files changed, 7 insertions(+), 9 deletions(-)
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 5b1f7adf78eafa7..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 && !MF->empty()) {
+ if (MBBProfileDumpFileOutput && !MF->empty() &&
+ MF->getFunction().getEntryCount()) {
if (!MF->hasBBLabels())
MF->getContext().reportError(
SMLoc(),
@@ -1949,13 +1950,9 @@ void AsmPrinter::emitFunctionBody() {
// "frequencies" may be quite large.
const double EntryCount =
static_cast<double>(MF->getFunction().getEntryCount()->getCount());
- const double EntryFrequency =
- static_cast<double>(MBFI.getBlockFreq(&*MF->begin()).getFrequency());
- const double FreqAdjustmentFactor = EntryCount / EntryFrequency;
for (const auto &MBB : *MF) {
- const double MBBFreq =
- static_cast<double>(MBFI.getBlockFreq(&MBB).getFrequency());
- const double AbsMBBFreq = MBBFreq * FreqAdjustmentFactor;
+ const double MBBRelFreq = MBFI.getBlockFreqRelativeToEntryBlock(&MBB);
+ const double AbsMBBFreq = MBBRelFreq * EntryCount;
*MBBProfileDumpFileOutput.get()
<< MF->getName() << "," << MBB.getBBID() << "," << AbsMBBFreq << "\n";
}
More information about the llvm-commits
mailing list