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

Kan Shengchen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 9 19:54:17 PST 2019


skan 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 D71238#1776488 <https://reviews.llvm.org/D71238#1776488>, @reames wrote:
> >
> > > In D71238#1776443 <https://reviews.llvm.org/D71238#1776443>, @skan wrote:
> > >
> > > > The patch doesn't handle with the hard code case either,  but as far as I can see that change was not mentioned in the description of this patch.
> > >
> > >
> > > Unless I'm misreading the original patch, the notion of "hard code" is only relevant to the prefix padding scheme.  Admittedly, I might be misreading, it isn't clearly stated anywhere in the original patch exactly what "hard code" is or what purpose it serves.  .
> >
> >
> > 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?




      .text
      nop
  .Ltmp0:
      .p2align 3, 0x90
      .rept 16
      nop
      .endr
  .Ltmp3:
      movl  %eax, -4(%rsp)
      .rept 2
      .byte 0x2e
      .endr
      jmp .Ltmp0

The prefix `.rept 2 .byte 0x2e .endr` is used to prefix instruction `jmp .Ltmp0`. We shouldn't add a nop before the this jump, since it makes  `.rept 2 .byte 0x2e .endr` prefix instruction `nop`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71238





More information about the llvm-commits mailing list