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

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

> On 24.11.2023 08:02, Cui, Lili wrote:
> > --- a/gas/testsuite/gas/i386/x86-64-apx-egpr-inval.l
> > +++ b/gas/testsuite/gas/i386/x86-64-apx-egpr-inval.l
> > @@ -12,4 +12,192 @@
> >  .*:16: Error: unsupported extended GPR for addressing for `xsaveopt64'
> >  .*:17: Error: unsupported extended GPR for addressing for `xsavec'
> >  .*:18: Error: unsupported extended GPR for addressing for `xsavec64'
> > +.*:20: Error: unsupported extended GPR for addressing for `blendpd'
> > +.*:21: Error: unsupported extended GPR for addressing for `blendps'
> > +.*:22: Error: unsupported extended GPR for addressing for `blendvpd'
> > +.*:23: Error: unsupported extended GPR for addressing for `blendvpd'
> > +.*:24: Error: unsupported extended GPR for addressing for `blendvps'
> > +.*:25: Error: unsupported extended GPR for addressing for `blendvps'
> > +.*:26: Error: unsupported extended GPR for addressing for `dppd'
> > +.*:27: Error: unsupported extended GPR for addressing for `dpps'
> 
> Seeing this diagnostic in action, I'm afraid I think this would better be closer to
> the diagnostics i386_index_check() issues. E.g.
> "extended GPR cannot be used as base/index for ...".
> 

Done.

> > --- a/gas/testsuite/gas/i386/x86-64-apx-egpr-inval.s
> > +++ b/gas/testsuite/gas/i386/x86-64-apx-egpr-inval.s
> > @@ -1,4 +1,4 @@
> > -# Check Illegal 64bit APX_F instructions
> > +# Check illegal 64bit APX_F instructions
> 
> ???
> 

Changed in the original patch.

> > +#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.

> > +	vrsqrtps (%r27),%xmm6
> > +	vrsqrtps (%r27),%ymm6
> > +	vrsqrtss (%r27),%xmm6,%xmm6
> > +	vstmxcsr (%r27)
> > +	vtestpd (%r27),%xmm6
> > +	vtestpd (%r27),%ymm6
> > +	vtestps (%r27),%xmm6
> > +	vtestps (%r27),%ymm6
> > +	vtestps (%r27),%ymm6
> > +	vptest (%r27),%xmm6
> 
> This one wants moving up, now that sorting was mostly done.
> 

Done.

> > --- /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

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

Thanks,
Lili.