[AARCH64] Fix error checking for SIMD udot (by element)

Message ID VI1PR0801MB2014E1720C036970BF196097E0EA0@VI1PR0801MB2014.eurprd08.prod.outlook.com
State New
Headers show
Series
  • [AARCH64] Fix error checking for SIMD udot (by element)
Related show

Commit Message

Matthew Malcomson Oct. 4, 2018, 9:15 a.m.
[PATCH][BINUTILS][AARCH64] Fix error checking for SIMD udot (by element)

The SIMD UDOT instruction assembly has an unusual operand that selects a single
32 bit element with the mnemonic 4B.
This unusual mnemonic is handled by a special operand qualifier and associated
qualifier data in `aarch64_opnd_qualifiers`.

The current qualifier data describes 4 1-byte elements with the structure
{1, 4, 0x0, "4b", OQK_OPD_VARIANT}
This makes sense, as the instruction does work on 4 1-byte elements, however
some logic in the `operand_general_constraint_met_p` makes assumptions about
the range of index allowed when selecting a SIMD_ELEMENT depending on element
size.
That function reasons that e.g. in order to select a byte-sized element in a 16
byte V register an index must allow selection of one of the 16 elements and
hence its range will be in [0,15].

This reasoning breaks with the above description of a 4 part selection of 1
byte elements and allows an index outside the valid [0,3] range, triggering an
assert later on in the program in `aarch64_ins_reglane`.

vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new -march=armv8.4-a
as-new: ../../binutils-gdb/opcodes/aarch64-asm.c:134: aarch64_ins_reglane: Assertion `reglane_index < 4' failed.
{standard input}: Assembler messages:
{standard input}:1: Internal error (Aborted).
Please report this bug.


This patch changes the operand qualifier data so that it describes a single
32 bit element.
{4, 1, 0x0, "4b", OQK_OPD_VARIANT}
Hence the calculation in `operand_general_constraint_met_p` provides the
correct answer and the usual error checking machinery is used.

vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new -march=armv8.4-a
{standard input}: Assembler messages:
{standard input}:1: Error: register element index out of range 0 to 3 at operand 3 -- `udot v0.2s,v1.8b,v2.4b[4]'

Ran tests on aarch64-none-linux-gnu.
Ok for trunk?

gas/ChangeLog:

2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

	* testsuite/gas/aarch64/illegal-dotproduct.d: New test.
	* testsuite/gas/aarch64/illegal-dotproduct.l: New test.
	* testsuite/gas/aarch64/illegal-dotproduct.s: New test.

opcodes/ChangeLog:

2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

	* aarch64-opc.c (struct operand_qualifier_data): Change qualifier data
	corresponding to AARCH64_OPND_QLF_S_4B qualifier.


###############     Attachment also inlined for ease of reply    ###############
diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.d b/gas/testsuite/gas/aarch64/illegal-dotproduct.d
new file mode 100644
index 0000000000000000000000000000000000000000..3f8928da83bb64891528cf5860236f4b73c184b8
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.d
@@ -0,0 +1,4 @@
+#as: -march=armv8.2-a+dotprod
+#name: Invalid dotproduct instructions.
+#source: illegal-dotproduct.s
+#error_output: illegal-dotproduct.l
diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.l b/gas/testsuite/gas/aarch64/illegal-dotproduct.l
new file mode 100644
index 0000000000000000000000000000000000000000..06d0d78b8dc91d7d140415aa5a4a67d4b20edefd
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.l
@@ -0,0 +1,13 @@
+[^:]+: Assembler messages:
+[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 -- `udot V0.2S,V0.8B,V0.4B\[4\]'
+[^:]+:[0-9]+: Error: operand mismatch -- `udot V0.4S,V0.8B,V0.4B\[4\]'
+[^:]+:[0-9]+: Info:    did you mean this\?
+[^:]+:[0-9]+: Info:    	udot v0.2s, v0.8b, v0.4b\[4\]
+[^:]+:[0-9]+: Info:    other valid variant\(s\):
+[^:]+:[0-9]+: Info:    	udot v0.4s, v0.16b, v0.4b\[4\]
+[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 -- `sdot V0.2S,V0.8B,V0.4B\[4\]'
+[^:]+:[0-9]+: Error: operand mismatch -- `sdot V0.2S,V0.8B,V0.4H\[4\]'
+[^:]+:[0-9]+: Info:    did you mean this\?
+[^:]+:[0-9]+: Info:    	sdot v0.2s, v0.8b, v0.4b\[4\]
+[^:]+:[0-9]+: Info:    other valid variant\(s\):
+[^:]+:[0-9]+: Info:    	sdot v0.4s, v0.16b, v0.4b\[4\]
diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.s b/gas/testsuite/gas/aarch64/illegal-dotproduct.s
new file mode 100644
index 0000000000000000000000000000000000000000..9c714ae54d8e0b45dea8d7cb6455801d53a00ddc
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.s
@@ -0,0 +1,4 @@
+UDOT	V0.2S, V0.8B, V0.4B[4]
+UDOT	V0.4S, V0.8B, V0.4B[4]
+SDOT	V0.2S, V0.8B, V0.4B[4]
+SDOT	V0.2S, V0.8B, V0.4H[4]
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index ba2af7bfc26d2f760f19cdbdd07ebe5535308d72..e59240c98db3ea8472b992a423b53db6340033ea 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -698,7 +698,7 @@ struct operand_qualifier_data aarch64_opnd_qualifiers[] =
   {4, 1, 0x2, "s", OQK_OPD_VARIANT},
   {8, 1, 0x3, "d", OQK_OPD_VARIANT},
   {16, 1, 0x4, "q", OQK_OPD_VARIANT},
-  {1, 4, 0x0, "4b", OQK_OPD_VARIANT},
+  {4, 1, 0x0, "4b", OQK_OPD_VARIANT},
 
   {1, 4, 0x0, "4b", OQK_OPD_VARIANT},
   {1, 8, 0x0, "8b", OQK_OPD_VARIANT},
@@ -2501,6 +2501,7 @@ operand_general_constraint_met_p (const aarch64_opnd_info *opnds, int idx,
       else
 	num = 16;
       num = num / aarch64_get_qualifier_esize (qualifier) - 1;
+      assert (aarch64_get_qualifier_nelem (qualifier) == 1);
 
       /* Index out-of-range.  */
       if (!value_in_range_p (opnd->reglane.index, 0, num))

Comments

Tamar Christina Oct. 4, 2018, 11:56 a.m. | #1
Hi Matthew,

Thanks for fixing this!
I'm not a maintainer so you still need approval, but this looks good to me.

We should also backport this to 2.31

Thanks,
Tamar

> -----Original Message-----

> From: binutils-owner@sourceware.org <binutils-owner@sourceware.org>

> On Behalf Of Matthew Malcomson

> Sent: Thursday, October 4, 2018 10:16

> To: binutils@sourceware.org

> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;

> Marcus Shawcroft <Marcus.Shawcroft@arm.com>

> Subject: [PATCH][BINUTILS][AARCH64] Fix error checking for SIMD udot (by

> element)

> 

> [PATCH][BINUTILS][AARCH64] Fix error checking for SIMD udot (by element)

> 

> The SIMD UDOT instruction assembly has an unusual operand that selects a

> single

> 32 bit element with the mnemonic 4B.

> This unusual mnemonic is handled by a special operand qualifier and

> associated qualifier data in `aarch64_opnd_qualifiers`.

> 

> The current qualifier data describes 4 1-byte elements with the structure {1,

> 4, 0x0, "4b", OQK_OPD_VARIANT} This makes sense, as the instruction does

> work on 4 1-byte elements, however some logic in the

> `operand_general_constraint_met_p` makes assumptions about the range

> of index allowed when selecting a SIMD_ELEMENT depending on element

> size.

> That function reasons that e.g. in order to select a byte-sized element in a 16

> byte V register an index must allow selection of one of the 16 elements and

> hence its range will be in [0,15].

> 

> This reasoning breaks with the above description of a 4 part selection of 1

> byte elements and allows an index outside the valid [0,3] range, triggering an

> assert later on in the program in `aarch64_ins_reglane`.

> 

> vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new

> -march=armv8.4-a

> as-new: ../../binutils-gdb/opcodes/aarch64-asm.c:134: aarch64_ins_reglane:

> Assertion `reglane_index < 4' failed.

> {standard input}: Assembler messages:

> {standard input}:1: Internal error (Aborted).

> Please report this bug.

> 

> 

> This patch changes the operand qualifier data so that it describes a single

> 32 bit element.

> {4, 1, 0x0, "4b", OQK_OPD_VARIANT}

> Hence the calculation in `operand_general_constraint_met_p` provides the

> correct answer and the usual error checking machinery is used.

> 

> vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new

> -march=armv8.4-a {standard input}: Assembler messages:

> {standard input}:1: Error: register element index out of range 0 to 3 at

> operand 3 -- `udot v0.2s,v1.8b,v2.4b[4]'

> 

> Ran tests on aarch64-none-linux-gnu.

> Ok for trunk?

> 

> gas/ChangeLog:

> 

> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

> 

> 	* testsuite/gas/aarch64/illegal-dotproduct.d: New test.

> 	* testsuite/gas/aarch64/illegal-dotproduct.l: New test.

> 	* testsuite/gas/aarch64/illegal-dotproduct.s: New test.

> 

> opcodes/ChangeLog:

> 

> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

> 

> 	* aarch64-opc.c (struct operand_qualifier_data): Change qualifier

> data

> 	corresponding to AARCH64_OPND_QLF_S_4B qualifier.

> 

> 

> ###############     Attachment also inlined for ease of reply

> ###############

> 

> 

> diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.d

> b/gas/testsuite/gas/aarch64/illegal-dotproduct.d

> new file mode 100644

> index

> 0000000000000000000000000000000000000000..3f8928da83bb64891528cf5860

> 236f4b73c184b8

> --- /dev/null

> +++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.d

> @@ -0,0 +1,4 @@

> +#as: -march=armv8.2-a+dotprod

> +#name: Invalid dotproduct instructions.

> +#source: illegal-dotproduct.s

> +#error_output: illegal-dotproduct.l

> diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.l

> b/gas/testsuite/gas/aarch64/illegal-dotproduct.l

> new file mode 100644

> index

> 0000000000000000000000000000000000000000..06d0d78b8dc91d7d140415aa5

> a4a67d4b20edefd

> --- /dev/null

> +++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.l

> @@ -0,0 +1,13 @@

> +[^:]+: Assembler messages:

> +[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 --

> `udot V0.2S,V0.8B,V0.4B\[4\]'

> +[^:]+:[0-9]+: Error: operand mismatch -- `udot V0.4S,V0.8B,V0.4B\[4\]'

> +[^:]+:[0-9]+: Info:    did you mean this\?

> +[^:]+:[0-9]+: Info:    	udot v0.2s, v0.8b, v0.4b\[4\]

> +[^:]+:[0-9]+: Info:    other valid variant\(s\):

> +[^:]+:[0-9]+: Info:    	udot v0.4s, v0.16b, v0.4b\[4\]

> +[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 --

> `sdot V0.2S,V0.8B,V0.4B\[4\]'

> +[^:]+:[0-9]+: Error: operand mismatch -- `sdot V0.2S,V0.8B,V0.4H\[4\]'

> +[^:]+:[0-9]+: Info:    did you mean this\?

> +[^:]+:[0-9]+: Info:    	sdot v0.2s, v0.8b, v0.4b\[4\]

> +[^:]+:[0-9]+: Info:    other valid variant\(s\):

> +[^:]+:[0-9]+: Info:    	sdot v0.4s, v0.16b, v0.4b\[4\]

> diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.s

> b/gas/testsuite/gas/aarch64/illegal-dotproduct.s

> new file mode 100644

> index

> 0000000000000000000000000000000000000000..9c714ae54d8e0b45dea8d7cb6

> 455801d53a00ddc

> --- /dev/null

> +++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.s

> @@ -0,0 +1,4 @@

> +UDOT	V0.2S, V0.8B, V0.4B[4]

> +UDOT	V0.4S, V0.8B, V0.4B[4]

> +SDOT	V0.2S, V0.8B, V0.4B[4]

> +SDOT	V0.2S, V0.8B, V0.4H[4]

> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c index

> ba2af7bfc26d2f760f19cdbdd07ebe5535308d72..e59240c98db3ea8472b992a42

> 3b53db6340033ea 100644

> --- a/opcodes/aarch64-opc.c

> +++ b/opcodes/aarch64-opc.c

> @@ -698,7 +698,7 @@ struct operand_qualifier_data

> aarch64_opnd_qualifiers[] =

>    {4, 1, 0x2, "s", OQK_OPD_VARIANT},

>    {8, 1, 0x3, "d", OQK_OPD_VARIANT},

>    {16, 1, 0x4, "q", OQK_OPD_VARIANT},

> -  {1, 4, 0x0, "4b", OQK_OPD_VARIANT},

> +  {4, 1, 0x0, "4b", OQK_OPD_VARIANT},

> 

>    {1, 4, 0x0, "4b", OQK_OPD_VARIANT},

>    {1, 8, 0x0, "8b", OQK_OPD_VARIANT},

> @@ -2501,6 +2501,7 @@ operand_general_constraint_met_p (const

> aarch64_opnd_info *opnds, int idx,

>        else

>  	num = 16;

>        num = num / aarch64_get_qualifier_esize (qualifier) - 1;

> +      assert (aarch64_get_qualifier_nelem (qualifier) == 1);

> 

>        /* Index out-of-range.  */

>        if (!value_in_range_p (opnd->reglane.index, 0, num))
Matthew Malcomson Oct. 15, 2018, 2:21 p.m. | #2
ping


On 04/10/18 12:56, Tamar Christina wrote:
> Hi Matthew,

>

> Thanks for fixing this!

> I'm not a maintainer so you still need approval, but this looks good to me.

>

> We should also backport this to 2.31

>

> Thanks,

> Tamar

>

>> -----Original Message-----

>> From: binutils-owner@sourceware.org <binutils-owner@sourceware.org>

>> On Behalf Of Matthew Malcomson

>> Sent: Thursday, October 4, 2018 10:16

>> To: binutils@sourceware.org

>> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;

>> Marcus Shawcroft <Marcus.Shawcroft@arm.com>

>> Subject: [PATCH][BINUTILS][AARCH64] Fix error checking for SIMD udot (by

>> element)

>>

>> [PATCH][BINUTILS][AARCH64] Fix error checking for SIMD udot (by element)

>>

>> The SIMD UDOT instruction assembly has an unusual operand that selects a

>> single

>> 32 bit element with the mnemonic 4B.

>> This unusual mnemonic is handled by a special operand qualifier and

>> associated qualifier data in `aarch64_opnd_qualifiers`.

>>

>> The current qualifier data describes 4 1-byte elements with the structure {1,

>> 4, 0x0, "4b", OQK_OPD_VARIANT} This makes sense, as the instruction does

>> work on 4 1-byte elements, however some logic in the

>> `operand_general_constraint_met_p` makes assumptions about the range

>> of index allowed when selecting a SIMD_ELEMENT depending on element

>> size.

>> That function reasons that e.g. in order to select a byte-sized element in a 16

>> byte V register an index must allow selection of one of the 16 elements and

>> hence its range will be in [0,15].

>>

>> This reasoning breaks with the above description of a 4 part selection of 1

>> byte elements and allows an index outside the valid [0,3] range, triggering an

>> assert later on in the program in `aarch64_ins_reglane`.

>>

>> vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new

>> -march=armv8.4-a

>> as-new: ../../binutils-gdb/opcodes/aarch64-asm.c:134: aarch64_ins_reglane:

>> Assertion `reglane_index < 4' failed.

>> {standard input}: Assembler messages:

>> {standard input}:1: Internal error (Aborted).

>> Please report this bug.

>>

>>

>> This patch changes the operand qualifier data so that it describes a single

>> 32 bit element.

>> {4, 1, 0x0, "4b", OQK_OPD_VARIANT}

>> Hence the calculation in `operand_general_constraint_met_p` provides the

>> correct answer and the usual error checking machinery is used.

>>

>> vshcmd: > echo 'udot v0.2s, v1.8b, v2.4b[4]' | ../src/binutils-build/gas/as-new

>> -march=armv8.4-a {standard input}: Assembler messages:

>> {standard input}:1: Error: register element index out of range 0 to 3 at

>> operand 3 -- `udot v0.2s,v1.8b,v2.4b[4]'

>>

>> Ran tests on aarch64-none-linux-gnu.

>> Ok for trunk?

>>

>> gas/ChangeLog:

>>

>> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

>>

>> 	* testsuite/gas/aarch64/illegal-dotproduct.d: New test.

>> 	* testsuite/gas/aarch64/illegal-dotproduct.l: New test.

>> 	* testsuite/gas/aarch64/illegal-dotproduct.s: New test.

>>

>> opcodes/ChangeLog:

>>

>> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

>>

>> 	* aarch64-opc.c (struct operand_qualifier_data): Change qualifier

>> data

>> 	corresponding to AARCH64_OPND_QLF_S_4B qualifier.

>>

>>

>> ###############     Attachment also inlined for ease of reply

>> ###############

>>

>>

>> diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.d

>> b/gas/testsuite/gas/aarch64/illegal-dotproduct.d

>> new file mode 100644

>> index

>> 0000000000000000000000000000000000000000..3f8928da83bb64891528cf5860

>> 236f4b73c184b8

>> --- /dev/null

>> +++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.d

>> @@ -0,0 +1,4 @@

>> +#as: -march=armv8.2-a+dotprod

>> +#name: Invalid dotproduct instructions.

>> +#source: illegal-dotproduct.s

>> +#error_output: illegal-dotproduct.l

>> diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.l

>> b/gas/testsuite/gas/aarch64/illegal-dotproduct.l

>> new file mode 100644

>> index

>> 0000000000000000000000000000000000000000..06d0d78b8dc91d7d140415aa5

>> a4a67d4b20edefd

>> --- /dev/null

>> +++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.l

>> @@ -0,0 +1,13 @@

>> +[^:]+: Assembler messages:

>> +[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 --

>> `udot V0.2S,V0.8B,V0.4B\[4\]'

>> +[^:]+:[0-9]+: Error: operand mismatch -- `udot V0.4S,V0.8B,V0.4B\[4\]'

>> +[^:]+:[0-9]+: Info:    did you mean this\?

>> +[^:]+:[0-9]+: Info:    	udot v0.2s, v0.8b, v0.4b\[4\]

>> +[^:]+:[0-9]+: Info:    other valid variant\(s\):

>> +[^:]+:[0-9]+: Info:    	udot v0.4s, v0.16b, v0.4b\[4\]

>> +[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 --

>> `sdot V0.2S,V0.8B,V0.4B\[4\]'

>> +[^:]+:[0-9]+: Error: operand mismatch -- `sdot V0.2S,V0.8B,V0.4H\[4\]'

>> +[^:]+:[0-9]+: Info:    did you mean this\?

>> +[^:]+:[0-9]+: Info:    	sdot v0.2s, v0.8b, v0.4b\[4\]

>> +[^:]+:[0-9]+: Info:    other valid variant\(s\):

>> +[^:]+:[0-9]+: Info:    	sdot v0.4s, v0.16b, v0.4b\[4\]

>> diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.s

>> b/gas/testsuite/gas/aarch64/illegal-dotproduct.s

>> new file mode 100644

>> index

>> 0000000000000000000000000000000000000000..9c714ae54d8e0b45dea8d7cb6

>> 455801d53a00ddc

>> --- /dev/null

>> +++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.s

>> @@ -0,0 +1,4 @@

>> +UDOT	V0.2S, V0.8B, V0.4B[4]

>> +UDOT	V0.4S, V0.8B, V0.4B[4]

>> +SDOT	V0.2S, V0.8B, V0.4B[4]

>> +SDOT	V0.2S, V0.8B, V0.4H[4]

>> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c index

>> ba2af7bfc26d2f760f19cdbdd07ebe5535308d72..e59240c98db3ea8472b992a42

>> 3b53db6340033ea 100644

>> --- a/opcodes/aarch64-opc.c

>> +++ b/opcodes/aarch64-opc.c

>> @@ -698,7 +698,7 @@ struct operand_qualifier_data

>> aarch64_opnd_qualifiers[] =

>>     {4, 1, 0x2, "s", OQK_OPD_VARIANT},

>>     {8, 1, 0x3, "d", OQK_OPD_VARIANT},

>>     {16, 1, 0x4, "q", OQK_OPD_VARIANT},

>> -  {1, 4, 0x0, "4b", OQK_OPD_VARIANT},

>> +  {4, 1, 0x0, "4b", OQK_OPD_VARIANT},

>>

>>     {1, 4, 0x0, "4b", OQK_OPD_VARIANT},

>>     {1, 8, 0x0, "8b", OQK_OPD_VARIANT},

>> @@ -2501,6 +2501,7 @@ operand_general_constraint_met_p (const

>> aarch64_opnd_info *opnds, int idx,

>>         else

>>   	num = 16;

>>         num = num / aarch64_get_qualifier_esize (qualifier) - 1;

>> +      assert (aarch64_get_qualifier_nelem (qualifier) == 1);

>>

>>         /* Index out-of-range.  */

>>         if (!value_in_range_p (opnd->reglane.index, 0, num))
Nick Clifton Oct. 16, 2018, 4:16 p.m. | #3
Hi Matthew,

> ping


Sorry...

>>> gas/ChangeLog:

>>>

>>> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

>>>

>>>     * testsuite/gas/aarch64/illegal-dotproduct.d: New test.

>>>     * testsuite/gas/aarch64/illegal-dotproduct.l: New test.

>>>     * testsuite/gas/aarch64/illegal-dotproduct.s: New test.

>>>

>>> opcodes/ChangeLog:

>>>

>>> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

>>>

>>>     * aarch64-opc.c (struct operand_qualifier_data): Change qualifier

>>> data

>>>     corresponding to AARCH64_OPND_QLF_S_4B qualifier.

>>>

 
Approved - please apply.

Cheers
  Nick
Tamar Christina Oct. 16, 2018, 4:20 p.m. | #4
Hi Nick,

Is this also ok for backporting to binutils-2.31?

> -----Original Message-----

> From: binutils-owner@sourceware.org <binutils-owner@sourceware.org>

> On Behalf Of Nick Clifton

> Sent: Tuesday, October 16, 2018 17:17

> To: Matthew Malcomson <Matthew.Malcomson@arm.com>;

> binutils@sourceware.org

> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;

> Marcus Shawcroft <Marcus.Shawcroft@arm.com>

> Subject: Re: [PATCH][BINUTILS][AARCH64] Fix error checking for SIMD udot

> (by element)

> 

> Hi Matthew,

> 

> > ping

> 

> Sorry...

> 

> >>> gas/ChangeLog:

> >>>

> >>> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

> >>>

> >>>     * testsuite/gas/aarch64/illegal-dotproduct.d: New test.

> >>>     * testsuite/gas/aarch64/illegal-dotproduct.l: New test.

> >>>     * testsuite/gas/aarch64/illegal-dotproduct.s: New test.

> >>>

> >>> opcodes/ChangeLog:

> >>>

> >>> 2018-10-04  Matthew Malcomson  <matthew.malcomson@arm.com>

> >>>

> >>>     * aarch64-opc.c (struct operand_qualifier_data): Change

> >>> qualifier data

> >>>     corresponding to AARCH64_OPND_QLF_S_4B qualifier.

> >>>

> 

> Approved - please apply.

> 

> Cheers

>   Nick
Nick Clifton Oct. 16, 2018, 4:40 p.m. | #5
Hi Tamar,

> Is this also ok for backporting to binutils-2.31?


Yes, please do.

Cheers
  Nick

Patch

diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.d b/gas/testsuite/gas/aarch64/illegal-dotproduct.d
new file mode 100644
index 0000000000000000000000000000000000000000..3f8928da83bb64891528cf5860236f4b73c184b8
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.d
@@ -0,0 +1,4 @@ 
+#as: -march=armv8.2-a+dotprod
+#name: Invalid dotproduct instructions.
+#source: illegal-dotproduct.s
+#error_output: illegal-dotproduct.l
diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.l b/gas/testsuite/gas/aarch64/illegal-dotproduct.l
new file mode 100644
index 0000000000000000000000000000000000000000..06d0d78b8dc91d7d140415aa5a4a67d4b20edefd
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.l
@@ -0,0 +1,13 @@ 
+[^:]+: Assembler messages:
+[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 -- `udot V0.2S,V0.8B,V0.4B\[4\]'
+[^:]+:[0-9]+: Error: operand mismatch -- `udot V0.4S,V0.8B,V0.4B\[4\]'
+[^:]+:[0-9]+: Info:    did you mean this\?
+[^:]+:[0-9]+: Info:    	udot v0.2s, v0.8b, v0.4b\[4\]
+[^:]+:[0-9]+: Info:    other valid variant\(s\):
+[^:]+:[0-9]+: Info:    	udot v0.4s, v0.16b, v0.4b\[4\]
+[^:]+:[0-9]+: Error: register element index out of range 0 to 3 at operand 3 -- `sdot V0.2S,V0.8B,V0.4B\[4\]'
+[^:]+:[0-9]+: Error: operand mismatch -- `sdot V0.2S,V0.8B,V0.4H\[4\]'
+[^:]+:[0-9]+: Info:    did you mean this\?
+[^:]+:[0-9]+: Info:    	sdot v0.2s, v0.8b, v0.4b\[4\]
+[^:]+:[0-9]+: Info:    other valid variant\(s\):
+[^:]+:[0-9]+: Info:    	sdot v0.4s, v0.16b, v0.4b\[4\]
diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.s b/gas/testsuite/gas/aarch64/illegal-dotproduct.s
new file mode 100644
index 0000000000000000000000000000000000000000..9c714ae54d8e0b45dea8d7cb6455801d53a00ddc
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/illegal-dotproduct.s
@@ -0,0 +1,4 @@ 
+UDOT	V0.2S, V0.8B, V0.4B[4]
+UDOT	V0.4S, V0.8B, V0.4B[4]
+SDOT	V0.2S, V0.8B, V0.4B[4]
+SDOT	V0.2S, V0.8B, V0.4H[4]
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index ba2af7bfc26d2f760f19cdbdd07ebe5535308d72..e59240c98db3ea8472b992a423b53db6340033ea 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -698,7 +698,7 @@  struct operand_qualifier_data aarch64_opnd_qualifiers[] =
   {4, 1, 0x2, "s", OQK_OPD_VARIANT},
   {8, 1, 0x3, "d", OQK_OPD_VARIANT},
   {16, 1, 0x4, "q", OQK_OPD_VARIANT},
-  {1, 4, 0x0, "4b", OQK_OPD_VARIANT},
+  {4, 1, 0x0, "4b", OQK_OPD_VARIANT},
 
   {1, 4, 0x0, "4b", OQK_OPD_VARIANT},
   {1, 8, 0x0, "8b", OQK_OPD_VARIANT},
@@ -2501,6 +2501,7 @@  operand_general_constraint_met_p (const aarch64_opnd_info *opnds, int idx,
       else
 	num = 16;
       num = num / aarch64_get_qualifier_esize (qualifier) - 1;
+      assert (aarch64_get_qualifier_nelem (qualifier) == 1);
 
       /* Index out-of-range.  */
       if (!value_in_range_p (opnd->reglane.index, 0, num))