[PATCH] D156235: [MachineBlockPlacement] Remove the pad limit for no-fallthrough loops

[Momchil Velikov via Phabricator via llvm-commits]

chill added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:2954
+      if (MaxBytesForAlignmentOverride.getNumOccurrences() == 0
+        && (ChainBB == LoopHeader || MLI->getLoopFor(LayoutPred) != L))
+        ChainBB->setAlignment(Align, 0);
----------------
efriedma wrote:
> The `!LayoutPred->isSuccessor(ChainBB)` check already ensures the padding will never be executed.  Given that, I guess the remaining checks here are to try to maximize icache hits.  In that context, why are loop headers special?  Do we care if LayoutPred is part of a subloop of L?
> Given that, I guess the remaining checks here are to try to maximize icache hits.
Yes, I'd like to avoid excessive padding //inside// a loop.

> In that context, why are loop headers special?

They aren't really special, that check for loop header is a shortcut for not having to dive into
`getLoopFor()`, IIUC `findBestLoopTop` will place the loop header first (so the layout predecessor will be
in a different loop), or place another loop block in front of the header with a fallthrough to the header.

>  Do we care if LayoutPred is part of a subloop of L?
I haven't thought about this case. I'm going to experiment with doing this only for innermost loops.



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

https://reviews.llvm.org/D156235