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

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

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:

  	 .byte	0x67
  	 mov	%rdx, %rax

That is from boringssl, fwiw, https://github.com/google/boringssl/blob/master/crypto/fipsmodule/bn/asm/rsaz-avx2.pl.

Or, an example from Mesa xform4.S:

  p4_general_done:
  	.byte 0xf3
  	ret

Inserting anything between the tacked-on-prefix with .byte, and the rest of the instruction written mnemonically, would be bad.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71238