Re: [PATCH] x86: fold AVX512-VNNI disassembler entries with AVX-VNNI ones

["H.J. Lu" <hjl.tools@gmail.com>]

On Fri, Oct 14, 2022 at 3:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Make %XV also print the separating blank in the VEX case, while making
> it do nothing for EVEX-encoded insns. This way the AVX-VNNI entries
> can be re-used for AVX512-VNNI, at the same time fixing the lack of
> EVEX.W decoding.
>
> For the AVX-VNNI ones further make sure only VEX.66 forms are actually
> decoded.
> ---
> Irrespective of this change I continue to disagree with the arbitrary
> printing of "{vex}" for the AVX-VNNI insns: If that's meant for
> disambiguation purposes, then EVEX-encoded insns not using EVEX-specific
> functionality by having VEX counterparts (vaddps %xmm0, %xmm0, %xmm0)
> should also be prefixed by "{evex}".

This is done to match the assembler.  There are 3 kinds of VNNI processors:

1. AVX512-VNNI only.
2. AVX-VNNI only.
3. AVX512-VNNI and AVX-VNNI.

Since AVX512-VNNI came out first, all VNNI instructions without a prefix
are encoded as AVX512-VNNI.  The existing VNNI instructions without
a prefix, generated by compiler or hand written, are encoded with EVEX.
If one needs VNNI with VEX encoding, the {vex} prefix should be used.
This applies to any AVX extensions which come after EVEX ones, including
AVX-IFMA.

> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -1755,7 +1755,7 @@ struct dis386 {
>     "XD" => print 'd' if !EVEX or EVEX.W=1, EVEX.W=0 is not a valid encoding
>     "XH" => print 'h' if EVEX.W=0, EVEX.W=1 is not a valid encoding (for FP16)
>     "XS" => print 's' if !EVEX or EVEX.W=0, EVEX.W=1 is not a valid encoding
> -   "XV" => print "{vex3}" pseudo prefix
> +   "XV" => print "{vex} " pseudo prefix
>     "LQ" => print 'l' ('d' in Intel mode) or 'q' for memory operand, cond
>            being false, or no operand at all in 64bit mode, or if suffix_always
>            is true.
> @@ -7545,19 +7545,19 @@ static const struct dis386 vex_w_table[]
>    },
>    {
>      /* VEX_W_0F3850 */
> -    { "%XV vpdpbusd",  { XM, Vex, EXx }, 0 },
> +    { "%XVvpdpbusd",   { XM, Vex, EXx }, PREFIX_DATA },
>    },
>    {
>      /* VEX_W_0F3851 */
> -    { "%XV vpdpbusds", { XM, Vex, EXx }, 0 },
> +    { "%XVvpdpbusds",  { XM, Vex, EXx }, PREFIX_DATA },
>    },
>    {
>      /* VEX_W_0F3852 */
> -    { "%XV vpdpwssd",  { XM, Vex, EXx }, 0 },
> +    { "%XVvpdpwssd",   { XM, Vex, EXx }, PREFIX_DATA },
>    },
>    {
>      /* VEX_W_0F3853 */
> -    { "%XV vpdpwssds", { XM, Vex, EXx }, 0 },
> +    { "%XVvpdpwssds",  { XM, Vex, EXx }, PREFIX_DATA },
>    },
>    {
>      /* VEX_W_0F3858 */
> @@ -10711,22 +10711,29 @@ putop (instr_info *ins, const char *in_t
>         case 'V':
>           if (l == 0)
>             abort ();
> -         else if (l == 1
> -                  && (last[0] == 'L' || last[0] == 'X'))
> +         else if (l == 1)
>             {
> -             if (last[0] == 'X')
> +             switch (last[0])
>                 {
> +               case 'X':
> +                 if (ins->vex.evex)
> +                   break;
>                   *ins->obufp++ = '{';
>                   *ins->obufp++ = 'v';
>                   *ins->obufp++ = 'e';
>                   *ins->obufp++ = 'x';
>                   *ins->obufp++ = '}';
> -               }
> -             else if (ins->rex & REX_W)
> -               {
> +                 *ins->obufp++ = ' ';
> +                 break;
> +               case 'L':
> +                 if (!(ins->rex & REX_W))
> +                   break;
>                   *ins->obufp++ = 'a';
>                   *ins->obufp++ = 'b';
>                   *ins->obufp++ = 's';
> +                 break;
> +               default:
> +                 abort ();
>                 }
>             }
>           else
> --- a/opcodes/i386-dis-evex.h
> +++ b/opcodes/i386-dis-evex.h
> @@ -383,8 +383,8 @@ static const struct dis386 evex_table[][
>      { "vrsqrt14p%XW",  { XM, EXx }, 0 },
>      { "vrsqrt14s%XW",  { XMScalar, VexScalar, EXdq }, PREFIX_DATA },
>      /* 50 */
> -    { "vpdpbusd",      { XM, Vex, EXx }, PREFIX_DATA },
> -    { "vpdpbusds",     { XM, Vex, EXx }, PREFIX_DATA },
> +    { VEX_W_TABLE (VEX_W_0F3850) },
> +    { VEX_W_TABLE (VEX_W_0F3851) },
>      { PREFIX_TABLE (PREFIX_EVEX_0F3852) },
>      { PREFIX_TABLE (PREFIX_EVEX_0F3853) },
>      { "vpopcnt%BW",    { XM, EXx }, PREFIX_DATA },
> --- a/opcodes/i386-dis-evex-prefix.h
> +++ b/opcodes/i386-dis-evex-prefix.h
> @@ -233,14 +233,14 @@
>    {
>      { Bad_Opcode },
>      { "vdpbf16p%XS", { XM, Vex, EXx }, 0 },
> -    { "vpdpwssd",      { XM, Vex, EXx }, 0 },
> +    { VEX_W_TABLE (VEX_W_0F3852) },
>      { "vp4dpwssd",     { XM, Vex, EXxmm }, 0 },
>    },
>    /* PREFIX_EVEX_0F3853 */
>    {
>      { Bad_Opcode },
>      { Bad_Opcode },
> -    { "vpdpwssds",     { XM, Vex, EXx }, 0 },
> +    { VEX_W_TABLE (VEX_W_0F3853) },
>      { "vp4dpwssds",    { XM, Vex, EXxmm }, 0 },
>    },
>    /* PREFIX_EVEX_0F3868 */

OK.

-- 
H.J.