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

[Jan Beulich <jbeulich@suse.com>]

On 16.11.2023 16:34, Cui, Lili wrote:
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-egpr-promote-inval.s
>>> @@ -0,0 +1,17 @@
>>> +# Check illegal 64bit APX EVEX promoted instructions
>>> +	.text
>>> +	.arch .nomovbe
>>> +	movbe (%r16), %r17
>>> +	movbe (%rax), %rcx
>>> +	.arch .noept
>>> +	invept (%r16), %r17
>>> +	invept (%rax), %rcx
>>> +	.arch .noavx512bw
>>> +	kmovq %k1, (%r16)
>>> +	kmovq %k1, (%r8)
>>> +	.arch .noavx512dq
>>> +	kmovb %k1, %r16d
>>> +	kmovb %k1, %r8d
>>> +	.arch .noavx512f
>>> +	kmovw %k1, %r16d
>>> +	kmovw %k1, %r8d
>>
>> What about BMI/BMI2 insns? Or AMX ones? (I surely missed further groups.)
> 
> We don’t want to list all the instructions here, just a few representatives.

Sure. I'm asking for representatives from the BMI and BMI2 groups.

>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
>>> @@ -0,0 +1,29 @@
> # Check Illegal prefix for 64bit EVEX-promoted instructions
> 
>         .allow_index_reg
>         .text
> _start:
>         #movbe %r18w,%ax set EVEX.pp = f3 (illegal value).
>         .byte 0x62, 0xfc, 0x7e, 0x08, 0x60, 0xc2
>         .byte 0xff, 0xff
>         #movbe %r18w,%ax set EVEX.pp = f2 (illegal value).
>         .byte 0x62, 0xfc, 0x7f, 0x08, 0x60, 0xc2
>         .byte 0xff, 0xff
>         #VSIB vpgatherqq 0x7b(%rbp,%zmm17,8),%zmm16{%k1} set EVEX.P[10] == 0
>         #(illegal value).
>         .byte 0x62, 0xe2, 0xf9, 0x41, 0x91, 0x84, 0xcd, 0x7b, 0x00, 0x00, 0x00
>         .byte 0xff
>         #EVEX_MAP4 movbe %r18w,%ax set EVEX.mm == b01 (illegal value).
>         .byte 0x62, 0xfd, 0x7d, 0x08, 0x60, 0xc2
>         .byte 0xff, 0xff
>         #EVEX_MAP4 movbe %r18w,%ax set EVEX.aa(P[17:16]) == b01 (illegal value).
>         .byte 0x62, 0xfd, 0x7d, 0x09, 0x60, 0xc2
>         .byte 0xff, 0xff
>         #EVEX_MAP4 movbe %r18w,%ax set EVEX.zL'L == b001 (illegal value).
>         .byte 0x62, 0xfd, 0x7d, 0x28, 0x60, 0xc2
>         .byte 0xff, 0xff
>         #EVEX from VEX ldtilecfg 0x123(%r31,%rax,4),%r31 EVEX.P[17:16](EVEX.aa) == 1 (illegal value).
>         .byte 0x62, 0xda, 0x7c, 0x09, 0x49, 0x84, 0x87, 0x23, 0x01, 0x00, 0x00
>         #EVEX from VEX ldtilecfg 0x123(%r31,%rax,4),%r31 EVEX.P[22:21](EVEX.L’L) == 1 (illegal value).
>         .byte 0x62, 0xda, 0x7c, 0x28, 0x49, 0x84, 0x87, 0x23, 0x01, 0x00, 0x00
>         #EVEX from VEX ldtilecfg 0x123(%r31,%rax,4),%r31 EVEX.P[20](EVEX.b) == 1 (illegal value).
>         .byte 0x62, 0xda, 0x7c, 0x18, 0x49, 0x84, 0x87, 0x23, 0x01, 0x00, 0x00
> 
>> I suspect at least some of these can be expressed via .insn, which would
>> greatly help readability (i.e. recognizing what is actually being done, and
>> what's expected-wrong about it).
>>
> 
> Update test cases.
> I try to express the first case using .insn. I can't find a way to express EVEX.P[3:2] == 11, do you have any ideas?
> 
> 0x62, 0xfc  ---> EVEX.P[3:2] of normal EVEX must be 00.

There are terminology issues here again. The first case in the test talks
about EVEX.pp set to the equivalent of an F3 prefix. That's neither
encoded as 11, nor in EVEX.P[3:2] (I don't like the EVEX.P[] notation
anyway), but in EVEX.P[9:8].

Irrespective, these are some examples of what I use to encode MOVBE (note
that all of this Intel syntax and the comments are MASM-style):

	.insn EVEX.L0.66.M12.W0 0x60, di, ax		; movbe di, r16w
	.insn EVEX.L0.66.M12.W0 0x60, di, [rax]		; movbe di, [r16]
	.insn EVEX.L0.M4 0x60, xmm16, rdi		; movbe r16, rdi
	.insn EVEX.L0.M4.W0 0x60, xmm16, [rdi]		; movbe r16d, [rdi]
	.insn EVEX.L0.66.M4.W0 0x61, [rdi], xmm16	; movbe [rdi], r16w
	.insn EVEX.L0.M4 0x61, xmm16, edi		; movbe edi, r16d
	.insn EVEX.L0.M12 0x61, [rax], rdi		; movbe [r16], rdi

Surely you can find variations to support the forms you're after. Plus
if you think the .insn documentation is unclear, please point out what
you think needs improving.

>>> [...]
>>> +	crc32	r22,r31
>>> +	crc32	r22,QWORD PTR [r31]
>>> +	crc32	r17,r19b
>>> +	crc32	r21d,r19b
>>> +	crc32	ebx,BYTE PTR [r19]
>>> +	crc32	r23d,r31d
>>> +	crc32	r23d,DWORD PTR [r31]
>>> +	crc32	r21d,r31w
>>> +	crc32	r21d,WORD PTR [r31]
>>> +	crc32	r18,rax
>>
>> These could do with moving up, since otherwise things look to be sorted
>> alphabetically here. But seeing these also reminds me that the noreg64 test
>> also needs extending, to cover these new forms (handled by separate
>> templates).
>>
> 
> 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.

Jan