x86: Support Intel AVX VNNI

[H.J. Lu]

On Thu, Oct 15, 2020 at 12:24 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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.

AVX VNNI always requires the {vex} prefix.  It isn't optional.
It is similar to

vmovdqu32 %xmm5, %xmm6

vs

vmovdqu %xmm5, %xmm6

It is the 32 suffix vs the {vex} 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.

Please only keep the AT&T syntax test.


-- 
H.J.