[PATCH 1/2] [PATCH 1/2] Enable Intel AVX512_FP16 instructions

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, July 2, 2021 11:09 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: hjl.tools@gmail.com; binutils@sourceware.org
> Subject: Re: [PATCH 1/2] [PATCH 1/2] Enable Intel AVX512_FP16 instructions
> 
> On 01.07.2021 09:47, Cui,Lili wrote:
> >  opcodes/i386-dis-evex-mod.h    |  10 +
> >  opcodes/i386-dis-evex-prefix.h | 212 ++++++++++++
> >  opcodes/i386-dis-evex-w.h      | 442 +++++++++++++++++++++++-
> 
> Some of the vcvt* entries have two identical table entries. This suggests that
> splitting decode from the W bit is not needed (and this really is a general
> pattern: If all table entries at a level end up identical, then this decode step
> can be omitted unless there's a side effect of that step); see the pre-existing
> entries, where this gets handled (iirc) in the processing of Gdq. Or are you
> suggesting that there's a bug in the handling of pre-existing encodings?
> 
> >  opcodes/i386-dis-evex.h        | 600 ++++++++++++++++++++++++++++++++-
> >  opcodes/i386-dis.c             | 263 ++++++++++++++-
> 
VCVT{,T}SH2{,U}SI don't need W bit, I deleted W bit for them.


> xmmqh_mode seems to rather parallel evex_half_bcst_xmmq_mode, not
> xmmq_mode. Perhaps it would then also better be named similarly? Even if
> there might not be a pre-existing similar entry there, an analogous concern
> then applies to xmmqdh_mode wrt its name.
> 
Replace xmmqh_mode with evex_half_bcst_xmmqh_mode.
Replace xmmqdh_mode with evex_half_bcst_xmmqdh_mode.

> Is w_scalar_swap_mode really (as the comment says) like b_mode, not
> w_mode?

It should be like w_mode, I changed comments.

> While in pre-existing enumerators like PREFIX_EVEX_0F38AB I'm fine with the
> sequence of numbers (they express a sort-of sequence of opcode bytes,
> after all), I think names like
> PREFIX_EVEX_MAP510 really want an underscore inserted:
> PREFIX_EVEX_MAP5_10.
> 
Agree with you, I changed MAP510/ MAP6 to MAP5_/ MAP6_.


> Jan