Skip to content

Commit

Permalink
[MBP] Add whole chain to BlockFilterSet instead of individual BB
Browse files Browse the repository at this point in the history
Currently we add individual BB to BlockFilterSet if its frequency satisfies

  LoopFreq / Freq <= LoopToColdBlockRatio

LoopFreq is edge frequency from outside to loop header.
LoopToColdBlockRatio is a command line parameter.

It doesn't make sense since we always layout whole chain, not individual BBs.

It may also cause a tricky problem. Sometimes it is possible that the LoopFreq
of an inner loop is smaller than LoopFreq of outer loop. So a BB can be in
BlockFilterSet of inner loop, but not in BlockFilterSet of outer loop,
like .cold in the test case. So it is added to the chain of inner loop. When
work on the outer loop, .cold is not added to BlockFilterSet, so the edge to
successor .problem is not counted in UnscheduledPredecessors of .problem chain.
But other blocks in the inner loop are added BlockFilterSet, so the whole inner
loop chain can be layout, and markChainSuccessors is called to decrease
UnscheduledPredecessors of following chains. markChainSuccessors calls
markBlockSuccessors for every BB, even it is not in BlockFilterSet, like .cold,
so .problem chain's UnscheduledPredecessors is decreased, but this edge was not
counted on in fillWorkLists, so .problem chain's UnscheduledPredecessors
becomes 0 when it still has an unscheduled predecessor .pred! And it causes
problems in following various successor BB selection algorithms.

Differential Revision: https://reviews.llvm.org/D89088
  • Loading branch information
weiguozhi committed Dec 16, 2020
1 parent 35ec3ff commit 687e80b
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 1 deletion.
6 changes: 5 additions & 1 deletion llvm/lib/CodeGen/MachineBlockPlacement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2543,10 +2543,14 @@ MachineBlockPlacement::collectLoopBlockSet(const MachineLoop &L) {
MBPI->getEdgeProbability(LoopPred, L.getHeader());

for (MachineBasicBlock *LoopBB : L.getBlocks()) {
if (LoopBlockSet.count(LoopBB))
continue;
auto Freq = MBFI->getBlockFreq(LoopBB).getFrequency();
if (Freq == 0 || LoopFreq.getFrequency() / Freq > LoopToColdBlockRatio)
continue;
LoopBlockSet.insert(LoopBB);
BlockChain *Chain = BlockToChain[LoopBB];
for (MachineBasicBlock *ChainBB : *Chain)
LoopBlockSet.insert(ChainBB);
}
} else
LoopBlockSet.insert(L.block_begin(), L.block_end());
Expand Down
64 changes: 64 additions & 0 deletions llvm/test/CodeGen/X86/block_set.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
; RUN: llc -mtriple=i686-linux < %s | FileCheck %s

define i1 @block_filter() !prof !22{
; CHECK-LABEL: block_filter
; CHECK: %.entry
; CHECK: %.header1
; CHECK: %.bb1
; CHECK: %.header2
; CHECK: %.latch2
; CHECK: %.cold
; CHECK: %.pred
; CHECK: %.problem
; CHECK: %.latch1
; CHECK: %.exit
.entry:
%val0 = call i1 @bar()
br label %.header1

.header1:
%val1 = call i1 @foo()
br i1 %val1, label %.bb1, label %.pred, !prof !2

.bb1:
%val11 = call i1 @foo()
br i1 %val11, label %.header2, label %.pred, !prof !2

.header2:
%val2 = call i1 @foo()
br i1 %val2, label %.latch2, label %.cold, !prof !10

.cold:
%val4 = call i1 @bar()
br i1 %val4, label %.latch2, label %.problem

.latch2:
%val5 = call i1 @foo()
br i1 %val5, label %.header2, label %.latch1, !prof !1

.pred:
%valp = call i1 @foo()
br label %.problem

.problem:
%val3 = call i1 @foo()
br label %.latch1

.latch1:
%val6 = call i1 @foo()
br i1 %val6, label %.header1, label %.exit, !prof !1

.exit:
%val7 = call i1 @foo()
ret i1 %val7
}

declare i1 @foo()
declare i1 @bar()

!1 = !{!"branch_weights", i32 5, i32 5}
!2 = !{!"branch_weights", i32 60, i32 40}
!3 = !{!"branch_weights", i32 90, i32 10}
!10 = !{!"branch_weights", i32 90, i32 10}

!22 = !{!"function_entry_count", i64 100}

0 comments on commit 687e80b

Please sign in to comment.