-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
[AsmPrint] Dump raw frequencies in -mbb-profile-dump
#66818
Conversation
We were losing the function entry count, which is useful to check profile quality. For the original cases where we want entrypoint-relative MBB frequencies, the user would just need to divide these values by the entrypoint (first MBB, with ID=0) value.
@llvm/pr-subscribers-mlgo ChangesWe were losing the function entry count, which is useful to check Full diff: https://github.com/llvm/llvm-project/pull/66818.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 2ce08a2ff43955b..0c4ea1b3d9f04d9 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -1940,7 +1940,7 @@ void AsmPrinter::emitFunctionBody() {
for (const auto &MBB : *MF) {
*MBBProfileDumpFileOutput.get()
<< MF->getName() << "," << MBB.getBBID() << ","
- << MBFI.getBlockFreqRelativeToEntryBlock(&MBB) << "\n";
+ << MBFI.getBlockFreq(&MBB).getFrequency() << "\n";
}
}
}
diff --git a/llvm/test/CodeGen/MLRegAlloc/bb-profile-dump.ll b/llvm/test/CodeGen/MLRegAlloc/bb-profile-dump.ll
index e0ac148456cacbb..60a1fafc3c92c80 100644
--- a/llvm/test/CodeGen/MLRegAlloc/bb-profile-dump.ll
+++ b/llvm/test/CodeGen/MLRegAlloc/bb-profile-dump.ll
@@ -22,10 +22,10 @@ ifNotEqual:
ret i64 %sum
}
-; CHECK: f2,0,1.000000e+00
-; CHECK-NEXT: f1,0,1.000000e+00
-; CHECK-NEXT: f1,1,5.000000e-01
-; CHECK-NEXT: f1,2,1.000000e+00
+; CHECK: f2,0,8
+; CHECK-NEXT: f1,0,16
+; CHECK-NEXT: f1,1,8
+; CHECK-NEXT: f1,2,16
; Check that if we pass -mbb-profile-dump but don't set -basic-block-sections,
; we get an appropriate error message
|
Side note, maybe we should move this test in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took me a minute to realize that f2 and f1 were normalized separate with 8 and 16 previously. Perhaps add a comment to clarify that in the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Even in downstream applications like llvm-cm
it probably makes more sense to do this than using the relative block frequency.
I can push a commit to move the test once this lands. Definitely doesn't make a lot of sense to keep it in CodeGen/MLRegAlloc
when it's not specific to the register allocator at all.
…quencies (#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.
We were losing the function entry count, which is useful to check
profile quality. For the original cases where we want
entrypoint-relative MBB frequencies, the user would just need to divide
these values by the entrypoint (first MBB, with ID=0) value.