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

[Jan Beulich <jbeulich@suse.com>]

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).

Jan