[PATCH] D27742: CodeGen: Allow small copyable blocks to "break" the CFG.

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 6 14:30:18 PST 2017


On Fri, Jan 6, 2017 at 1:36 PM, Kyle Butt via Phabricator <
reviews at reviews.llvm.org> wrote:

> iteratee added inline comments.
>
>
> ================
> Comment at: test/CodeGen/PowerPC/tail-dup-break-cfg.ll:7
> +; The code for tail-duplication during layout will produce the layout:
> +; test1
> +; test2
> ----------------
> davidxl wrote:
> > This test demonstrates that  this patch helps when test2 is more likely
> (but the branch probability is not biased enough to make the successor
> selection to pick test2 as successor without the patch).
> >
> > What happens if body1 a more likely successor such that even with the
> patch, body1 will be selected as test1's successor. Later when block test2
> is checked, it is still beneficial to tail duplicate it. Does it happen
> with the patch?
> This patch only prefers blocks that can be tail-duplicated if they are
> more probable.
>
> selectBestSuccessor looks at the blocks in order and picks the most
> probable block that doesn't have a CFG conflict, or is hot enough to ignore
> the CFG conflict. Most of that logic is in hasBetterLayoutPredecessor. This
> patch just adds "can tail-duplicate to unplaced preds" as another condition
> to allow CFG-breaking.
>
> If body1 is laid out after test1, then test2 will not be copied into the
> end of test1, as test1 has more than one successor. Since test2 would be
> laid out after body1, it would not be tail-duplicated anywhere.
>


Right -- it is not profitable to taildup test2 in this case.




>
> In an if-else case where both bodies go to a small test afterward:
> ```
>     t1
>    / \
>  b1   e1
>    \ /
>     t2
> ```
>
> The existing code will duplicate the test without this patch, producing
> either
>     t1 b1 t2 ... e1(t2) or t1 e1 t2 ... b2(t2)
>
> I can create that test if you'd like, but since it's not changed by this
> patch, it should go in as a separate change.
>
>
Ok sounds fine to me.

David



>
> https://reviews.llvm.org/D27742
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170106/d296ae48/attachment.html>


More information about the llvm-commits mailing list