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

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

> Subject: Re: [PATCH 4/8] Add tests for APX GPR32 with extend evex prefix
> 
> 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.
> 

Done.

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

Done.

After your example, I found that M can cover the 4 bits of 00mm in EVEX. I was confused that mm is only 2 bits but can express M0-M15, so I didn’t understand what M meant before. 

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

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)
        adcw $1, (%r31)

        /* Set EVEX.W to 1.  */
        crc32  %rax,%r18
        adc %r15,%r16

Thanks.
Lili.