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.

Sign up for GitHub

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

[JumpThreading] Use [BB->SuccIndx] to get probability when updating BB info. #134585

Merged
merged 2 commits into from
Apr 8, 2025
Merged

[JumpThreading] Use [BB->SuccIndx] to get probability when updating BB info. #134585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c525f6
[JumpThreading] Use [BB->SuccIndx] to get probability when updating B…
tianleliu Apr 6, 2025
15b89f1
Add branch_weights checking.
tianleliu Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions llvm/lib/Transforms/Scalar/JumpThreading.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2530,17 +2530,16 @@ void JumpThreadingPass::updateBlockFreqAndEdgeWeight(BasicBlock *PredBB,
// frequency of BB.
auto BBOrigFreq = BFI->getBlockFreq(BB);
auto NewBBFreq = BFI->getBlockFreq(NewBB);
auto BB2SuccBBFreq = BBOrigFreq * BPI->getEdgeProbability(BB, SuccBB);
auto BBNewFreq = BBOrigFreq - NewBBFreq;
BFI->setBlockFreq(BB, BBNewFreq);

// Collect updated outgoing edges' frequencies from BB and use them to update
// edge probabilities.
SmallVector<uint64_t, 4> BBSuccFreq;
for (BasicBlock *Succ : successors(BB)) {
auto SuccFreq = (Succ == SuccBB)
? BB2SuccBBFreq - NewBBFreq
: BBOrigFreq * BPI->getEdgeProbability(BB, Succ);
for (succ_iterator I = succ_begin(BB), E = succ_end(BB); I != E; ++I) {
auto BB2SuccBBFreq =
BBOrigFreq * BPI->getEdgeProbability(BB, I.getSuccessorIndex());
auto SuccFreq = (*I == SuccBB) ? BB2SuccBBFreq - NewBBFreq : BB2SuccBBFreq;
BBSuccFreq.push_back(SuccFreq.getFrequency());
}

Expand Down
42 changes: 42 additions & 0 deletions llvm/test/Transforms/JumpThreading/thread-prob-8.ll
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please test that the resulting branch weights in the IR are correct, instead of checking debug dumps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Nikic, thanks for your review. I have added branch weight checking in test file.
I think dumping branch probability is helpful to understand what the patch changes. So I prefer to remain it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It limits the test to debug only, which is not great, but I agree that the output is clearer.

Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
; RUN: opt -debug-only=branch-prob -passes=jump-threading -S %s 2>&1 | FileCheck %s
; REQUIRES: asserts

; Make sure that edges' probabilities would not accumulate if they are
; the same target BB.
; Edge L0 -> 2 and L0 -> 3 's targets are both L2, but their respective
; probability should not be L0 -> L2, because prob[L0->L2] equls to
; prob[L0->2] + prob[L0->3]

; CHECK: Computing probabilities for entry
; CHECK: eraseBlock L0
; CHECK-NOT: set edge L0 -> 0 successor probability to 0x12492492 / 0x80000000 = 14.29%
; CHECK-NOT: set edge L0 -> 1 successor probability to 0x24924925 / 0x80000000 = 28.57%
; CHECK-NOT: set edge L0 -> 2 successor probability to 0x24924925 / 0x80000000 = 28.57%
; CHECK-NOT: set edge L0 -> 3 successor probability to 0x24924925 / 0x80000000 = 28.57%
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these CHECK-NOTs are useful if you already CHECK the corresponding lines.

; CHECK: set edge L0 -> 0 successor probability to 0x1999999a / 0x80000000 = 20.00%
; CHECK: set edge L0 -> 1 successor probability to 0x33333333 / 0x80000000 = 40.00%
; CHECK: set edge L0 -> 2 successor probability to 0x1999999a / 0x80000000 = 20.00%
; CHECK: set edge L0 -> 3 successor probability to 0x1999999a / 0x80000000 = 20.00%
; CHECK-NOT: !0 = !{!"branch_weights", i32 306783378, i32 613566757, i32 613566757, i32 613566757}
; CHECK: !0 = !{!"branch_weights", i32 429496730, i32 858993459, i32 429496730, i32 429496730}
define void @test_switch(i1 %cond, i8 %value) nounwind {
entry:
br i1 %cond, label %L0, label %L4
L0:
%expr = select i1 %cond, i8 1, i8 %value
switch i8 %expr, label %L3 [
i8 1, label %L1
i8 2, label %L2
i8 3, label %L2
], !prof !0

L1:
ret void
L2:
ret void
L3:
ret void
L4:
br label %L0
}
!0 = !{!"branch_weights", i32 1, i32 7, i32 1, i32 1}