x86: Support Intel AVX VNNI

[Jan Beulich]

On 15.10.2020 09:10, Cui, Lili wrote:
>>>> @@ -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.
>>
>> Lili, please look into it.
>>
> 
> I add an invalid test for it, we need cpuvex_prefix attribute for under scenario.
> 
> .arch .noavx512_vnni
> vpdpbusd %xmm2,%xmm4,%xmm2 
> 
> As without the pseudo {vex} prefix, this instruction should be encoded with EVEX prefix. 
> we should report error for it, I rename CpuVEX_PREFIX to PseudoVexPrefix
> and move it into opcode_modifier bit, thanks.

I disagree, unless AVX-VNNI was specified to have a dependency on
AVX512-VNNI (which would seem pretty odd, as meanwhile I've noticed
that another reason for introducing these encodings may be to allow
their use on AVX512-incapable hardware). The above very much should
result in the VEX encoding despite the absence of a {vex} prefix.
It's really only the default case of everything being enabled where
the pseudo-prefix should be mandated. This particularly implies
that an explicit ".arch .avx_vnni" ought to _also_ eliminate the
need for the pseudo prefix.

> Subject: [PATCH] Enhancement for avx-vnni patch

This title and the fact that the original patch was committed
points out once again that a little bit of time should be given
for people to look at proposed changes. I typically give it at
least over night (day on the other side of the ocean) until I
commit not sufficiently trivial changes, no matter how quickly an
ack arrives.

Irrespective of this - thanks for doing this incremental change.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/avx-vnni-inval.s
> @@ -0,0 +1,9 @@
> +# Check illegal in AVXVNNI instructions
> +
> +	.text
> +	.arch .noavx512_vnni
> +_start:
> +	vpdpbusd %xmm2,%xmm4,%xmm2
> +
> +	.intel_syntax noprefix
> +	vpdpbusd %xmm2,%xmm4,%xmm2

I question the need for Intel syntax tests in test cases like this
one.

Jan