[PATCH] D70157: Align branches within 32-Byte boundary(NOP padding)

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D70157#1789159 <https://reviews.llvm.org/D70157#1789159>, @skan wrote:

> In D70157#1788445 <https://reviews.llvm.org/D70157#1788445>, @reames wrote:
>
> > Specifically on the revised patch, I remain confused by the need for multiple subtypes.  The need for fragments *between* the potentially fused instructions doesn't make sense to me.  What I was expecting to see was the following:
> >  BoundaryAlign w/target=the branch fragment
> >  .. some possibly empty sequence of fragments (i.e. the test/cmp/etc..) ...
> >  the branch fragment
> >  a new data fragment if the branch fragment was a DF
> >
> > (i.e. a single BounaryAlign fragment which aligns a payload which is defined as "next fragment to target fragment inclusive".)
> >
> > To be specific, I'd expect to see the following for an example fused sequence:
> >
> > 1. BoundaryAlign w/Target = 3
> > 2. DataFragment containing TEST RAX, RAX
> > 3. RelaxeableFragment containing JNE symbo
> >
> >   Why do we need anything between the two fragments of the fused pair?
> >
> >   (As a reminder, I am new to this code.  If I'm missing the obvious, please just point it out.)
>
>
> JUMP is not always emiteed into `MCRelaxableFragment`, it also can be emitted into `MCDataFragment` and  arithmetic ops with constant arguments of unknown value (e.g. ADD,AND) can be emitted into `MCRelaxableFragment` ,  you can find related code in `MCObjectStreamer::EmitInstructionImpl`, `X86AsmBackend::mayNeedRelaxation`.  Let's say JCC is fused with TEST,  there are four possible positions for JCC and CMP
>
> 1. JCC and CMP are in same `MCDataFragment`
> 2. JCC and CMP are in  two different `MCDataFragment`
> 3. JCC and CMP are in two different `MCRelaxableFragment`
> 4. JCC in a `MCRelaxableFragment`, CMP is in a `MCDataFragment`
>
>   and since `MCCompactEncodedInstFragment` is not applicable yet, i don't what's its behaviour.
>
>   In order to compute the total size of CMP and JCC in `MCAssembler::relaxBoundaryAlign`, I insert a `FusedJccSplit` to force CMP and JCC in two fragments. Do you have any better idea?


I agree there are multiple cases here, see the original generic description instead of the specific example.  The general question is why a *range* of fragments can't be defined.  Computing the instruction size for the entire range then just requires walking from first to last fragment in the range summing the size of each.  If both instructions are within the same data fragment, then no relaxation is needed, and the size of both is simply the size of the data fragment.

Unless I'm missing something here?


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

https://reviews.llvm.org/D70157