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

[Jan Beulich]

On 01.07.2021 09:47, Cui,Lili wrote:
> ---
>  gas/config/tc-i386.c           |  15 +-
>  gas/doc/c-i386.texi            |   4 +-
>  opcodes/i386-dis-evex-mod.h    |  10 +
>  opcodes/i386-dis-evex-prefix.h | 212 ++++++++++++
>  opcodes/i386-dis-evex-w.h      | 442 +++++++++++++++++++++++-
>  opcodes/i386-dis-evex.h        | 600 ++++++++++++++++++++++++++++++++-
>  opcodes/i386-dis.c             | 263 ++++++++++++++-
>  opcodes/i386-gen.c             |   7 +-

The expansion of CPU_AVX512_FP16_FLAGS wants to use
CPU_AVX512BW_FLAGS instead of CPU_AVX512F_FLAGS. Similarly
CPU_ANY_AVX512BW_FLAGS wants to be altered to include
CPU_ANY_AVX512_FP16_FLAGS.

In the CPU flags enum (and related data) I wonder whether we wouldn't
better keep all AVX512* together. H.J. - do you have an opinion there
either way?

>  opcodes/i386-opc.h             |   7 +
>  opcodes/i386-opc.tbl           | 376 +++++++++++++++++++++

May I suggest SpaceEVexMap{5,6} to become just EVexMap{5,6}?
The table entries are hard enough to read, being partly far over 200
columns. Any unambiguous shortening of names is a win imo.

You appear to be adding IgnoreSize to most (all?) SAE templates. May
I ask why that is? I've taken quite a bit of time over the last
couple of years to remove stray uses, and I'd really prefer if no
unneeded new ones got introduced.

Some SAE templates have Disp8MemShift settings, despite these not
allowing memory operands in the first place.

You seem to be using a mixture of new-style shorthands (e.g. VexW0)
and old-style unreadable forms (e.g. VexW=2). Please use only
new-style variants (where ones exist already).

While purely cosmetic, I'm puzzled here (and I've been puzzled by
many other similar pre-existing inconsistencies) by the mixture of
operand attribute ordering. I'd really appreciate if you could
standardize on one form in all of the additions here, e.g.

RegXMM|Dword|Unspecified|BaseIndex

(that is: register specifier(s) [of course also always in the same
order when there are multiple], memory specifiers, and BaseIndex in
this order). Getting this uniform (eventually) helps when searching
for patterns or when comparing templates.

There looks to be a redundant vcmpph template.

vcvtqq2ph* and vcvtuqq2ph* would imo better move up next to the
other v*2ph group (initially I was suspecting they might be missing
altogether). This then makes it easier to see their similarity with
vcvtpd2ph*.

vcvtph2pd may need to have their AVX512VL templates split: Having
e.g. Word|RegXMM|Dword means Word is the broadcast form and both
XmmWord and Dword can be used with non-broadcast memory operands.
But that's not true - in Intel syntax only "dword ptr" ought to be
valid there, afaict. Same for vcvtph2qq (where the group also has
a stray blank line in the middle), vcvtph2uqq, vcvttph2qq, and
vcvttph2uqq. (The similarity of such seems to call for
templatization, but you may not be up to going this far.)

Like vcvtph2ps I think you can have a single template for the
512-bit form of vcvtph2psx handling both register and memory
operands. Assuming you started there, it's not clear to me why
you thought you'd need to split the template. (Also please don't
have double blank lines anywhere; right here they should all go
together anyway imo.) vcvtph2dq et al then should match the
resulting set here; again templatization may help avoid subtle
differences for sufficiently similar insns.

There looks to be a stray register-only (but not SAE) vcvtph2uw
template, when the prior one already handles register sources
alongside memory ones.

vcvtsi2sh and vcvtusi2sh specify Reg32|Reg64 and Word|Dword.
The former already implies the latter, so please drop the
redundant specifiers. Also their IntelSyntax SAE forms have
their operands in wrong order - see vcvtsi2ss/vcvtusi2ss.

Can vmulph and vmulsh please live next to each other?

So far for the assembler side of things; I'll see to get to the
disassembler part as well.

Jan