[PATCH] D71238: Align non-fused branches within 32-Byte boundary (basic case)

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D71238#1776539 <https://reviews.llvm.org/D71238#1776539>, @jyknight wrote:

> In D71238#1776518 <https://reviews.llvm.org/D71238#1776518>, @reames wrote:
>
> > In D71238#1776512 <https://reviews.llvm.org/D71238#1776512>, @skan wrote:
> >
> > > In function `X86AsmBackend::alignBranchesBegin`, I added the comment
> > >
> > >   // The prefix or nop isn't inserted if the previous item is hard code, which
> > >   // may be used to hardcode an instruction, since there is no clear instruction
> > >   // boundary.
> > >   
> > >
> > > I'm sorry if I didn't make it clear. As far as i am concerned, I think the hard code case should be dealt with.
> >
> >
> > I'm still not following.  Can you explain the use case here?  What test case differs based on the presence of the "hard code" support in the original patch?
>
>
> Consider hand-written assembly code which looks like this:
>  ...
>  Inserting anything between the tacked-on-prefix with .byte, and the rest of the instruction written mnemonically, would be bad.


Ok, this makes more sense now.

> That said...I don't like the "hard-code" code. It feels like a workaround for the root issue -- that this feature represents a big shift in what x86 assemblers might do to your code, and thus should be opt-in by asm writers, and not enabled by any global flag.

Honestly, I both strongly agree with this, and see the argument as a somewhat lost cause.  I think in practical terms, we *will* have to support something which works for both compiler generated assembly and legacy hand written assembly.  I dislike that fact and the design it encourages, but it's reality.

On the hard code notion, after thinking about it over night, I suggest that we do *not* include it in the first patch.

First, exactly what "hard code" is has not been settled and is worthy of careful discussion.  For instance, I see no particular reason why the "manual prefix" trick described here should be supported, but the assumptions about label alignment mentioned in the previous thread shouldn't be.  There is room for discussion here, and if we block all progress on settling this, the review will stall.

Second, there are use cases for which the "hard code" support is unneeded.  For instance, compiler generated assembly which doesn't do the silly "manual prefix" trick.  Supporting them while we work through the support needed for opt in legacy assembly seems worthwhile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71238