[PATCH] D57067: [MBP] Don't move bottom block before header if it can't reduce taken branches

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1775
+// layout the other successor below it, so it can't reduce taken branch.
+// In this case we keep its original layout.
+bool
----------------
Carrot wrote:
> Carrot wrote:
> > davidxl wrote:
> > > Is there any benefit of keeping the original layout?
> > 1 If there is no benefit, the original layout is much better for readability.
> > 
> > 2 In theory on modern processor the consecutive two branches should be slower than two branches with other instructions between them.
> > 
> > case 1
> >      ...
> >      jne 
> >      jmp
> > 
> > case 2
> >      jne
> >      alu_1
> >      ...
> >      alu_n
> >      jmp
> > 
> > When prediction for jne is correct, there is no difference.
> > When prediction for jne is false and branch is not taken, following instructions is fetched and executed, in case 1 only one instruction is fetched and executed, in case 2 multiple instruction is fetched and executed at the same time. Usually branch instructions cause bubble in pipeline. In case 1 when jmp is executed, the pipeline is completely empty. In case 2 when jmp is executed, the pipeline can still execute previous instructions.
> > 
> I also noticed findBestLoopExit is only called when it can't find a better new Header. So if we move bottom BB as new Header, it will prevent finding a better loop exit edge.
Right. Suppose oldTop has another predecessor from outside the loop, when bottom BB is rotated before the oldTop, there will be a missing fall through from that predecessor to oldTop.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57067/new/

https://reviews.llvm.org/D57067