[BPI] Reuse the AsmWriter's BB naming scheme in BranchProbabilityPrinterPass #73593
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using `BranchProbabilityPrinterPass`, if a BB has no name, we get pretty unusable information like `edge -> has probability...` (i.e. no idea what the vertices of that edge are). This patch reuses the same naming sheme `Function::dump`. Especially during debugging sessions, the IR obtained from a function and the names used by BPPP will match.
llvmbot
added
llvm:ir
llvm:analysis
|
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-llvm-ir Author: Mircea Trofin (mtrofin) ChangesWhen using This patch reuses the same naming sheme Full diff: https://github.com/llvm/llvm-project/pull/73593.diff 5 Files Affected:
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index ec916acc25151c8..98ff865dba96974 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -420,8 +420,8 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// Print the basic block to an output stream with an optional
/// AssemblyAnnotationWriter.
void print(raw_ostream &OS, AssemblyAnnotationWriter *AAW = nullptr,
- bool ShouldPreserveUseListOrder = false,
- bool IsForDebug = false) const;
+ bool ShouldPreserveUseListOrder = false, bool IsForDebug = false,
+ bool NameOnly = false) const;
//===--------------------------------------------------------------------===//
/// Instruction iterator methods
diff --git a/llvm/lib/Analysis/BranchProbabilityInfo.cpp b/llvm/lib/Analysis/BranchProbabilityInfo.cpp
index 9789a3e5364c3c6..04de2d2cebe4f3c 100644
--- a/llvm/lib/Analysis/BranchProbabilityInfo.cpp
+++ b/llvm/lib/Analysis/BranchProbabilityInfo.cpp
@@ -1188,8 +1188,13 @@ BranchProbabilityInfo::printEdgeProbability(raw_ostream &OS,
const BasicBlock *Src,
const BasicBlock *Dst) const {
const BranchProbability Prob = getEdgeProbability(Src, Dst);
- OS << "edge " << Src->getName() << " -> " << Dst->getName()
- << " probability is " << Prob
+ OS << "edge ";
+ Src->print(OS, /*AAW=*/nullptr, /*ShouldPreserveUseListOrder=*/false,
+ /*IsForDebug=*/false, /*NameOnly=*/true);
+ OS << " -> ";
+ Dst->print(OS, /*AAW=*/nullptr, /*ShouldPreserveUseListOrder=*/false,
+ /*IsForDebug=*/false, /*NameOnly=*/true);
+ OS << " probability is " << Prob
<< (isEdgeHot(Src, Dst) ? " [HOT edge]\n" : "\n");
return OS;
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 601b636f306adeb..9b14ad974139783 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -2624,7 +2624,7 @@ class AssemblyWriter {
void printComdat(const Comdat *C);
void printFunction(const Function *F);
void printArgument(const Argument *FA, AttributeSet Attrs);
- void printBasicBlock(const BasicBlock *BB);
+ void printBasicBlock(const BasicBlock *BB, bool NameOnly = false);
void printInstructionLine(const Instruction &I);
void printInstruction(const Instruction &I);
void printDPMarker(const DPMarker &DPI);
@@ -3960,21 +3960,26 @@ void AssemblyWriter::printArgument(const Argument *Arg, AttributeSet Attrs) {
}
/// printBasicBlock - This member is called for each basic block in a method.
-void AssemblyWriter::printBasicBlock(const BasicBlock *BB) {
- bool IsEntryBlock = BB->getParent() && BB->isEntryBlock();
- if (BB->hasName()) { // Print out the label if it exists...
+void AssemblyWriter::printBasicBlock(const BasicBlock *BB, bool NameOnly) {
+ const bool IsEntryBlock = BB->getParent() && BB->isEntryBlock();
+ const bool UnnamedEntry = !BB->hasName() && IsEntryBlock;
+ if (!NameOnly && !UnnamedEntry)
Out << "\n";
+ if (BB->hasName()) { // Print out the label if it exists...
PrintLLVMName(Out, BB->getName(), LabelPrefix);
- Out << ':';
} else if (!IsEntryBlock) {
- Out << "\n";
int Slot = Machine.getLocalSlot(BB);
if (Slot != -1)
- Out << Slot << ":";
+ Out << Slot;
else
- Out << "<badref>:";
+ Out << "<badref>";
}
+ if (NameOnly)
+ return;
+ if (!UnnamedEntry)
+ Out << ":";
+
if (!IsEntryBlock) {
// Output predecessors for the block.
Out.PadToColumn(50);
@@ -4658,14 +4663,14 @@ void Function::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,
}
void BasicBlock::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,
- bool ShouldPreserveUseListOrder,
- bool IsForDebug) const {
+ bool ShouldPreserveUseListOrder, bool IsForDebug,
+ bool NameOnly) const {
SlotTracker SlotTable(this->getParent());
formatted_raw_ostream OS(ROS);
AssemblyWriter W(OS, SlotTable, this->getModule(), AAW,
IsForDebug,
ShouldPreserveUseListOrder);
- W.printBasicBlock(this);
+ W.printBasicBlock(this, NameOnly);
}
void Module::print(raw_ostream &ROS, AssemblyAnnotationWriter *AAW,
diff --git a/llvm/test/Analysis/BranchProbabilityInfo/anonymous-bb.ll b/llvm/test/Analysis/BranchProbabilityInfo/anonymous-bb.ll
new file mode 100644
index 000000000000000..7ca97699dfba2b1
--- /dev/null
+++ b/llvm/test/Analysis/BranchProbabilityInfo/anonymous-bb.ll
@@ -0,0 +1,25 @@
+; RUN: opt < %s -passes='print<branch-prob>' -disable-output 2>&1 | FileCheck %s
+
+define void @fct() {
+; CHECK: fct
+entry:
+ br label %0
+0:
+ br label %1
+1:
+ ret void
+}
+
+; CHECK: edge entry -> 0
+; CHECK: edge 0 -> 1
+
+define void @fct2() {
+; CHECK: fct2
+ br label %1
+1:
+ br label %2
+2:
+ ret void
+}
+; CHECK: edge -> 1
+; CHECK: edge 1 -> 2
\ No newline at end of file
diff --git a/llvm/test/Analysis/BranchProbabilityInfo/pointer_heuristics.ll b/llvm/test/Analysis/BranchProbabilityInfo/pointer_heuristics.ll
index 3c6798d31f9ba71..76a57de9ef82cdd 100644
--- a/llvm/test/Analysis/BranchProbabilityInfo/pointer_heuristics.ll
+++ b/llvm/test/Analysis/BranchProbabilityInfo/pointer_heuristics.ll
@@ -4,18 +4,18 @@ define i32 @cmp1(ptr readnone %0, ptr readnone %1) {
; CHECK-LABEL: 'cmp1'
%3 = icmp eq ptr %0, %1
br i1 %3, label %4, label %6
-; CHECK: edge -> probability is 0x30000000 / 0x80000000 = 37.50%
-; CHECK: edge -> probability is 0x50000000 / 0x80000000 = 62.50%
+; CHECK: edge -> 4 probability is 0x30000000 / 0x80000000 = 37.50%
+; CHECK: edge -> 6 probability is 0x50000000 / 0x80000000 = 62.50%
4: ; preds = %2
%5 = tail call i32 @f() #2
br label %8
-; CHECK: edge -> probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge 4 -> 8 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
6: ; preds = %2
%7 = tail call i32 @g() #2
br label %8
-; CHECK: edge -> probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge 6 -> 8 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
8: ; preds = %6, %4
%9 = phi i32 [ %5, %4 ], [ %7, %6 ]
@@ -26,18 +26,18 @@ define i32 @cmp2(ptr readnone %0, ptr readnone %1) {
; CHECK-LABEL: 'cmp2'
%3 = icmp eq ptr %0, %1
br i1 %3, label %6, label %4
-; CHECK: edge -> probability is 0x30000000 / 0x80000000 = 37.50%
-; CHECK: edge -> probability is 0x50000000 / 0x80000000 = 62.50%
+; CHECK: edge -> 6 probability is 0x30000000 / 0x80000000 = 37.50%
+; CHECK: edge -> 4 probability is 0x50000000 / 0x80000000 = 62.50%
4: ; preds = %2
%5 = tail call i32 @f() #2
br label %8
-; CHECK: edge -> probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge 4 -> 8 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
6: ; preds = %2
%7 = tail call i32 @g() #2
br label %8
-; CHECK: edge -> probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge 6 -> 8 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
8: ; preds = %6, %4
%9 = phi i32 [ %5, %4 ], [ %7, %6 ]
@@ -48,18 +48,18 @@ define i32 @cmp3(ptr readnone %0) {
; CHECK-LABEL: 'cmp3'
%2 = icmp eq ptr %0, null
br i1 %2, label %3, label %5
-; CHECK: edge -> probability is 0x30000000 / 0x80000000 = 37.50%
-; CHECK: edge -> probability is 0x50000000 / 0x80000000 = 62.50%
+; CHECK: edge -> 3 probability is 0x30000000 / 0x80000000 = 37.50%
+; CHECK: edge -> 5 probability is 0x50000000 / 0x80000000 = 62.50%
3: ; preds = %1
%4 = tail call i32 @f() #2
br label %7
-; CHECK: edge -> probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge 3 -> 7 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
5: ; preds = %1
%6 = tail call i32 @g() #2
br label %7
-; CHECK: edge -> probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
+; CHECK: edge 5 -> 7 probability is 0x80000000 / 0x80000000 = 100.00% [HOT edge]
7: ; preds = %5, %3
%8 = phi i32 [ %6, %5 ], [ %4, %3 ]
|
jmorse
reviewed
Nov 30, 2023
There was a problem hiding this comment.
I'm not an expert on branch-probability facilities, but the test changes look highly valuable,
It seems awkward to add an extra flag to the block-printer, as it introduces a number of conditionals -- would it be possible to refactor as a dedicated printBlockName method rather than an extra option for block printing?
MatzeB
reviewed
Dec 1, 2023
Ah, nice! Yes. Only thing is that this will re-initialize a SlotTracker for every edge vertex, but probably this inefficiency doesn't matter for the scenario. If it does, we can address it later. |
I think @MatzeB's suggestion tackles this, ptal. |
MatzeB
reviewed
Dec 1, 2023
Ack - I did for the new test, and updated the remaining tests - since now all the BB names have a preceding |
mtrofin
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using
BranchProbabilityPrinterPass, if a BB has no name, we get pretty unusable information likeedge -> has probability...(i.e. we have no idea what the vertices of that edge are).This patch uses
printAsOperand, which uses the same naming scheme asFunction::dump, so for example during debugging sessions, the IR obtained from a function and the names used byBranchProbabilityPrinterPasswill match.A shortcoming is that
printAsOperandwill result in the numbering algorithm re-running for every edge and every vertex (whenBranchProbabilityPrinterPassis run on a function). If, for the given scenario, this is a problem, we can revisit this subsequently.Another nuance is that the entry basic block will be numbered, which may be slightly confusing when it's anonymous, but it's easily identifiable - the first edge would have it as source (and the number should be easily recognizable)