x86: Support Intel AVX VNNI

[Jan Beulich]

On 14.10.2020 08:37, Cui, Lili wrote:
> Subject: [PATCH] x86: Support Intel AVX VNNI
> 
> Intel AVX VNNI instructions are marked with CpuVEX_PREFIX.  Without the
> pseudo {vex} prefix, mnemonics of Intel VNNI instructions are encoded
> with the EVEX prefix.  The pseudo {vex} prefix can be used to encode
> mnemonics of Intel VNNI instructions with the VEX prefix.

I take it this is (on the gas side) to avoid breaking existing code
by suddenly switching from EVEX- to VEX-encoding. Ugly, but well ...
(I imply that there are or are going to be CPUs with AVX512_VNNI but
without AVX_VNNI, or else all of this would be quite pointless.) On
the disassembler side, though, I don't see at all why the {vex3}
thingy needs printing. We don't do so for other mnemonics allowing
perhaps all of 2-byte VEX, 3-byte VEX, and EVEX encoding.

I wonder anyway why these separate encodings are being introduced
after their EVEX ones. Is this just because in many cases the
encodings are shorter? If so, why isn't this done more consistently
for all AVX512 insns not (yet) having VEX encoded counterparts?

> @@ -1964,7 +1967,14 @@ cpu_flags_match (const insn_template *t)
>        cpu = cpu_flags_and (x, cpu);
>        if (!cpu_flags_all_zero (&cpu))
>  	{
> -	  if (x.bitfield.cpuavx)
> +	  if (x.bitfield.cpuvex_prefix)
> +	    {
> +	      /* We need to check a few extra flags with VEX_PREFIX.  */
> +	      if (i.vec_encoding == vex_encoding_vex
> +		  || i.vec_encoding == vex_encoding_vex3)
> +		match |= CPU_FLAGS_ARCH_MATCH;
> +	    }
> +	  else if (x.bitfield.cpuavx)

Is this (including the new cpuvex_prefix attribute, which imo shouldn't
be a Cpu* bit) really needed? Couldn't you achieve the same by placing
the templates _after_ the AVX512 counterparts? Iirc templates get
tried in order, and the first match wins. The {vex3} prefix would then
prevent a match on the EVEX-encoded AVX512_VNNI templates.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/avx-vnni.s
> @@ -0,0 +1,22 @@
> +	.allow_index_reg
> +
> +.macro test_insn mnemonic
> +	\mnemonic	%xmm2, %xmm4, %xmm2
> +	{evex} \mnemonic %xmm2, %xmm4, %xmm2
> +	{vex}  \mnemonic %xmm2, %xmm4, %xmm2
> +	{vex2} \mnemonic %xmm2, %xmm4, %xmm2
> +	{vex3} \mnemonic %xmm2, %xmm4, %xmm2
> +	{vex}  \mnemonic (%ecx), %xmm4, %xmm2
> +	{vex2} \mnemonic (%ecx), %xmm4, %xmm2
> +	{vex3} \mnemonic (%ecx), %xmm4, %xmm2
> +.endm

I question the {vex2} cases here: Bot {vex} and {vex3} are
mandatory prefixes, while {vex2} has been deprecated (and it's
not even documented anymore). If anything, {vex2} should result
in an error here. But for simplicity omitting the tests now
(allowing the case to become an error later on) would be fine
with me.

> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -3904,6 +3904,16 @@ vpshrdw, 4, 0x6672, None, 1, CpuAVX512_VBMI2, Modrm|Masking=3|OpcodePrefix=2|Vex
>  
>  // AVX512_VBMI2 instructions end
>  
> +// AVX_VNNI instructions
> +
> +vpdpbusd, 3, 0x6650, None, 1, CpuAVX_VNNI|CpuVEX_PREFIX, Modrm|Vex|OpcodePrefix=1|VexVVVV=1|VexW=1|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex|RegXMM|RegYMM, RegXMM|RegYMM, RegXMM|RegYMM }
> +vpdpwssd, 3, 0x6652, None, 1, CpuAVX_VNNI|CpuVEX_PREFIX, Modrm|Vex|OpcodePrefix=1|VexVVVV=1|VexW=1|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex|RegXMM|RegYMM, RegXMM|RegYMM, RegXMM|RegYMM }
> +
> +vpdpbusds, 3, 0x6651, None, 1, CpuAVX_VNNI|CpuVEX_PREFIX, Modrm|Vex|OpcodePrefix=1|VexVVVV=1|VexW=1|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex|RegXMM|RegYMM, RegXMM|RegYMM, RegXMM|RegYMM }
> +vpdpwssds, 3, 0x6653, None, 1, CpuAVX_VNNI|CpuVEX_PREFIX, Modrm|Vex|OpcodePrefix=1|VexVVVV=1|VexW=1|CheckRegSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { Unspecified|BaseIndex|RegXMM|RegYMM, RegXMM|RegYMM, RegXMM|RegYMM }

At the very least please use more "modern" (and more meaningful /
readable) constructs where available, e.g. VexW0 instead of VexW=1.
But in new code I'd like to further encourage to avoid any
attributes of the form <name>=<value>, and instead introduce
suitable #define-s instead. Since we're already transforming the
code (slowly, but still), this will reduce future churn. (For
VexVVVV=1 please note that this is equivalent to just VexVVVV
anyway.)

Jan