[binutils-gdb] AArch64: Fix error checking for SIMD udot (by element)

[Tamar Christina <tnfchris at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=66e6f0b760c2480e4ebdac8169adcee1fc31f689

commit 66e6f0b760c2480e4ebdac8169adcee1fc31f689
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Tue Oct 16 18:49:36 2018 +0100

    AArch64: Fix error checking for SIMD udot (by element)
    
    Committed on behalf of Matthew Malcomson:
    
    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]'

Diff:
---
 gas/ChangeLog                                  |  7 +++++++
 gas/testsuite/gas/aarch64/illegal-dotproduct.d |  4 ++++
 gas/testsuite/gas/aarch64/illegal-dotproduct.l | 13 +++++++++++++
 gas/testsuite/gas/aarch64/illegal-dotproduct.s |  4 ++++
 opcodes/ChangeLog                              |  5 +++++
 opcodes/aarch64-opc.c                          |  3 ++-
 6 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 69187bb..7ba30cb 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,10 @@
+2018-10-16  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.
+
+
 2018-10-15  Alan Modra  <amodra@gmail.com>
 
 	PR 23534
diff --git a/gas/testsuite/gas/aarch64/illegal-dotproduct.d b/gas/testsuite/gas/aarch64/illegal-dotproduct.d
new file mode 100644
index 0000000..3f8928d
--- /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 0000000..06d0d78
--- /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 0000000..9c714ae
--- /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/ChangeLog b/opcodes/ChangeLog
index 42a3469..4f5e200 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,8 @@
+2018-10-16  Matthew Malcomson  <matthew.malcomson@arm.com>
+
+	* aarch64-opc.c (struct operand_qualifier_data): Change qualifier data
+	corresponding to AARCH64_OPND_QLF_S_4B qualifier.
+
 2018-10-10  Jan Beulich  <jbeulich@suse.com>
 
 	* i386-gen.c (opcode_modifiers): Drop Size16, Size32, and
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index 5cd57d5..44d2ca6 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -703,7 +703,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},
@@ -2527,6 +2527,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))