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

Momchil Velikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
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



More information about the llvm-commits mailing list