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

Wed Jul 26 05:58:10 PDT 2023

chill added a comment.

In D156235#4532640 <https://reviews.llvm.org/D156235#4532640>, @efriedma wrote:

> I'm having a hard time following what changes are actually semantically significant here.  Can you split the "remove MBB argument from getMaxPermittedBytesForAlignment" part into a separate patch?

Done.

In D156235#4532920 <https://reviews.llvm.org/D156235#4532920>, @dmgreen wrote:

> I think if someone specifies -max-bytes-for-alignment, the command line argument should not be ignored for non-fallthrough loop headers. It should ideally take precedence over this heuristic.

Done.

> I would also make this AArch64 specific, as it has not been verified on any other architectures. That is debatable though, it just might be using more space than is desirable. If that can't be done via a basic block arg, maybe getMaxPermittedBytesForAlignment could take a bool indicating whether it is non-fallthrough loop header.

The behaviour already exists on all the other architectures (except LoongArch) since they keep `TargetLoweringBase::MaxBytesForAlignment` initialised to `0`, i.e. no limit. Thus I would not expect regressions.
I've added people who touched loop alignment on LoongArch as reviewers.

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

https://reviews.llvm.org/D156235