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

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D70157#1771841 <https://reviews.llvm.org/D70157#1771841>, @jyknight wrote:

> 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.


I can definitely see removing the code padding stuff, since it's unused and untested.

As for the bundle mechanisms, why?  It seems like exactly what we're going to want here.  Regardless of the auto-detect feature, we're going to need a representation of a bundle which needs to be properly placed to avoid splitting, and the current code does that.  Why not reuse the, presumable reasonable well tested, existing infrastructure?   The only extra thing we seem to need is the ability to toggle off bundle formation for instruction types we don't care about.  Since we're going to need an assembler spelling of that regardless, it seems like the current code is a decent baseline?


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

https://reviews.llvm.org/D70157