Re: [PATCH v3 5/9] Add tests for APX GPR32 with extend evex prefix

[Jan Beulich <jbeulich@suse.com>]

On 11.12.2023 07:16, Cui, Lili wrote:
>> On 24.11.2023 08:02, Cui, Lili wrote:
>>> +#VEX without evex
>>> +	vaesimc (%r27), %xmm3
>>> +	vaeskeygenassist $7,(%r27),%xmm3
>>> +	vblendpd $7,(%r27),%xmm6,%xmm2
>>> +	vblendpd $7,(%r27),%ymm6,%ymm2
>>> +	vblendps $7,(%r27),%xmm6,%xmm2
>>> +	vblendps $7,(%r27),%ymm6,%ymm2
>>> +	vblendvpd %xmm4,(%r27),%xmm2,%xmm7
>>> +	vblendvpd %ymm4,(%r27),%ymm2,%ymm7
>>> +	vblendvps %xmm4,(%r27),%xmm2,%xmm7
>>> +	vblendvps %ymm4,(%r27),%ymm2,%ymm7
>>> +	vdppd $7,(%r27),%xmm6,%xmm2
>>> +	vdpps $7,(%r27),%xmm6,%xmm2
>>> +	vdpps $7,(%r27),%ymm6,%ymm2
>>> +	vhaddpd (%r27),%xmm6,%xmm5
>>> +	vhaddpd (%r27),%ymm6,%ymm5
>>> +	vhsubps (%r27),%xmm6,%xmm5
>>> +	vhsubps (%r27),%ymm6,%ymm5
>>> +	vlddqu (%r27),%xmm4
>>> +	vlddqu (%r27),%ymm4
>>> +	vldmxcsr (%r27)
>>> +	vmaskmovpd %xmm4,%xmm6,(%r27)
>>> +	vmaskmovpd %ymm4,%ymm6,(%r27)
>>> +	vmaskmovpd (%r27),%xmm4,%xmm6
>>> +	vmaskmovpd (%r27),%ymm4,%ymm6
>>> +	vmaskmovps %xmm4,%xmm6,(%r27)
>>> +	vmaskmovps %ymm4,%ymm6,(%r27)
>>> +	vmaskmovps (%r27),%xmm4,%xmm6
>>> +	vmaskmovps (%r27),%ymm4,%ymm6
>>> +	vmovmskpd %xmm4,%r27d
>>> +	vmovmskpd %xmm8,%r27d
>>> +	vmovmskps %xmm4,%r27d
>>> +	vmovmskps %ymm8,%r27d
>>> +	vpblendd $7,(%r27),%xmm6,%xmm2
>>> +	vpblendd $7,(%r27),%ymm6,%ymm2
>>> +	vpblendvb %xmm4,(%r27),%xmm2,%xmm7
>>> +	vpblendvb %ymm4,(%r27),%ymm2,%ymm7
>>> +	vpblendw $7,(%r27),%xmm6,%xmm2
>>> +	vpblendw $7,(%r27),%ymm6,%ymm2
>>> +	vpcmpeqb (%r26),%ymm6,%ymm2
>>> +	vpcmpeqd (%r26),%ymm6,%ymm2
>>> +	vpcmpeqq (%r16),%ymm6,%ymm2
>>> +	vpcmpeqw (%r16),%ymm6,%ymm2
>>> +	vpcmpestri $7,(%r27),%xmm6
>>> +	vpcmpestrm $7,(%r27),%xmm6
>>> +	vpcmpgtb (%r26),%ymm6,%ymm2
>>> +	vpcmpgtd (%r26),%ymm6,%ymm2
>>> +	vpcmpgtq (%r16),%ymm6,%ymm2
>>> +	vpcmpgtw (%r16),%ymm6,%ymm2
>>> +	vpcmpistri $100,(%r25),%xmm6
>>> +	vpcmpistrm $100,(%r25),%xmm6
>>> +	vperm2f128 $7,(%r27),%ymm6,%ymm2
>>> +	vperm2i128 $7,(%r27),%ymm6,%ymm2
>>> +	vphaddd (%r27),%xmm6,%xmm7
>>> +	vphaddd (%r27),%ymm6,%ymm7
>>> +	vphaddsw (%r27),%xmm6,%xmm7
>>> +	vphaddsw (%r27),%ymm6,%ymm7
>>> +	vphaddw (%r27),%xmm6,%xmm7
>>> +	vphaddw (%r27),%ymm6,%ymm7
>>> +	vphminposuw (%r27),%xmm6
>>> +	vphsubd (%r27),%xmm6,%xmm7
>>> +	vphsubd (%r27),%ymm6,%ymm7
>>> +	vphsubsw (%r27),%xmm6,%xmm7
>>> +	vphsubsw (%r27),%ymm6,%ymm7
>>> +	vphsubw (%r27),%xmm6,%xmm7
>>> +	vphsubw (%r27),%ymm6,%ymm7
>>> +	vpmaskmovd %xmm4,%xmm6,(%r27)
>>> +	vpmaskmovd %ymm4,%ymm6,(%r27)
>>> +	vpmaskmovd (%r27),%xmm4,%xmm6
>>> +	vpmaskmovd (%r27),%ymm4,%ymm6
>>> +	vpmaskmovq %xmm4,%xmm6,(%r27)
>>> +	vpmaskmovq %ymm4,%ymm6,(%r27)
>>> +	vpmaskmovq (%r27),%xmm4,%xmm6
>>> +	vpmaskmovq (%r27),%ymm4,%ymm6
>>> +	vpmovmskb %xmm4,%r27
>>> +	vpmovmskb %ymm4,%r27d
>>> +	vpsignb (%r27),%xmm6,%xmm7
>>> +	vpsignb (%r27),%xmm6,%xmm7
>>> +	vpsignd (%r27),%xmm6,%xmm7
>>> +	vpsignd (%r27),%xmm6,%xmm7
>>> +	vpsignw (%r27),%xmm6,%xmm7
>>> +	vpsignw (%r27),%xmm6,%xmm7
>>> +	vptest (%r27),%ymm6
>>> +	vrcpps (%r27),%xmm6
>>> +	vrcpps (%r27),%ymm6
>>> +	vrcpss (%r27),%xmm6,%xmm6
>>> +	vroundpd $1,(%r24),%xmm6
>>> +	vroundps $2,(%r24),%xmm6
>>> +	vroundsd $3,(%r24),%xmm6,%xmm3
>>> +	vroundss $4,(%r24),%xmm6,%xmm3
>>
>> There's still the pending question of whether these really need to be treated
>> as invalid (rather than being converted to VRNDSCALE*). Also (to a lesser
>> degree) for {LD,ST}MXCSR.
>>
> 
> GCC already performs these conversions, and many instructions require this. it has converted vstmxcsr/vldmxcsr to ldmxcsr/stmxcsr under APX.

What other instructions are covered by "many"? I don't see a similar pattern
applying for other than the named ones.

Also, how does it help an assembler programmer if gcc already does the
conversion? Or even a C programmer using inline assembly? It's still not
really clear to me how inline assembly is going to be dealt with in a
fully flexible, yet sufficiently restricting way. Hence any help that
can be provided to avoid non-standard constructs ought to be put in place
(imo).

>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
>>> @@ -0,0 +1,28 @@
>>> +# Check Illegal prefix for 64bit EVEX-promoted instructions
>>> +
>>> +        .allow_index_reg
>>> +        .text
>>> +_start:
>>> +	#movbe %r23w,%ax set EVEX.pp = f3 (illegal value).
>>> +	.insn EVEX.L0.f3.M12.W0 0x60, %di, %ax
>>> +	#movbe %r23w,%ax set EVEX.pp = f2 (illegal value).
>>> +	.insn EVEX.L0.f2.M12.W0 0x60, %di, %ax
>>> +	#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
>>
>> For the purpose of this test (whatever P[10] again is) you don't need a 32-bit
>> displacement, do you? Shorter is (almost always) better in such tests.
>>
> 
> P[10] is a fixed value, in normal EVEX format we don't use this bit.  Dropped 0x7b.
> 
>>> +	#EVEX_MAP4 movbe %r23w,%ax set EVEX.mm == b01 (illegal value).
>>> +	.insn EVEX.L0.66.M13.W0 0x60, %di, %ax
>>> +	#EVEX_MAP4 movbe %r23w,%ax set EVEX.aa(P[17:16]) == b01 (illegal
>> value).
>>
>> There's aaa, but no aa afaik.
>>
> 
> Change it to EVEX.a1a0, aaa is split into two parts in EVEX-promoted format, a3 is NF and a1a0 is a fixed value.
> 
> EVEX.a1a0 (P[17:16]) == b01

But a1a0 isn't a term documentation uses either. Just to repeat an earlier
request of mine: These comments need to be easy to decipher and follow.
Hence they want to use as easily understandable terminology as possible.
One way to express what you're after may be "EVEX.aaa[1:0] (P[17:16])".
I'm sure there are further ways while stay in line with what the SDM uses.

>>> +	.insn EVEX.L0.66.M12.W0 0x60, %di, %ax{%k1}
>>> +	#EVEX_MAP4 movbe %r18w,%ax set EVEX.zL'L == 0b11 (illegal value).
>>
>> How's z relevant when the value is just a 2-bit one? And then z should likely
>> have a separate test (also for the from-VEX case below)?
>>
> 
> Modified it and added EVEX.z testcase for MAP4 and from-VEX.
> 
>>> +	.insn EVEX.L0.66.M12.W0 0x60, %di, {rd-sae}, %ax
>>> +	#EVEX from VEX bzhi %ebx,%eax,%ecx EVEX.P[17:16](EVEX.aa) == 1
>> (illegal value).
>>> +	.insn EVEX.L0.NP.0f38.W0 0xf5, %eax, %ebx, %ecx{%k1}
>>> +	.byte 0xff, 0xff, 0xff
>>> +	#EVEX from VEX bzhi %ebx,%eax,%ecx EVEX.P[22:21](EVEX.L’L) == 1
>> (illegal value).
>>> +	.insn EVEX.L0.NP.0f38.W0 0xf5, %eax, {rd-sae}, %ebx, %ecx
>>> +	.byte 0xff, 0xff, 0xff
>>
>> If you arranged for a ModR/M byte of 0xc9 (among other possibilities) in both
>> of these cases, you could avoid the .byte lines altogether afaict.
>>
>  
> Use other value instead of 0xc9,
> 
>         #EVEX from VEX bzhi %rax,(%rax,%rbx),%rcx EVEX.P[17:16](EVEX.aa) == 0b01
>         #(illegal value).
>         .insn EVEX.L0.NP.0f38.W0 0xf5, %rax, (%rax,%rbx), %rcx{%k1}
>         #EVEX from VEX bzhi %rax,(%rax,%rbx),%ecx EVEX.P[22:21](EVEX.L’L) == 0b01
>         #(illegal value).
>         .insn EVEX.L1.NP.0f38.W0 0xf5, %rax, (%rax,%rbx), %rcx
>         #EVEX from VEX bzhi %rax,(%rax,%rbx),%rcx EVEX.P[23](EVEX.z) == 0b1
>         #(illegal value).
>         .insn EVEX.L0.NP.0f38.W0 0xf5, %rax, (%rax,%rbx), %rcx {%k7}{z}
>         #EVEX from VEX bzhi %rax,(%rax,%rbx),%rcx EVEX.P[20](EVEX.b) == 0b1
>         #(illegal value).
>         .insn EVEX.L0.NP.0f38.W0 0xf5, %rax ,(%rax,%rbx){1to8}, %rcx       

Hmm, yes, these are memory operands now. I didn't check what ModR/M bytes these
specifically encode to, but with the .byte gone I expect things are better now.

Btw, readability of these would greatly improve if between each .insn and the
following comment there was a blank line. That way what belongs together and
what is separate can be spotted at the first glance.

Jan