[PATCH v5 4/5] x86: correct VFPCLASSP{S,D} operand size handling
H.J. Lu
hjl.tools@gmail.com
Tue Feb 11 12:56:00 GMT 2020
On Tue, Feb 11, 2020 at 4:49 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 11.02.2020 12:49, H.J. Lu wrote:
> > 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?
>
> There aren't "many" changes, it's just one larger block of code that
> needs adding (paralleling the suffix checking for GPR(-like) operands).
>
> > If they are really needed, can you make them
> > conditioned on Intel
> > syntax?
>
> Well, that block of code is the meat of the change, so yes, it is
> needed. Some of what it does may also be beneficial for AT&T mode,
Do you have a testcase to show that it is useful for AT&T syntax?
If not, please enable the code path only for Intel syntax with a
comment.
> but yes, I think I can make all of it Intel-syntax only (with a
> comment saying why this is).
>
> Jan
--
H.J.
More information about the Binutils
mailing list