public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Cui, Lili" <lili.cui@intel.com>
Cc: hongjiu.lu@intel.com, binutils@sourceware.org
Subject: Re: [PATCH v4 4/9] Add tests for APX GPR32 with extend evex prefix
Date: Fri, 22 Dec 2023 15:41:19 +0100	[thread overview]
Message-ID: <e176f762-bfc0-49c5-adc2-fd446f172a1c@suse.com> (raw)
In-Reply-To: <20231219121218.974012-5-lili.cui@intel.com>

On 19.12.2023 13:12, Cui, Lili wrote:
> --- a/gas/testsuite/gas/i386/x86-64-apx-egpr-inval.s
> +++ b/gas/testsuite/gas/i386/x86-64-apx-egpr-inval.s
> @@ -16,3 +16,194 @@
>  	xsaveopt64 (%r16, %r31)
>  	xsavec (%r16, %rbx)
>  	xsavec64 (%r16, %r31)
> +#SSE
> +	blendpd $100,(%r18),%xmm6
> +	blendps $100,(%r18),%xmm6
> +	blendvpd %xmm0,(%r19),%xmm6
> +	blendvpd (%r19),%xmm6
> +	blendvps %xmm0,(%r19),%xmm6
> +	blendvps (%r19),%xmm6
> +	dppd $100,(%r20),%xmm6
> +	dpps $100,(%r20),%xmm6
> +	extractps $100,%xmm4,%r21
> +	extractps $100,%xmm4,(%r21)
> +	insertps $100,(%r21),%xmm6
> +	movntdqa (%r21),%xmm4
> +	mpsadbw $100,(%r21),%xmm6
> +	pabsb (%r17),%xmm0
> +	pabsd (%r17),%xmm0
> +	pabsw (%r17),%xmm0
> +	packusdw (%r21),%xmm6
> +	palignr $100,(%r17),%xmm6
> +	pblendvb %xmm0,(%r22),%xmm6
> +	pblendvb (%r22),%xmm6
> +	pblendw $100,(%r22),%xmm6
> +	pcmpeqq (%r22),%xmm6
> +	pcmpestri $100,(%r25),%xmm6
> +	pcmpestrm $100,(%r25),%xmm6
> +	pcmpgtq (%r25),%xmm4
> +	pcmpistri $100,(%r25),%xmm6
> +	pcmpistrm $100,(%r25),%xmm6
> +	pextrb $100,%xmm4,%r22
> +	pextrb $100,%xmm4,(%r22)
> +	pextrd $100,%xmm4,(%r22)
> +	pextrq $100,%xmm4,(%r22)
> +	pextrw $100,%xmm4,(%r22)
> +	phaddd  (%r17),%xmm0
> +	phaddsw (%r17),%xmm0
> +	phaddw  (%r17),%xmm0
> +	phminposuw (%r23),%xmm4
> +	phsubw (%r17),%xmm0
> +	pinsrb $100,%r23,%xmm4
> +	pinsrb $100,(%r23),%xmm4
> +	pinsrd $100, %r23d, %xmm4
> +	pinsrd $100,(%r23),%xmm4
> +	pinsrq $100, %r24, %xmm4
> +	pinsrq $100,(%r24),%xmm4
> +	pmaddubsw (%r17),%xmm0
> +	pmaxsb (%r24),%xmm6
> +	pmaxsd (%r24),%xmm6
> +	pmaxud (%r24),%xmm6
> +	pmaxuw (%r24),%xmm6
> +	pminsb (%r24),%xmm6
> +	pminsd (%r24),%xmm6
> +	pminud (%r24),%xmm6
> +	pminuw (%r24),%xmm6
> +	pmovsxbd (%r24),%xmm4
> +	pmovsxbq (%r24),%xmm4
> +	pmovsxbw (%r24),%xmm4
> +	pmovsxbw (%r24),%xmm4
> +	pmovsxdq (%r24),%xmm4
> +	pmovsxwd (%r24),%xmm4
> +	pmovsxwq (%r24),%xmm4
> +	pmovzxbd (%r24),%xmm4
> +	pmovzxbq (%r24),%xmm4
> +	pmovzxdq (%r24),%xmm4
> +	pmovzxwd (%r24),%xmm4
> +	pmovzxwq (%r24),%xmm4
> +	pmuldq (%r24),%xmm4
> +	pmulhrsw (%r17),%xmm0
> +	pmulld (%r24),%xmm4
> +	pshufb (%r17),%xmm0
> +	psignb (%r17),%xmm0
> +	psignd (%r17),%xmm0
> +	psignw (%r17),%xmm0
> +	roundpd $100,(%r24),%xmm6
> +	roundps $100,(%r24),%xmm6
> +	roundsd $100,(%r24),%xmm6
> +	roundss $100,(%r24),%xmm6
> +#AES
> +	aesdec (%r26),%xmm6
> +	aesdeclast (%r26),%xmm6
> +	aesenc (%r26),%xmm6
> +	aesenclast (%r26),%xmm6
> +	aesimc (%r26),%xmm6
> +	aeskeygenassist $100,(%r26),%xmm6
> +	pclmulhqhqdq (%r26),%xmm6
> +	pclmulhqlqdq (%r26),%xmm6
> +	pclmullqhqdq (%r26),%xmm6
> +	pclmullqlqdq (%r26),%xmm6
> +	pclmulqdq $100,(%r26),%xmm6
> +#GFNI
> +	gf2p8affineinvqb $100,(%r26),%xmm6
> +	gf2p8affineqb $100,(%r26),%xmm6
> +	gf2p8mulb (%r26),%xmm6
> +#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
> +	vptest (%r27),%xmm6
> +	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

These are still here, when they can be expressed.

> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-evex-promoted-bad.s
> @@ -0,0 +1,34 @@
> +# 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 (%rbp,%zmm17,8),%zmm16{%k1} set EVEX.P[10] == 0
> +	#(illegal value).
> +	.byte 0x62, 0xe2, 0xf9, 0x41, 0x91, 0x84, 0xcd
> +	.byte 0xff

This one's still using .byte and still referencing P[10] in the comment.
If that's really unavoidable, then - as elsewhere - the description of
the patch could (and should) provide the reason. And there continue to
be no separating blank lines, making it as hard as before to find ones
way through all of this.

> +	#EVEX_MAP4 movbe %r23w,%ax set EVEX.mm == 0b01 (illegal value).
> +	.insn EVEX.L0.66.M13.W0 0x60, %di, %ax
> +	#EVEX_MAP4 movbe %r23w,%ax set EVEX.a1a0 (P[17:16]) == 0b01

See my earlier comment about a1a0. Once again - please can you use SDM
terms as much as possible, and prefer descriptive names over non-
descriptive ones?

> +        #(illegal value).

??? At the very least indentation is bogus here, but as to this
particular comment (recurring elsewhere) - isn't the entire test about
illegal values?

> +	.insn EVEX.L0.66.M12.W0 0x60, %di, %ax{%k1}
> +	#EVEX_MAP4 movbe %r18w,%ax set EVEX.L'L == 0b01 (illegal value).
> +	.insn EVEX.L1.66.M12.W0 0x60, %di, %ax
> +	#EVEX_MAP4 movbe %r18w,%ax set EVEX.z == 0b1 (illegal value).
> +	.insn EVEX.L0.66.M12.W0 0x60, %di, %ax {%k7}{z}
> +	#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}

Isn't {%k7} alone rendering the encoding invalid, which was checked
for above already? Also - bogus indentation again.

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

64-bit register operands with .W0 are kind of okay, but still odd to
see. Note however the one misplaced comma here.

Jan

  reply	other threads:[~2023-12-22 14:41 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-19 12:12 [PATCH v4 0/9] Support Intel APX EGPR Cui, Lili
2023-12-19 12:12 ` [PATCH v4 1/9] Support APX GPR32 with rex2 prefix Cui, Lili
2023-12-22 13:08   ` Jan Beulich
2023-12-25  6:14     ` Cui, Lili
2024-01-04  8:57       ` Jan Beulich
2023-12-19 12:12 ` [PATCH v4 2/9] Created an empty EVEX_MAP4_ sub-table for EVEX instructions Cui, Lili
2023-12-19 12:12 ` [PATCH v4 3/9] Support APX GPR32 with extend evex prefix Cui, Lili
2023-12-22 13:49   ` Jan Beulich
2023-12-25 12:23     ` Cui, Lili
2024-01-04  9:08       ` Jan Beulich
2024-01-04 12:32         ` Cui, Lili
2024-01-04 12:55           ` Jan Beulich
2023-12-22 14:19   ` Jan Beulich
2023-12-26  7:00     ` Cui, Lili
2024-01-04  9:01       ` Jan Beulich
2024-01-04 12:47         ` Cui, Lili
2023-12-19 12:12 ` [PATCH v4 4/9] Add tests for " Cui, Lili
2023-12-22 14:41   ` Jan Beulich [this message]
2023-12-25 13:40     ` Cui, Lili
2024-01-04  9:16       ` Jan Beulich
2024-01-05  6:58         ` Cui, Lili
2023-12-19 12:12 ` [PATCH v4 5/9] Support APX NDD Cui, Lili
2023-12-19 12:12 ` [PATCH v4 6/9] Support APX Push2/Pop2 Cui, Lili
2023-12-19 12:12 ` [PATCH v4 7/9] Support APX PUSHP/POPP Cui, Lili
2023-12-19 12:12 ` [PATCH v4 `8/9] Support APX NDD optimized encoding Cui, Lili
2023-12-19 12:12 ` [PATCH v4 9/9] Support APX JMPABS for disassembler Cui, Lili
2023-12-19 12:35 ` [PATCH v4 0/9] Support Intel APX EGPR Jan Beulich
2023-12-20  8:50   ` Cui, Lili
2023-12-20  8:57     ` Jan Beulich
2023-12-20 10:42       ` Cui, Lili
2023-12-20 11:00         ` Jan Beulich
2023-12-20 11:50           ` Cui, Lili
2023-12-20 12:01             ` Jan Beulich
2023-12-20 12:16               ` Cui, Lili

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e176f762-bfc0-49c5-adc2-fd446f172a1c@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    --cc=hongjiu.lu@intel.com \
    --cc=lili.cui@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).