[PATCH v5 4/5] x86: correct VFPCLASSP{S,D} operand size handling

[H.J. Lu]

On Tue, Feb 11, 2020 at 2:26 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> With AVX512VL disabled (e.g. when writing code for the Knights family
> of processors) these insns aren't ambiguous when used with a memory
> source, and hence should be accepted without suffix or operand size
> specifier. When AVX512VL is enabled, to be consistent with this as
> well as other ambiguous operand size handling it would seem better to
> just warn about the ambiguity in AT&T mode, and still default to 512-bit
> operands (on the assumption that the code may have been written without
> AVX512VL in mind yet), but it was requested to leave AT&T syntax mode
> alone here.
>
> gas/
> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * config/tc-i386.c (avx512): New (at file scope), moved from
>         (check_VecOperands): ... here.
>         (process_suffix): Add [XYZ]MMword operand size handling.
>         * testsuite/gas/i386/avx512dq-inval.s: Add VFPCLASS tests.
>         * testsuite/gas/i386/noavx512-2.s: Add Intel syntax VFPCLASS
>         tests.
>         * testsuite/gas/i386/avx512dq-inval.l,
>         testsuite/gas/i386/noavx512-2.l: Adjust expectations.
>
> opcodes/
> 2020-02-XX  Jan Beulich  <jbeulich@suse.com>
>
>         * i386-opc.tbl (vfpclasspd, vfpclassps): Add Intel sytax form
>         with Unspecified, making the present one AT&T syntax only.
>         * i386-tbl.h: Re-generate.
> ---
> v5: Re-base.
> v4: Restrict to just Intel syntax mode. Re-base.
> v3: Re-base.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -1840,6 +1840,8 @@ cpu_flags_and_not (i386_cpu_flags x, i38
>    return x;
>  }
>
> +static const i386_cpu_flags avx512 = CPU_ANY_AVX512F_FLAGS;
> +
>  #define CPU_FLAGS_ARCH_MATCH           0x1
>  #define CPU_FLAGS_64BIT_MATCH          0x2
>
> @@ -5352,7 +5354,6 @@ check_VecOperands (const insn_template *
>  {
>    unsigned int op;
>    i386_cpu_flags cpu;
> -  static const i386_cpu_flags avx512 = CPU_ANY_AVX512F_FLAGS;
>
>    /* Templates allowing for ZMMword as well as YMMword and/or XMMword for
>       any one operand are implicity requiring AVX512VL support if the actual
> @@ -6447,7 +6448,8 @@ process_suffix (void)
>        /* Accept FLDENV et al without suffix.  */
>        && (i.tm.opcode_modifier.no_ssuf || i.tm.opcode_modifier.floatmf))
>      {
> -      unsigned int suffixes;
> +      unsigned int suffixes, evex = 0;
> +      i386_cpu_flags cpu;
>
>        suffixes = !i.tm.opcode_modifier.no_bsuf;
>        if (!i.tm.opcode_modifier.no_wsuf)
> @@ -6461,7 +6463,55 @@ process_suffix (void)
>        if (flag_code == CODE_64BIT && !i.tm.opcode_modifier.no_qsuf)
>         suffixes |= 1 << 5;
>
> -      /* Are multiple suffixes allowed?  */
> +      /* For [XYZ]MMWORD operands inspect operand sizes.  */
> +      cpu = cpu_flags_and (i.tm.cpu_flags, avx512);
> +      if (!cpu_flags_all_zero (&cpu) && !i.broadcast)
> +       {
> +         unsigned int op;
> +
> +         for (op = 0; op < i.tm.operands; ++op)
> +           {
> +             if (!cpu_arch_flags.bitfield.cpuavx512vl)
> +               {
> +                 if (i.tm.operand_types[op].bitfield.ymmword)
> +                   i.tm.operand_types[op].bitfield.xmmword = 0;
> +                 if (i.tm.operand_types[op].bitfield.zmmword)
> +                   i.tm.operand_types[op].bitfield.ymmword = 0;
> +                 if (!i.tm.opcode_modifier.evex
> +                     || i.tm.opcode_modifier.evex == EVEXDYN)
> +                   i.tm.opcode_modifier.evex = EVEX512;
> +               }
> +
> +             if (i.tm.operand_types[op].bitfield.xmmword
> +                 + i.tm.operand_types[op].bitfield.ymmword
> +                 + i.tm.operand_types[op].bitfield.zmmword < 2)
> +               continue;
> +
> +             /* Any properly sized operand disambiguates the insn.  */
> +             if (i.types[op].bitfield.xmmword
> +                 || i.types[op].bitfield.ymmword
> +                 || i.types[op].bitfield.zmmword)
> +               {
> +                 suffixes &= ~(7 << 6);
> +                 evex = 0;
> +                 break;
> +               }
> +
> +             if ((i.flags[op] & Operand_Mem)
> +                 && i.tm.operand_types[op].bitfield.unspecified)
> +               {
> +                 if (i.tm.operand_types[op].bitfield.xmmword)
> +                   suffixes |= 1 << 6;
> +                 if (i.tm.operand_types[op].bitfield.ymmword)
> +                   suffixes |= 1 << 7;
> +                 if (i.tm.operand_types[op].bitfield.zmmword)
> +                   suffixes |= 1 << 8;
> +                 evex = EVEX512;
> +               }
> +           }
> +       }
> +
> +      /* Are multiple suffixes / operand sizes allowed?  */
>        if (suffixes & (suffixes - 1))
>         {
>           if (intel_syntax
> @@ -6491,6 +6541,8 @@ process_suffix (void)
>                    || (i.tm.base_opcode == 0x63
>                        && i.tm.cpu_flags.bitfield.cpu64))
>             /* handled below */;
> +         else if (evex)
> +           i.tm.opcode_modifier.evex = evex;
>           else if (flag_code == CODE_16BIT)
>             i.suffix = WORD_MNEM_SUFFIX;
>           else if (!i.tm.opcode_modifier.no_lsuf)

So this change only impacts Intel syntax with AVX512VL disabled.  Why
are there so many
assembler changes?  If they are really needed, can you make them
conditioned on Intel
syntax?

-- 
H.J.