V2 [PATCH] x86: Rename VexOpcode to OpcodePrefix

[H.J. Lu]

On Wed, Oct 14, 2020 at 6:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.10.2020 14:55, H.J. Lu wrote:
> > On Wed, Oct 14, 2020 at 5:38 AM Jan Beulich <jbeulich@suse.com> wrote:
> >> On 14.10.2020 04:21, H.J. Lu via Binutils wrote:
> >>> * i386-opc.tbl (Prefix_0X66): New.
> >>> (Prefix_0XF2): Likewise.
> >>> (Prefix_0XF3): Likewise.
> >>> Replace VexOpcode= with OpcodePrefix=.  Use Prefix_0X66 on xorpd.
> >>> Use Prefix_0XF3 on cvtdq2pd.  Use Prefix_0XF2 on cvtpd2dq.
> >>
> >> Is this an arbitrary choice? Such things would be helpful to be
> >> spelled out in a little bit of a description, not just for reviewing
> >
> > How about
> >
> > // Add 0x66/0xf2/0xf3 prefix to non-VEX/EVEX/prefix instructions.
> > #define Prefix_0X66 OpcodePrefix=PREFIX_0X66
> > #define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
> > #define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
>
> I don't understand your reply, which makes me assume you didn't
> understand my question. I was asking whether the choice of
> adjusting just xorpd, cvtdq2pd, and cvtpd2dq was arbitrary.

It was a trial run, followed by the full patch.

> >> now, but also when later running into this commit. If so, I expect
> >> there'll be a full run through all opcodes to replace the current
> >> model with the new one?
> >
> > This is
> >
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8b65b8953af2d49ae1d2d7fcc5b49c5308febbc1
>
> Yes, I've seen this in the meantime, thanks. The same will of
> course want doing for the VEX etc encoded insns.
>
> >> I'd also like to note that your "Use prefix ... on ..." isn't
> >> actually correct, as you only changed the non-SSE2AVX encodings.
> >
> > It should be non-VEX/EVEX/prefix instructions.
> >
> >> (quoting the actual patch)
> >>
> >>> --- a/opcodes/i386-opc.h
> >>> +++ b/opcodes/i386-opc.h
> >>> @@ -561,6 +561,16 @@ enum
> >>> #define VEXW1 2
> >>> #define VEXWIG        3
> >>>   VexW,
> >>> +  /* Regular opcode prefix:
> >>> +     0: None
> >>> +     1: Add 0x66 opcode prefix.
> >>> +     2: Add 0xf2 opcode prefix.
> >>> +     3: Add 0xf3 opcode prefix.
> >>> +   */
> >>> +#define PREFIX_NONE   0
> >>> +#define PREFIX_0X66   1
> >>> +#define PREFIX_0XF2   2
> >>> +#define PREFIX_0XF3   3
> >>>   /* VEX opcode prefix:
> >>>      0: VEX 0x0F opcode prefix.
> >>>      1: VEX 0x0F38 opcode prefix.
> >>> @@ -575,7 +585,7 @@ enum
> >>> #define XOP08         3
> >>> #define XOP09         4
> >>> #define XOP0A         5
> >>> -  VexOpcode,
> >>> +  OpcodePrefix,
> >>
> >> I don't think this is a helpful repurposing. Instead I was
> >> thinking to encode 0F, 0F38, and 0F3A for non-VEX the same way,
> >> re-using the encodings here. 66, F3, and F2 as a result would
> >> need separate encoding.
> >
> > Free feel to change.  It should be easy to change with:
> >
> > #define Prefix_0X66 OpcodePrefix=PREFIX_0X66
> > #define Prefix_0XF2 OpcodePrefix=PREFIX_0XF2
> > #define Prefix_0XF3 OpcodePrefix=PREFIX_0XF3
> >
> >> I also think it would have been more logical (because of the
> >> way VEX/XOP/EVEX encode them) if it were
> >>
> >> #define PREFIX_NONE     0
> >> #define PREFIX_66       1
> >> #define PREFIX_F3       2
> >> #define PREFIX_F2       3
> >
> > Can you submit a patch?
>
> Sure. I may be able to find time for this in several months ...
>
> Jan



-- 
H.J.