public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Carlotti <andrew.carlotti@arm.com>
To: Srinath Parvathaneni <srinath.parvathaneni@arm.com>
Cc: Binutils <binutils@sourceware.org>,
	Nick Clifton <nickc@redhat.com>,
	Richard Earnshaw <richard.earnshaw@arm.com>,
	Jan Beulich <jbeulich@suse.com>
Subject: Re: [PATCH v1 1/2][Biuntils] aarch64: Fix sve2p1 dupq/extq instructions encoding and syntax.
Date: Tue, 27 Feb 2024 19:08:27 +0000	[thread overview]
Message-ID: <a59223fd-d0db-79a1-5544-5140b347c697@e124511.cambridge.arm.com> (raw)
In-Reply-To: <15bd8211-0e3c-4aed-b316-5749d32f6fb7@arm.com>

On Fri, Feb 23, 2024 at 01:57:55PM +0000, Srinath Parvathaneni wrote:
> Hi,
> 
> This patch enables "FEAT_SVE2p1" by default for Armv9.4-A and fixes the
> encoding
> and syntax for sve2p1 "dupq" and "extq" instructions which were reported
> here.
> https://sourceware.org/pipermail/binutils/2024-February/132408.html
> 
> Regression testing for aarch64-none-elf target and found no regressions.
> 
> Ok for binutils-master? Also ok to be backported to binutils-2.42 branch?
> 
> Regards,
> Srinath.

I have a few comments, some of which would have been more relevant against the
original patch series and don't need to be addressed now.  I also think this
would have been easier to read if the dupq fixes and the extq fixes were in
separate patches. 

> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
> index 0c6de289408f4c53633e468c610623c22a0fdec8..01e2bbf70c8b9812df8f1c6f8677bce2181701fa 100644
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -6895,6 +6895,7 @@ parse_operands (char *str, const aarch64_opcode *opcode)
>  	case AARCH64_OPND_SVE_UIMM7:
>  	case AARCH64_OPND_SVE_UIMM8:
>  	case AARCH64_OPND_SVE_UIMM8_53:
> +	case AARCH64_OPND_SVE_UIMM4:
>  	case AARCH64_OPND_IMM_ROT1:
>  	case AARCH64_OPND_IMM_ROT2:
>  	case AARCH64_OPND_IMM_ROT3:
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-1-bad.d b/gas/testsuite/gas/aarch64/sve2p1-1-bad.d
> index a2ca49ef487563a55ae8c26ca4318e68da850e64..c28cdc76c4c3c002528b9de3636be8bf1c991160 100644
> --- a/gas/testsuite/gas/aarch64/sve2p1-1-bad.d
> +++ b/gas/testsuite/gas/aarch64/sve2p1-1-bad.d
> @@ -1,4 +1,4 @@
>  #name: Illegal test of SVE2.1 min max instructions.
> -#as: -march=armv9.4-a
> +#as: -march=armv9.3-a
>  #source: sve2p1-1.s
>  #error_output: sve2p1-1-bad.l
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-1-bad.l b/gas/testsuite/gas/aarch64/sve2p1-1-bad.l
> index 50a4bacc73c20324ae50b8688dd8cf5123a238ae..83917440e8187252c01892ce32100213cfef998c 100644
> --- a/gas/testsuite/gas/aarch64/sve2p1-1-bad.l
> +++ b/gas/testsuite/gas/aarch64/sve2p1-1-bad.l
> @@ -35,26 +35,12 @@
>  .*: Error: selected processor does not support `uminqv v4.2d,p3,z2.d'
>  .*: Error: selected processor does not support `uminqv v8.2d,p4,z1.d'
>  .*: Error: selected processor does not support `uminqv v16.4s,p7,z0.s'
> -.*: Error: selected processor does not support `dupq z10.b,z20.b\[0\]'
> -.*: Error: selected processor does not support `dupq z10.b,z20.b\[15\]'
> -.*: Error: selected processor does not support `dupq z10.h,z20.h\[0\]'
> -.*: Error: selected processor does not support `dupq z10.h,z20.h\[7\]'
> -.*: Error: selected processor does not support `dupq z10.s,z20.s\[0\]'
> -.*: Error: selected processor does not support `dupq z10.s,z20.s\[3\]'
> -.*: Error: selected processor does not support `dupq z10.d,z20.d\[0\]'
> -.*: Error: selected processor does not support `dupq z10.d,z20.d\[1\]'
>  .*: Error: selected processor does not support `eorqv v0.16b,p0,z16.b'
>  .*: Error: selected processor does not support `eorqv v1.8h,p1,z8.h'
>  .*: Error: selected processor does not support `eorqv v2.4s,p2,z4.s'
>  .*: Error: selected processor does not support `eorqv v4.2d,p3,z2.d'
>  .*: Error: selected processor does not support `eorqv v8.2d,p4,z1.d'
>  .*: Error: selected processor does not support `eorqv v16.4s,p7,z0.s'
> -.*: Error: selected processor does not support `extq z0.b,z0.b,z10.b\[15\]'
> -.*: Error: selected processor does not support `extq z1.b,z1.b,z15.b\[7\]'
> -.*: Error: selected processor does not support `extq z2.b,z2.b,z5.b\[3\]'
> -.*: Error: selected processor does not support `extq z4.b,z4.b,z12.b\[1\]'
> -.*: Error: selected processor does not support `extq z8.b,z8.b,z7.b\[4\]'
> -.*: Error: selected processor does not support `extq z16.b,z16.b,z1.b\[8\]'
>  .*: Error: selected processor does not support `faddqv v1.8h,p1,z8.h'
>  .*: Error: selected processor does not support `faddqv v2.4s,p2,z4.s'
>  .*: Error: selected processor does not support `faddqv v4.2d,p3,z2.d'
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-1.d b/gas/testsuite/gas/aarch64/sve2p1-1.d
> index daece899b38bba4daa2ca9e58dba2d551f6cf988..ea8c5448a4181f7fc73bee8e46119cc796f2784c 100644
> --- a/gas/testsuite/gas/aarch64/sve2p1-1.d
> +++ b/gas/testsuite/gas/aarch64/sve2p1-1.d
> @@ -44,26 +44,12 @@
>  .*:	04cf2c44 	uminqv	v4.2d, p3, z2.d
>  .*:	04cf3028 	uminqv	v8.2d, p4, z1.d
>  .*:	048f3c10 	uminqv	v16.4s, p7, z0.s
> -.*:	0530268a 	dupq	z10.b, z20.b\[0\]
> -.*:	053f268a 	dupq	z10.b, z20.b\[15\]
> -.*:	0521268a 	dupq	z10.h, z20.h\[0\]
> -.*:	052f268a 	dupq	z10.h, z20.h\[7\]
> -.*:	0522268a 	dupq	z10.s, z20.s\[0\]
> -.*:	052e268a 	dupq	z10.s, z20.s\[3\]
> -.*:	0524268a 	dupq	z10.d, z20.d\[0\]
> -.*:	052c268a 	dupq	z10.d, z20.d\[1\]
>  .*:	041d2200 	eorqv	v0.16b, p0, z16.b
>  .*:	045d2501 	eorqv	v1.8h, p1, z8.h
>  .*:	049d2882 	eorqv	v2.4s, p2, z4.s
>  .*:	04dd2c44 	eorqv	v4.2d, p3, z2.d
>  .*:	04dd3028 	eorqv	v8.2d, p4, z1.d
>  .*:	049d3c10 	eorqv	v16.4s, p7, z0.s
> -.*:	056a27c0 	extq	z0.b, z0.b, z10.b\[15\]
> -.*:	056f25c1 	extq	z1.b, z1.b, z15.b\[7\]
> -.*:	056524c2 	extq	z2.b, z2.b, z5.b\[3\]
> -.*:	056c2444 	extq	z4.b, z4.b, z12.b\[1\]
> -.*:	05672508 	extq	z8.b, z8.b, z7.b\[4\]
> -.*:	05612610 	extq	z16.b, z16.b, z1.b\[8\]
>  .*:	6450a501 	faddqv	v1.8h, p1, z8.h
>  .*:	6490a882 	faddqv	v2.4s, p2, z4.s
>  .*:	64d0ac44 	faddqv	v4.2d, p3, z2.d
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-1.s b/gas/testsuite/gas/aarch64/sve2p1-1.s
> index 2a1c7c107d757ae922cec5566adbace1f03e0dce..12a086b078f259b4aeb840f95b1e40e37e379fe6 100644
> --- a/gas/testsuite/gas/aarch64/sve2p1-1.s
> +++ b/gas/testsuite/gas/aarch64/sve2p1-1.s
> @@ -39,15 +39,6 @@ uminqv v2.4s, p2, z4.s
>  uminqv v4.2d, p3, z2.d
>  uminqv v8.2d, p4, z1.d
>  uminqv v16.4s, p7, z0.s
> -dupq z10.b, z20.b[0]
> -dupq z10.b, z20.b[15]
> -dupq z10.h, z20.h[0]
> -dupq z10.h, z20.h[7]
> -dupq z10.s, z20.s[0]
> -dupq z10.s, z20.s[3]
> -dupq z10.d, z20.d[0]
> -dupq z10.d, z20.d[1]
> -
>  eorqv v0.16b, p0, z16.b
>  eorqv v1.8h, p1, z8.h
>  eorqv v2.4s, p2, z4.s
> @@ -55,12 +46,6 @@ eorqv v4.2d, p3, z2.d
>  eorqv v8.2d, p4, z1.d
>  eorqv v16.4s, p7, z0.s
>  
> -extq z0.b, z0.b, z10.b[15]
> -extq z1.b, z1.b, z15.b[7]
> -extq z2.b, z2.b, z5.b[3]
> -extq z4.b, z4.b, z12.b[1]
> -extq z8.b, z8.b, z7.b[4]
> -extq z16.b, z16.b, z1.b[8]
>  faddqv v1.8h, p1, z8.h
>  faddqv v2.4s, p2, z4.s
>  faddqv v4.2d, p3, z2.d
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-2-invalid.d b/gas/testsuite/gas/aarch64/sve2p1-2-invalid.d
> new file mode 100644
> index 0000000000000000000000000000000000000000..3953ca57ac838ac658561c63e8ca742752e1f157
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-2-invalid.d
> @@ -0,0 +1,3 @@
> +#name: Test of illegal SVE2.1 dupq instructions.
> +#as: -march=armv9.4-a
> +#error_output: sve2p1-2-invalid.l
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-2-invalid.l b/gas/testsuite/gas/aarch64/sve2p1-2-invalid.l
> new file mode 100644
> index 0000000000000000000000000000000000000000..e6ba2f737455835465fa69f6e67b145575fe00d9
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-2-invalid.l
> @@ -0,0 +1,26 @@
> +.*: Assembler messages:
> +.*: Error: register element index out of range 0 to 15 at operand 2 -- `dupq z0.b,z0.b\[16\]'
> +.*: Error: operand mismatch -- `dupq z0.h,z0.b\[16\]'
> +.*: Info:    did you mean this\?
> +.*: Info:    	dupq z0.b, z0.b\[16\]
> +.*: Info:    other valid variant\(s\):
> +.*: Info:    	dupq z0.h, z0.h\[16\]
> +.*: Info:    	dupq z0.s, z0.s\[16\]
> +.*: Info:    	dupq z0.d, z0.d\[16\]
> +.*: Error: operand mismatch -- `dupq z0.h,z0.s\[16\]'
> +.*: Info:    did you mean this\?
> +.*: Info:    	dupq z0.h, z0.h\[16\]
> +.*: Info:    other valid variant\(s\):
> +.*: Info:    	dupq z0.b, z0.b\[16\]
> +.*: Info:    	dupq z0.s, z0.s\[16\]
> +.*: Info:    	dupq z0.d, z0.d\[16\]
> +.*: Error: operand mismatch -- `dupq z0.s,z0.d\[16\]'
> +.*: Info:    did you mean this\?
> +.*: Info:    	dupq z0.s, z0.s\[16\]
> +.*: Info:    other valid variant\(s\):
> +.*: Info:    	dupq z0.b, z0.b\[16\]
> +.*: Info:    	dupq z0.h, z0.h\[16\]
> +.*: Info:    	dupq z0.d, z0.d\[16\]
> +.*: Error: register element index out of range 0 to 7 at operand 2 -- `dupq z0.h,z0.h\[8\]'
> +.*: Error: register element index out of range 0 to 3 at operand 2 -- `dupq z0.s,z0.s\[4\]'
> +.*: Error: register element index out of range 0 to 1 at operand 2 -- `dupq z0.d,z0.d\[2\]'
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-2-invalid.s b/gas/testsuite/gas/aarch64/sve2p1-2-invalid.s
> new file mode 100644
> index 0000000000000000000000000000000000000000..a032f224b15873d6d533f23377ca33e639730874
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-2-invalid.s
> @@ -0,0 +1,7 @@
> +	dupq z0.b, z0.b[16]
> +	dupq z0.h, z0.b[16]
> +	dupq z0.h, z0.s[16]
> +	dupq z0.s, z0.d[16]
> +	dupq z0.h, z0.h[8]
> +	dupq z0.s, z0.s[4]
> +	dupq z0.d, z0.d[2]
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-2.d b/gas/testsuite/gas/aarch64/sve2p1-2.d
> new file mode 100644
> index 0000000000000000000000000000000000000000..228a884898c9e684a204ad8484db50815f2873bf
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-2.d
> @@ -0,0 +1,210 @@
> +#name: Test of SVE2.1 dupq instructions.
> +#as: -march=armv9.4-a
> +#objdump: -dr
> +
> +[^:]+:     file format .*
> +
> +
> +[^:]+:
> +
> +[^:]+:
> +.*:	05212400 	dupq	z0.b, z0.b\[0\]
> +.*:	05232400 	dupq	z0.b, z0.b\[1\]
> +.*:	05252400 	dupq	z0.b, z0.b\[2\]
> +.*:	05272400 	dupq	z0.b, z0.b\[3\]
> +.*:	05292400 	dupq	z0.b, z0.b\[4\]
> +.*:	052f2400 	dupq	z0.b, z0.b\[7\]
> +.*:	05312400 	dupq	z0.b, z0.b\[8\]
> +.*:	053f2400 	dupq	z0.b, z0.b\[15\]
> +.*:	05222400 	dupq	z0.h, z0.h\[0\]
> +.*:	05262400 	dupq	z0.h, z0.h\[1\]
> +.*:	052a2400 	dupq	z0.h, z0.h\[2\]
> +.*:	052e2400 	dupq	z0.h, z0.h\[3\]
> +.*:	05322400 	dupq	z0.h, z0.h\[4\]
> +.*:	053e2400 	dupq	z0.h, z0.h\[7\]
> +.*:	05242400 	dupq	z0.s, z0.s\[0\]
> +.*:	052c2400 	dupq	z0.s, z0.s\[1\]
> +.*:	05342400 	dupq	z0.s, z0.s\[2\]
> +.*:	053c2400 	dupq	z0.s, z0.s\[3\]
> +.*:	05282400 	dupq	z0.d, z0.d\[0\]
> +.*:	05382400 	dupq	z0.d, z0.d\[1\]
> +.*:	05212421 	dupq	z1.b, z1.b\[0\]
> +.*:	05232421 	dupq	z1.b, z1.b\[1\]
> +.*:	05252421 	dupq	z1.b, z1.b\[2\]
> +.*:	05272421 	dupq	z1.b, z1.b\[3\]
> +.*:	05292421 	dupq	z1.b, z1.b\[4\]
> +.*:	052f2421 	dupq	z1.b, z1.b\[7\]
> +.*:	05312421 	dupq	z1.b, z1.b\[8\]
> +.*:	053f2421 	dupq	z1.b, z1.b\[15\]
> +.*:	05222421 	dupq	z1.h, z1.h\[0\]
> +.*:	05262421 	dupq	z1.h, z1.h\[1\]
> +.*:	052a2421 	dupq	z1.h, z1.h\[2\]
> +.*:	052e2421 	dupq	z1.h, z1.h\[3\]
> +.*:	05322421 	dupq	z1.h, z1.h\[4\]
> +.*:	053e2421 	dupq	z1.h, z1.h\[7\]
> +.*:	05242421 	dupq	z1.s, z1.s\[0\]
> +.*:	052c2421 	dupq	z1.s, z1.s\[1\]
> +.*:	05342421 	dupq	z1.s, z1.s\[2\]
> +.*:	053c2421 	dupq	z1.s, z1.s\[3\]
> +.*:	05282421 	dupq	z1.d, z1.d\[0\]
> +.*:	05382421 	dupq	z1.d, z1.d\[1\]
> +.*:	05212442 	dupq	z2.b, z2.b\[0\]
> +.*:	05232442 	dupq	z2.b, z2.b\[1\]
> +.*:	05252442 	dupq	z2.b, z2.b\[2\]
> +.*:	05272442 	dupq	z2.b, z2.b\[3\]
> +.*:	05292442 	dupq	z2.b, z2.b\[4\]
> +.*:	052f2442 	dupq	z2.b, z2.b\[7\]
> +.*:	05312442 	dupq	z2.b, z2.b\[8\]
> +.*:	053f2442 	dupq	z2.b, z2.b\[15\]
> +.*:	05222442 	dupq	z2.h, z2.h\[0\]
> +.*:	05262442 	dupq	z2.h, z2.h\[1\]
> +.*:	052a2442 	dupq	z2.h, z2.h\[2\]
> +.*:	052e2442 	dupq	z2.h, z2.h\[3\]
> +.*:	05322442 	dupq	z2.h, z2.h\[4\]
> +.*:	053e2442 	dupq	z2.h, z2.h\[7\]
> +.*:	05242442 	dupq	z2.s, z2.s\[0\]
> +.*:	052c2442 	dupq	z2.s, z2.s\[1\]
> +.*:	05342442 	dupq	z2.s, z2.s\[2\]
> +.*:	053c2442 	dupq	z2.s, z2.s\[3\]
> +.*:	05282442 	dupq	z2.d, z2.d\[0\]
> +.*:	05382442 	dupq	z2.d, z2.d\[1\]
> +.*:	05212463 	dupq	z3.b, z3.b\[0\]
> +.*:	05232463 	dupq	z3.b, z3.b\[1\]
> +.*:	05252463 	dupq	z3.b, z3.b\[2\]
> +.*:	05272463 	dupq	z3.b, z3.b\[3\]
> +.*:	05292463 	dupq	z3.b, z3.b\[4\]
> +.*:	052f2463 	dupq	z3.b, z3.b\[7\]
> +.*:	05312463 	dupq	z3.b, z3.b\[8\]
> +.*:	053f2463 	dupq	z3.b, z3.b\[15\]
> +.*:	05222463 	dupq	z3.h, z3.h\[0\]
> +.*:	05262463 	dupq	z3.h, z3.h\[1\]
> +.*:	052a2463 	dupq	z3.h, z3.h\[2\]
> +.*:	052e2463 	dupq	z3.h, z3.h\[3\]
> +.*:	05322463 	dupq	z3.h, z3.h\[4\]
> +.*:	053e2463 	dupq	z3.h, z3.h\[7\]
> +.*:	05242463 	dupq	z3.s, z3.s\[0\]
> +.*:	052c2463 	dupq	z3.s, z3.s\[1\]
> +.*:	05342463 	dupq	z3.s, z3.s\[2\]
> +.*:	053c2463 	dupq	z3.s, z3.s\[3\]
> +.*:	05282463 	dupq	z3.d, z3.d\[0\]
> +.*:	05382463 	dupq	z3.d, z3.d\[1\]
> +.*:	05212484 	dupq	z4.b, z4.b\[0\]
> +.*:	05232484 	dupq	z4.b, z4.b\[1\]
> +.*:	05252484 	dupq	z4.b, z4.b\[2\]
> +.*:	05272484 	dupq	z4.b, z4.b\[3\]
> +.*:	05292484 	dupq	z4.b, z4.b\[4\]
> +.*:	052f2484 	dupq	z4.b, z4.b\[7\]
> +.*:	05312484 	dupq	z4.b, z4.b\[8\]
> +.*:	053f2484 	dupq	z4.b, z4.b\[15\]
> +.*:	05222484 	dupq	z4.h, z4.h\[0\]
> +.*:	05262484 	dupq	z4.h, z4.h\[1\]
> +.*:	052a2484 	dupq	z4.h, z4.h\[2\]
> +.*:	052e2484 	dupq	z4.h, z4.h\[3\]
> +.*:	05322484 	dupq	z4.h, z4.h\[4\]
> +.*:	053e2484 	dupq	z4.h, z4.h\[7\]
> +.*:	05242484 	dupq	z4.s, z4.s\[0\]
> +.*:	052c2484 	dupq	z4.s, z4.s\[1\]
> +.*:	05342484 	dupq	z4.s, z4.s\[2\]
> +.*:	053c2484 	dupq	z4.s, z4.s\[3\]
> +.*:	05282484 	dupq	z4.d, z4.d\[0\]
> +.*:	05382484 	dupq	z4.d, z4.d\[1\]
> +.*:	052124e7 	dupq	z7.b, z7.b\[0\]
> +.*:	052324e7 	dupq	z7.b, z7.b\[1\]
> +.*:	052524e7 	dupq	z7.b, z7.b\[2\]
> +.*:	052724e7 	dupq	z7.b, z7.b\[3\]
> +.*:	052924e7 	dupq	z7.b, z7.b\[4\]
> +.*:	052f24e7 	dupq	z7.b, z7.b\[7\]
> +.*:	053124e7 	dupq	z7.b, z7.b\[8\]
> +.*:	053f24e7 	dupq	z7.b, z7.b\[15\]
> +.*:	052224e7 	dupq	z7.h, z7.h\[0\]
> +.*:	052624e7 	dupq	z7.h, z7.h\[1\]
> +.*:	052a24e7 	dupq	z7.h, z7.h\[2\]
> +.*:	052e24e7 	dupq	z7.h, z7.h\[3\]
> +.*:	053224e7 	dupq	z7.h, z7.h\[4\]
> +.*:	053e24e7 	dupq	z7.h, z7.h\[7\]
> +.*:	052424e7 	dupq	z7.s, z7.s\[0\]
> +.*:	052c24e7 	dupq	z7.s, z7.s\[1\]
> +.*:	053424e7 	dupq	z7.s, z7.s\[2\]
> +.*:	053c24e7 	dupq	z7.s, z7.s\[3\]
> +.*:	052824e7 	dupq	z7.d, z7.d\[0\]
> +.*:	053824e7 	dupq	z7.d, z7.d\[1\]
> +.*:	05212508 	dupq	z8.b, z8.b\[0\]
> +.*:	05232508 	dupq	z8.b, z8.b\[1\]
> +.*:	05252508 	dupq	z8.b, z8.b\[2\]
> +.*:	05272508 	dupq	z8.b, z8.b\[3\]
> +.*:	05292508 	dupq	z8.b, z8.b\[4\]
> +.*:	052f2508 	dupq	z8.b, z8.b\[7\]
> +.*:	05312508 	dupq	z8.b, z8.b\[8\]
> +.*:	053f2508 	dupq	z8.b, z8.b\[15\]
> +.*:	05222508 	dupq	z8.h, z8.h\[0\]
> +.*:	05262508 	dupq	z8.h, z8.h\[1\]
> +.*:	052a2508 	dupq	z8.h, z8.h\[2\]
> +.*:	052e2508 	dupq	z8.h, z8.h\[3\]
> +.*:	05322508 	dupq	z8.h, z8.h\[4\]
> +.*:	053e2508 	dupq	z8.h, z8.h\[7\]
> +.*:	05242508 	dupq	z8.s, z8.s\[0\]
> +.*:	052c2508 	dupq	z8.s, z8.s\[1\]
> +.*:	05342508 	dupq	z8.s, z8.s\[2\]
> +.*:	053c2508 	dupq	z8.s, z8.s\[3\]
> +.*:	05282508 	dupq	z8.d, z8.d\[0\]
> +.*:	05382508 	dupq	z8.d, z8.d\[1\]
> +.*:	052125ef 	dupq	z15.b, z15.b\[0\]
> +.*:	052325ef 	dupq	z15.b, z15.b\[1\]
> +.*:	052525ef 	dupq	z15.b, z15.b\[2\]
> +.*:	052725ef 	dupq	z15.b, z15.b\[3\]
> +.*:	052925ef 	dupq	z15.b, z15.b\[4\]
> +.*:	052f25ef 	dupq	z15.b, z15.b\[7\]
> +.*:	053125ef 	dupq	z15.b, z15.b\[8\]
> +.*:	053f25ef 	dupq	z15.b, z15.b\[15\]
> +.*:	052225ef 	dupq	z15.h, z15.h\[0\]
> +.*:	052625ef 	dupq	z15.h, z15.h\[1\]
> +.*:	052a25ef 	dupq	z15.h, z15.h\[2\]
> +.*:	052e25ef 	dupq	z15.h, z15.h\[3\]
> +.*:	053225ef 	dupq	z15.h, z15.h\[4\]
> +.*:	053e25ef 	dupq	z15.h, z15.h\[7\]
> +.*:	052425ef 	dupq	z15.s, z15.s\[0\]
> +.*:	052c25ef 	dupq	z15.s, z15.s\[1\]
> +.*:	053425ef 	dupq	z15.s, z15.s\[2\]
> +.*:	053c25ef 	dupq	z15.s, z15.s\[3\]
> +.*:	052825ef 	dupq	z15.d, z15.d\[0\]
> +.*:	053825ef 	dupq	z15.d, z15.d\[1\]
> +.*:	05212610 	dupq	z16.b, z16.b\[0\]
> +.*:	05232610 	dupq	z16.b, z16.b\[1\]
> +.*:	05252610 	dupq	z16.b, z16.b\[2\]
> +.*:	05272610 	dupq	z16.b, z16.b\[3\]
> +.*:	05292610 	dupq	z16.b, z16.b\[4\]
> +.*:	052f2610 	dupq	z16.b, z16.b\[7\]
> +.*:	05312610 	dupq	z16.b, z16.b\[8\]
> +.*:	053f2610 	dupq	z16.b, z16.b\[15\]
> +.*:	05222610 	dupq	z16.h, z16.h\[0\]
> +.*:	05262610 	dupq	z16.h, z16.h\[1\]
> +.*:	052a2610 	dupq	z16.h, z16.h\[2\]
> +.*:	052e2610 	dupq	z16.h, z16.h\[3\]
> +.*:	05322610 	dupq	z16.h, z16.h\[4\]
> +.*:	053e2610 	dupq	z16.h, z16.h\[7\]
> +.*:	05242610 	dupq	z16.s, z16.s\[0\]
> +.*:	052c2610 	dupq	z16.s, z16.s\[1\]
> +.*:	05342610 	dupq	z16.s, z16.s\[2\]
> +.*:	053c2610 	dupq	z16.s, z16.s\[3\]
> +.*:	05282610 	dupq	z16.d, z16.d\[0\]
> +.*:	05382610 	dupq	z16.d, z16.d\[1\]
> +.*:	052127ff 	dupq	z31.b, z31.b\[0\]
> +.*:	052327ff 	dupq	z31.b, z31.b\[1\]
> +.*:	052527ff 	dupq	z31.b, z31.b\[2\]
> +.*:	052727ff 	dupq	z31.b, z31.b\[3\]
> +.*:	052927ff 	dupq	z31.b, z31.b\[4\]
> +.*:	052f27ff 	dupq	z31.b, z31.b\[7\]
> +.*:	053127ff 	dupq	z31.b, z31.b\[8\]
> +.*:	053f27ff 	dupq	z31.b, z31.b\[15\]
> +.*:	052227ff 	dupq	z31.h, z31.h\[0\]
> +.*:	052627ff 	dupq	z31.h, z31.h\[1\]
> +.*:	052a27ff 	dupq	z31.h, z31.h\[2\]
> +.*:	052e27ff 	dupq	z31.h, z31.h\[3\]
> +.*:	053227ff 	dupq	z31.h, z31.h\[4\]
> +.*:	053e27ff 	dupq	z31.h, z31.h\[7\]
> +.*:	052427ff 	dupq	z31.s, z31.s\[0\]
> +.*:	052c27ff 	dupq	z31.s, z31.s\[1\]
> +.*:	053427ff 	dupq	z31.s, z31.s\[2\]
> +.*:	053c27ff 	dupq	z31.s, z31.s\[3\]
> +.*:	052827ff 	dupq	z31.d, z31.d\[0\]
> +.*:	053827ff 	dupq	z31.d, z31.d\[1\]
This looks like too many tests to me.  Did you check them all individually?
Are are they just derived from compiler output, with little subsequent
checking?

> diff --git a/gas/testsuite/gas/aarch64/sve2p1-2.s b/gas/testsuite/gas/aarch64/sve2p1-2.s
> new file mode 100644
> index 0000000000000000000000000000000000000000..a9b4ae367f214ec8740274cd8832777935ca44c2
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-2.s
> @@ -0,0 +1,28 @@
> +	.text
> +	.irp op1 0,1,2,3,4,7,8,15,16,31
> +
> +	.irp op2 0,1,2,3,4,7,8,15
> +	.irp op3 .b
> +		dupq z\op1\op3, z\op1\op3[\op2]
> +	.endr
> +	.endr
> +
> +	.irp op2 0,1,2,3,4,7
> +	.irp op3 .h
> +		dupq z\op1\op3, z\op1\op3[\op2]
> +	.endr
> +	.endr
> +
> +	.irp op2 0,1,2,3
> +	.irp op3 .s
> +		dupq z\op1\op3, z\op1\op3[\op2]
> +	.endr
> +	.endr
> +
> +	.irp op2 0,1
> +	.irp op3 .d
> +		dupq z\op1\op3, z\op1\op3[\op2]
> +	.endr
> +	.endr
> +
> +	.endr
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-3-invalid.d b/gas/testsuite/gas/aarch64/sve2p1-3-invalid.d
> new file mode 100644
> index 0000000000000000000000000000000000000000..ff6ecb2027a486494ddd8e96d79c86caf76ddba9
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-3-invalid.d
> @@ -0,0 +1,3 @@
> +#name: Test of illegal SVE2.1 extq instructions.
> +#as: -march=armv9.4-a
> +#error_output: sve2p1-3-invalid.l
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-3-invalid.l b/gas/testsuite/gas/aarch64/sve2p1-3-invalid.l
> new file mode 100644
> index 0000000000000000000000000000000000000000..95a731c934a426ac4dd5dd7735c8cae3ab211176
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-3-invalid.l
> @@ -0,0 +1,12 @@
> +.*: Assembler messages:
> +.*: Error: operand mismatch -- `extq z0.b,z0.h,z0.b,#0'
> +.*: Info:    did you mean this\?
> +.*: Info:    	extq z0.b, z0.b, z0.b, #0
> +.*: Error: operand 2 must be the same register as operand 1 -- `extq z31.b,z15.b,z0.b,#0'
> +.*: Error: operand mismatch -- `extq z0.b,z0.b,z31.h,#0'
> +.*: Info:    did you mean this\?
> +.*: Info:    	extq z0.b, z0.b, z31.b, #0
> +.*: Error: immediate value out of range 0 to 15 at operand 4 -- `extq z0.b,z0.b,z0.b,#16'
> +.*: Error: operand mismatch -- `extq z0.h,z0.h,z0.h,#15'
> +.*: Info:    did you mean this\?
> +.*: Info:    	extq z0.b, z0.b, z0.b, #15
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-3-invalid.s b/gas/testsuite/gas/aarch64/sve2p1-3-invalid.s
> new file mode 100644
> index 0000000000000000000000000000000000000000..3abfb87af68942497da33de38f9a9ece291e2499
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-3-invalid.s
> @@ -0,0 +1,5 @@
> +extq z0.b, z0.h, z0.b, #0
> +extq z31.b, z15.b, z0.b, #0
> +extq z0.b, z0.b, z31.h, #0
> +extq z0.b, z0.b, z0.b, #16
> +extq z0.h, z0.h, z0.h, #15
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-3.d b/gas/testsuite/gas/aarch64/sve2p1-3.d
> new file mode 100644
> index 0000000000000000000000000000000000000000..6e7ee001f12a9b21a7f836e7379ce5ea4bb32e88
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-3.d
> @@ -0,0 +1,16 @@
> +#name: Test of SVE2.1 extq instructions.
> +#as: -march=armv9.4-a
> +#objdump: -dr
> +
> +[^:]+:     file format .*
> +
> +
> +[^:]+:
> +
> +[^:]+:
> +.*:	05602400 	extq	z0.b, z0.b, z0.b, #0
> +.*:	0560241f 	extq	z31.b, z31.b, z0.b, #0
> +.*:	056027e0 	extq	z0.b, z0.b, z31.b, #0
> +.*:	056f2400 	extq	z0.b, z0.b, z0.b, #15
> +.*:	056f27ff 	extq	z31.b, z31.b, z31.b, #15
> +.*:	056727ef 	extq	z15.b, z15.b, z31.b, #7
> diff --git a/gas/testsuite/gas/aarch64/sve2p1-3.s b/gas/testsuite/gas/aarch64/sve2p1-3.s
> new file mode 100644
> index 0000000000000000000000000000000000000000..09d30eaa43d3b807b452287f7684ff83df51bfb0
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sve2p1-3.s
> @@ -0,0 +1,6 @@
> +extq z0.b, z0.b, z0.b, #0
> +extq z31.b, z31.b, z0.b, #0
> +extq z0.b, z0.b, z31.b, #0
> +extq z0.b, z0.b, z0.b, #15
> +extq z31.b, z31.b, z31.b, #15
> +extq z15.b, z15.b, z31.b, #7
> diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
> index 02ee0fc2566d500359a9e89de3dfb954100f63ce..e70b9e6d348b81b6331e077f3bdf966663917e16 100644
> --- a/include/opcode/aarch64.h
> +++ b/include/opcode/aarch64.h
> @@ -307,6 +307,7 @@ enum aarch64_feature_bit {
>  					 | AARCH64_FEATBIT (X, PMUv3_ICNTR) \
>  					 | AARCH64_FEATBIT (X, SEBEP) \
>  					 | AARCH64_FEATBIT (X, PREDRES2) \
> +					 | AARCH64_FEATBIT (X, SVE2p1)	\

I agree with Jan that this belongs in a separate commit.

>  					)
>  
>  #define AARCH64_ARCH_V9A_FEATURES(X)	(AARCH64_FEATBIT (X, V9A)	\
> @@ -704,6 +705,7 @@ enum aarch64_opnd
>    AARCH64_OPND_SVE_UIMM7,	/* SVE unsigned 7-bit immediate.  */
>    AARCH64_OPND_SVE_UIMM8,	/* SVE unsigned 8-bit immediate.  */
>    AARCH64_OPND_SVE_UIMM8_53,	/* SVE split unsigned 8-bit immediate.  */
> +  AARCH64_OPND_SVE_UIMM4,	/* SVE unsigned 4-bit immediate.  */
>    AARCH64_OPND_SVE_VZn,		/* Scalar SIMD&FP register in Zn field.  */
>    AARCH64_OPND_SVE_Vd,		/* Scalar SIMD&FP register in Vd.  */
>    AARCH64_OPND_SVE_Vm,		/* Scalar SIMD&FP register in Vm.  */
> diff --git a/opcodes/aarch64-asm.c b/opcodes/aarch64-asm.c
> index 29e96e244bb2d01ec5514f5d66acc2ff0c724503..70eda9573795f9a35fa1810ca936d7870e47acbe 100644
> --- a/opcodes/aarch64-asm.c
> +++ b/opcodes/aarch64-asm.c
> @@ -1271,7 +1271,7 @@ aarch64_ins_sve_index_imm (const aarch64_operand *self,
>    insert_field (self->fields[0], code, info->reglane.regno, 0);
>    unsigned int esize = aarch64_get_qualifier_esize (info->qualifier);
>    insert_fields (code, (info->reglane.index * 2 + 1) * esize, 0,
> -		 2, self->fields[1],self->fields[2]);
> +		 2, self->fields[2],self->fields[1]);
You could use insert_all_fields_after here - i.e:
insert_all_fields_after (self, 1, code, (info->reglane.index * 2 + 1) * esize);

>    return true;
>  }
>  
> diff --git a/opcodes/aarch64-dis.c b/opcodes/aarch64-dis.c
> index 82d2f8f8251f0f48e06a85360ebfdf258247de20..33cd2731c9dc3e88e7ffeca918e37271dddcd3a5 100644
> --- a/opcodes/aarch64-dis.c
> +++ b/opcodes/aarch64-dis.c
> @@ -2173,7 +2173,18 @@ aarch64_ext_sve_index_imm (const aarch64_operand *self,
>    int val;
>  
>    info->reglane.regno = extract_field (self->fields[0], code, 0);
> -  val = extract_fields (code, 0, 2, self->fields[2], self->fields[1]);
> +  val = extract_fields (code, 0, 2, self->fields[1], self->fields[2]);
You could use extract_all_fields_after here - i.e:
val = extract_all_fields_after (self, 1, code);

> +
> +  /* val ranges from 1 to 31 decoded in bits[20:16] for the index immediate.
> +    (val & 01) == 0 is reserved and return false.
> +    (val & 01) == 1 is suffix .b for operand and immediate ranges from
> +		    0-15. info->reglane.index = value in bits[20:17].
> +    (val & 02) == 1 is suffix .h for operand and immediate ranges from
> +		    0-7. info->reglane.index = value in bits[20:18].
> +    (val & 04) == 1 is suffix .s for operand and immediate ranges from
> +		    0-3. info->reglane.index = value in bits[20:19].
> +    (val & 08) == 1 is suffix .d for operand and immediate ranges from
> +		    0-1. info->reglane.index = value in bit[20].  */
The bitmask tests in this comment are wrong.

>    if ((val & 15) == 0)
>      return 0;
>    while ((val & 1) == 0)
> @@ -3352,7 +3363,7 @@ aarch64_decode_variant_using_iclass (aarch64_inst *inst)
>        break;
>  
>      case sve_index1:
> -      i = extract_fields (inst->value, 0, 2, FLD_SVE_tsz, FLD_SVE_i2h);
> +      i = extract_fields (inst->value, 0, 2, FLD_SVE_i2h, FLD_SVE_tsz);
>        if ((i & 15) == 0)
>  	return false;
>        while ((i & 1) == 0)

I think the sve_index1 case here could be combined with the sve_index case, and
similarly sve_index_imm operand insert/extract functions could be merged with
the sve_index insert/extract functions.  However, that's not an issue with this
patch, but just an idea for future simplification.

> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
> index 965c1c0698c5ab97abde32cd502ae342ad9e34a1..00344604e5e8b88b17daafc60fbd395fb50c7006 100644
> --- a/opcodes/aarch64-opc.c
> +++ b/opcodes/aarch64-opc.c
> @@ -2697,10 +2697,14 @@ operand_general_constraint_met_p (const aarch64_opnd_info *opnds, int idx,
>  	case AARCH64_OPND_SVE_UIMM3:
>  	case AARCH64_OPND_SVE_UIMM7:
>  	case AARCH64_OPND_SVE_UIMM8:
> +	case AARCH64_OPND_SVE_UIMM4:
>  	case AARCH64_OPND_SVE_UIMM8_53:
>  	case AARCH64_OPND_CSSC_UIMM8:
>  	  size = get_operand_fields_width (get_operand_from_code (type));
> -	  assert (size < 32);
> +	  if (type == AARCH64_OPND_SVE_UIMM4)
> +	     assert (size < 16);
> +	  else
> +	     assert (size < 32);
This change is pointless; the assert is there to protect against overflow in
the subsequent code in case the operand fields were mistakenly specified as too
large for a 32-bit integer.  In any case, SIZE is 4 in the case you added.

>  	  if (!value_fit_unsigned_field_p (opnd->imm.value, size))
>  	    {
>  	      set_imm_out_of_range_error (mismatch_detail, idx, 0,
> @@ -4326,6 +4330,7 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
>      case AARCH64_OPND_SVE_UIMM3:
>      case AARCH64_OPND_SVE_UIMM7:
>      case AARCH64_OPND_SVE_UIMM8:
> +    case AARCH64_OPND_SVE_UIMM4:
>      case AARCH64_OPND_SVE_UIMM8_53:
>      case AARCH64_OPND_IMM_ROT1:
>      case AARCH64_OPND_IMM_ROT2:
> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
> index 18ac80251e9a15fc16f7070abd3be2d2e4dceb5a..21794022721a8e40d12dde1e383e3a40f42624e5 100644
> --- a/opcodes/aarch64-tbl.h
> +++ b/opcodes/aarch64-tbl.h
> @@ -6375,7 +6375,7 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>    SVE2p1_INSNC("fminqv",0x6417a000, 0xff3fe000, sve2_urqvs, 0, OP3 (Vd, SVE_Pg3, SVE_Zn), OP_SVE_vUS_HSD_HSD, F_OPD_SIZE, C_SCAN_MOVPRFX, 0),
>  
>    SVE2p1_INSN("dupq",0x05202400, 0xffe0fc00, sve_index1, 0, OP2 (SVE_Zd, SVE_Zn_5_INDEX), OP_SVE_VV_BHSD, 0, 0),
> -  SVE2p1_INSN("extq",0x05602400, 0xfff0fc00, sve_misc, 0, OP3 (SVE_Zd, SVE_Zd, SVE_Zm_imm4), OP_SVE_BBB, 0, 0),
> +  SVE2p1_INSNC("extq",0x05602400, 0xfff0fc00, sve_misc, 0, OP4 (SVE_Zd, SVE_Zd, SVE_Zm_5, SVE_UIMM4), OP_SVE_BBBU, 0, C_SCAN_MOVPRFX, 1),
>    SVE2p1_INSNC("ld1q",0xc400a000, 0xffe0e000, sve_misc, 0, OP3 (SVE_Zt, SVE_Pg3, SVE_ADDR_ZX), OP_SVE_SZS_QD, 0, C_SCAN_MOVPRFX, 0),
>    SVE2p1_INSNC("ld2q",0xa490e000, 0xfff0e000, sve_misc, 0, OP3 (SME_Zt2, SVE_Pg3, SVE_ADDR_RI_S4x2xVL), OP_SVE_QZU, 0, C_SCAN_MOVPRFX, 0),
>    SVE2p1_INSNC("ld3q",0xa510e000, 0xfff0e000, sve_misc, 0, OP3 (SME_Zt3, SVE_Pg3, SVE_ADDR_RI_S4x2xVL), OP_SVE_QZU, 0, C_SCAN_MOVPRFX, 0),
> @@ -6825,6 +6825,8 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>        "an 8-bit unsigned immediate")					\
>      Y(IMMEDIATE, imm, "SVE_UIMM8_53", 0, F(FLD_imm5,FLD_imm3_10),		\
>        "an 8-bit unsigned immediate")					\
> +    Y(IMMEDIATE, imm, "SVE_UIMM4", 0, F(FLD_SVE_imm4),			\
> +      "an 4-bit unsigned immediate")					\
>      Y(SIMD_REG, regno, "SVE_VZn", 0, F(FLD_SVE_Zn), "a SIMD register")	\
>      Y(SIMD_REG, regno, "SVE_Vd", 0, F(FLD_SVE_Vd), "a SIMD register")	\
>      Y(SIMD_REG, regno, "SVE_Vm", 0, F(FLD_SVE_Vm), "a SIMD register")	\


      parent reply	other threads:[~2024-02-27 19:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-23 13:57 Srinath Parvathaneni
2024-02-23 13:59 ` [PATCH v1 2/2][Binutils] aarch64: Fix sve2p1 ld[1-4]q/st[1-4]q " Srinath Parvathaneni
2024-02-26 10:10   ` Jan Beulich
2024-02-26  9:57 ` [PATCH v1 1/2][Biuntils] aarch64: Fix sve2p1 dupq/extq " Jan Beulich
2024-02-27 19:08 ` Andrew Carlotti [this message]

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=a59223fd-d0db-79a1-5544-5140b347c697@e124511.cambridge.arm.com \
    --to=andrew.carlotti@arm.com \
    --cc=binutils@sourceware.org \
    --cc=jbeulich@suse.com \
    --cc=nickc@redhat.com \
    --cc=richard.earnshaw@arm.com \
    --cc=srinath.parvathaneni@arm.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).