[PATCH] D22521: [MBP] do not reorder and move up loop latch block

[Xinliang David Li via llvm-commits]

On Fri, Aug 12, 2016 at 8:20 AM, Sjoerd Meijer <sjoerd.meijer at arm.com> wrote:
> SjoerdMeijer added a comment.
>
> Thanks for the suggestion!
> I have been looking at it again and with the change to jump to bb from entry the cfg looks likes this:
>
>         entry-------
>                     |
>         loop        |
>         /     \     |
>   blocka    bb  - -
>              |
>             exit
>

There are two back edges missing (one from blocka to loop, one from bb to loop).

> the problem however is that blocks are placed like this (before pass MBP):
>
> entry
> block_a
> loop
> bb
> exit
>
> and when it is looking for a best loop top for "loop", it finds that "block_a" is placed before "loop", and it can fallthrough to "loop". So this does not result in "block_a" being placed before "loop". Actually, while writing tests, there will always be some preheader that fallsthrough to the loop.
>
> The fact that it is difficult to come up with a test is a bit worrying, in the sense that we add code in form of hasFallthroughLayoutPredecessor() that is always going to return true, so we will never end rotating the latch block to the top (when optimising for size). That in itself is fine, I think, because it shows that it in theory there is this case, but in practise it never happens. If we agree on this (I may have missed something), then we can make the implementation much simpler. We get rid of hasFallthroughLayoutPredecessor(), and simply don't do findBestLoopTop() when we optimize for size. By the way, while looking at this again I found that hasFallthroughLayoutPredecessor() is a bit dodgy because there is much simpler implementation possible, but it is not really important for now.

Ok -- just rip off the code that can not be properly tested properly
for now and revisit it later (add FIXME comment).

David

>
>
> https://reviews.llvm.org/D22521
>
>
>