[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