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

H.J. Lu hjl.tools@gmail.com
Fri Aug 9 14:57:00 GMT 2019


On Fri, Aug 9, 2019 at 12:55 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.08.2019 18:22,  H.J. Lu  wrote:
> > On Thu, Aug 8, 2019 at 12:47 AM Jan Beulich <JBeulich@suse.com> wrote:
> >>
> >> On 07.08.2019 20:00,  H.J. Lu  wrote:
> >>> On Wed, Aug 7, 2019 at 1:29 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 06.08.2019 23:11,  H.J. Lu  wrote:
> >>>>> On Tue, Aug 6, 2019 at 7:29 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 seems better to just
> >>>>>> wanrn 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).
> >>>>>>
> >>>>>> gas/
> >>>>>> 2019-08-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/noavx512-2.s, testsuite/gas/i386/noreg16.s,
> >>>>>>            testsuite/gas/i386/noreg32.s, testsuite/gas/i386/noreg64.s: Add
> >>>>>>            VFPCLASS tests.
> >>>>>>            * testsuite/gas/i386/noavx512-2.l, testsuite/gas/i386/noreg16.d,
> >>>>>>            testsuite/gas/i386/noreg16.l, testsuite/gas/i386/noreg32.d,
> >>>>>>            testsuite/gas/i386/noreg32.l, testsuite/gas/i386/noreg64.d,
> >>>>>>            testsuite/gas/i386/noreg64.l: Adjust expectations.
> >>>>>>
> >>>>>> opcodes/
> >>>>>> 2019-08-XX  Jan Beulich  <jbeulich@suse.com>
> >>>>>>
> >>>>>>            * i386-opc.tbl (vfpclasspd, vfpclassps): Add Unspecified.
> >>>>>>            * i386-tbl.h: Re-generate.
> >>>>>>
> >>>>>
> >>>>> We should keep the suffix even if AVX512VL isn't enabled so that
> >>>>> we don't need to check if AVX512VL isn't enabled to interpret the
> >>>>> instruction.
> >>>>
> >>>> But that's wrong (and fixing this is the whole point of this patch).
> >>>> As you've said elsewhere, unambiguous (SIMD in particular) insns
> >>>> should not require any use of suffixes.
> >>>>
> >>>
> >>> When I look at such instruction, I should be able to tell what it is without
> >>> checking if AVX512VL is enabled.
> >>
> >> This entirely depends on the context: When all you think about is
> >> Knights hardware (or AVX512F/AVX512DQ in more general terms), then
> >> seeing (and in particular being _forced_ to use) the suffix is
> >> confusing (wrong). No-one says that in this case the suffix won't
> >> be allowed anymore. The problem here seems to be your use of "I",
> >> when instead you should also be considering how other people may
> >> view things.
> >>
> >> As a general statement: The assembler should accept (without any
> >> diagnostic) anything that's unambiguously mappable to a valid
> >> encoding.
> >>
> >
> > When I debug assembly codes:
> >
> > vfpclasspd $0, (%eax), %k0
> >
> > it is hard to tell the memory operand size.
>
> You're kidding? The presented patch makes no change whatsoever to
> the disassembler (as can also be seen from the testsuite extensions
> it makes). Doing so would actually be quite hard I think without it
> even knowing of the distinction between Knights family and other
> processors, and without having any CPU capabilities attribute
> attached to insns.
>

I can compile assembly codes with -g and use gdb to step
through the assembly code:

Breakpoint 2, __strstr_sse2_unaligned ()
    at ../sysdeps/x86_64/multiarch/strstr-sse2-unaligned.S:22
22 movzbl (%rsi), %eax
(gdb) next
23 testb %al, %al
(gdb)

When I see

22   vfpclasspd $0, (%eax), %k0

I can't tell what the memory size is.

-- 
H.J.



More information about the Binutils mailing list