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

[Sjoerd Meijer via llvm-commits]

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  

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.


https://reviews.llvm.org/D22521