[PATCH] D22892: Codegen: MachineBlockPlacement Improve probability layout.
Xinliang David Li via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 27 19:31:35 PDT 2016
A better way to illustrate forked diamond
// H
// / \
// / \
// BB Pred
// / \ /|
// | X |
// | / \ |
// S1 S2
// \ /
// M
On Wed, Jul 27, 2016 at 6:47 PM, David Li <davidxl at google.com> wrote:
> davidxl added a comment.
>
> This is a good change. I actually wanted to get rid of the forward check
> which is always true for case 2 and redundant for case 1. Now it is also
> bad for case 3.
>
> Please add a test case also.
>
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:639
> @@ +638,3 @@
> + // / \
> + // BB Pred
> + // / \ / \
> ----------------
> Nit: can you make the art work like the following to not split S2 into two
> 'blocks':
>
> //
> // Head (Or Entry, Top)
> // / \
> // / \
> // BB Pred
> // / \ / |
> // | S1 |
> // \ /
> // S2
> //
>
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:643
> @@ +642,3 @@
> + //
> + // The current block is BB and edge BB->S1 is now being evaluated.
> + // As above S->BB was already selected because
> ----------------
> Nit: can you make the art work like the following to not split S2 into two
> 'blocks':
>
> //
> // Head (Or Entry, Top)
> // / \
> // / \
> // BB Pred
> // / \ / |
> // | S1 |
> // \ /
> // S2
> //
>
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:660
> @@ +659,3 @@
> + // Non-topo-order cost:
> + // In the worst case, S2 will not get layed out after Pred.
> + // non-topo-cost = 2 * freq(S->Pred) + freq(BB->S2).
> ----------------
> layed out --> laid out
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:661
> @@ +660,3 @@
> + // In the worst case, S2 will not get layed out after Pred.
> + // non-topo-cost = 2 * freq(S->Pred) + freq(BB->S2).
> + // To be conservative, we can assume that min(freq(Pred->S1),
> freq(Pred->S2))
> ----------------
> Another way to explain in terms of savings instead of cost: the savings is
> the total freq of the fall through edges. In topo case, the savings is
>
> freq(S->BB) + max(freq(Pred->S1), freq(Pred->S2). (1)
>
> For non-top case, the saving is:
>
> freq(S->BB) + freq(BB->S1) + freq(Pred->S2) (2)
>
> When freq(Pred->S2) > freq(Pred->S1), (2) is strictly larger than (1). In
> the opposite case, the check below will also lead to (2) > (1)
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:679
> @@ +678,3 @@
> + // Do backward checking.
> + // For case 2 above, we need a backward checking to filter out edges
> that
> + // are not 'strongly' biased. With profile data available, the check
> is
> ----------------
> For case 1 and 2
>
> ================
> Comment at: lib/CodeGen/MachineBlockPlacement.cpp:683
> @@ -661,1 +682,3 @@
> + // than two successors.
> + // For case 3 above, this test is essential, even with profiling data.
> // BB Pred
> ----------------
> This comment does not fit here. With profile data, such check won't be
> skipped, but just does not need to be as biased.
>
>
> Repository:
> rL LLVM
>
> https://reviews.llvm.org/D22892
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160727/b58859a3/attachment.html>
More information about the llvm-commits
mailing list