[PATCH] D70157: Align branches within 32-Byte boundary

Thu Dec 5 15:31:07 PST 2019

jyknight added a comment.

In D70157#1771832 <https://reviews.llvm.org/D70157#1771832>, @reames wrote:

> I've been digging through the code for this for the last day or so.  This is a new area for me, so it's possible I'm off base, but I have some concerns about the current design.
>
> First, there appears to already be support for instruction bundling and alignment in the assembler today.  I stumbled across the .bundle_align_mode, .bundle_start, and .bundle_end mechanism (https://lists.llvm.org/pipermail/llvm-dev/2012-December/056723.html) which seems to *heavily* overlap with this proposal.  I suspect that the compiler support suggested by James and myself earlier in this thread could be implemented on to this existing mechanism.
>
> Second, the new callbacks and infrastructure added appear to overlap heavily w/the MCCodePadding infrastructure.  (Which, admitted, appears unused and untested.)

My conclusion after looking at all of that was actually that I plan to propose removing both the MCCodePadding and all the bundle-padding infrastructure, not add new stuff on top of it -- the former is unused, and I believe the latter is only for Chrome's NaCL, which is deprecated, and fairly close to being removed. If we need something similar in the future, we should certainly look to both of those for inspiration, but I don't think we need to be constrained by them.

> Third, I have not see a justification for why complexity for instruction prefix padding is necessary.  All the effected CPUs support multi-byte nops, so we're talking about a *single micro op* difference between the nop form and prefix form.  Can anyone point to a performance delta due to this?  If not, I'd suggest we should start with the nop form, and then build the prefix form in a generic manner for all alignment varieties.

+1.

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

https://reviews.llvm.org/D70157