RE: [PATCH 4/8] Add tests for APX GPR32 with extend evex prefix

["Cui, Lili" <lili.cui@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, November 17, 2023 10:38 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; ccoutant@gmail.com;
> binutils@sourceware.org
> Subject: Re: [PATCH 4/8] Add tests for APX GPR32 with extend evex prefix
> 
> On 17.11.2023 13:42, Cui, Lili wrote:
> >> Subject: Re: [PATCH 4/8] Add tests for APX GPR32 with extend evex
> >> prefix
> >>
> >> On 16.11.2023 16:34, Cui, Lili wrote:
> >>> I'm confused here about adding crc test case in noreg64.s, could you
> >> elaborate on what testcase you want to add?
> >>>
> >>>         pfx crc32       (%rax), %eax
> >>>         pfx16 crc32     (%rax), %rax
> >>> +       pfx crc32       (%r31),%r21d   ---> data size prefix invalid with `crc32'
> >>> +       pfx crc32       (%r31),%r21     ---> data size prefix invalid with `crc32'
> >>
> >> Well, of course you can't use the "pfx" macro (at least not as is),
> >> which will emit a data size prefix when DATA16 is defined. Likewise it
> would emit "rex64"
> >> when REX64 is defined, which doesn't make sense with EVEX-encoded
> insns.
> >> Ideally you would introduce a new macro to control operand size in an
> >> EVEX- like manner, just that I'm afraid that the way you're adding
> >> EVEX- encoding support to gas doesn't offer any means equivalent to
> >> that of legacy encodings. Hence only the "bare" EVEX-encoded insns
> >> (without the use of any
> >> pfx*) should be added for the time being.
> >>
> >> Also, ftaod, CRC32 was only an example here. Any new template you add
> >> which allows for potentially ambiguous operand size will need an
> >> example added here. This set of tests (noreg64*) is intended to be
> >> (and remain) exhaustive.
> >>
> >> Albeit, thinking a little further, perhaps you simply want to
> >> introduce a noreg64-evex.d referencing the same source file, but
> >> arranging for {evex} to be emitted in the pfx macro (or a further
> >> clone thereof, as some of the insns cannot be EVEX-encoded)? That
> >> would then also deal with covering all relevant new templates (I
> >> think). You'd need to check what, if anything, needs doing to the
> >> pfx16 and pfx64 macros. But of course you could also introduce a
> >> fully standalone noreg64-apx.{s,d} test, to escape some of the possible
> hassles.
> >>
> >
> > I listed some tests, most of EVEX-promoted instructions support prefix 66,
> we included all of these test cases in Part II 1/6 (except for crc32 which is
> already listed in the current file). Part II 1/6 it is suspended, because it also
> covers the NF patch instructions.
> >
> >         /* Set EVEX.pp to 66.  */
> >         crc32  %r31w,%r21d
> >         crc32w (%r31),%r21d
> >         adc $1, (%r31w)
> 
> This one ought to be a mistake.
> 
> >         adcw $1, (%r31)
> >
> >         /* Set EVEX.W to 1.  */
> >         crc32  %rax,%r18
> >         adc %r15,%r16
> 
> Of the above most aren't ambiguous as to operand size. The purpose of the
> test (or group of tests) is not so much to check correct encoding (except of
> course to prove correct [aka intended] choice of defaults), but to check that
> all ambiguities are properly detected and reported (with the exception of a
> few where H.J. is of the opinion that they shouldn't be diagnosed in AT&T
> mode, even if that lack of diagnostics had - back at the time - allowed for a gcc
> bug to go unnoticed for quite some time).
> 
> Therefore if e.g. "data16" cannot be used with an insn (as is the case for
> EVEX-encoded ones), there's also no need to have special checking for the
> EVEX.pp=01 case. Thus my suggestion to simply arrange for the pfx macro to
> emit {evex} prefixes (or to clone the test in order to escape issues with insns
> which don't allow for EVEX encodings).
> 

Ok, we still have some apx instructions supported in the NF patch, we will add the entire test case after it.

Thanks,
Lili.