From 66e6f0b760c2480e4ebdac8169adcee1fc31f689 Mon Sep 17 00:00:00 2001 From: Matthew Malcomson Date: Tue, 16 Oct 2018 18:49:36 +0100 Subject: [PATCH] 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]' --- 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(-) create mode 100644 gas/testsuite/gas/aarch64/illegal-dotproduct.d create mode 100644 gas/testsuite/gas/aarch64/illegal-dotproduct.l create mode 100644 gas/testsuite/gas/aarch64/illegal-dotproduct.s diff --git a/gas/ChangeLog b/gas/ChangeLog index 69187bb2fe3..7ba30cbd693 100644 --- a/gas/ChangeLog +++ b/gas/ChangeLog @@ -1,3 +1,10 @@ +2018-10-16 Matthew Malcomson + + * 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 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 00000000000..3f8928da83b --- /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 00000000000..06d0d78b8dc --- /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 00000000000..9c714ae54d8 --- /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 42a34690198..4f5e200e38d 100644 --- a/opcodes/ChangeLog +++ b/opcodes/ChangeLog @@ -1,3 +1,8 @@ +2018-10-16 Matthew Malcomson + + * aarch64-opc.c (struct operand_qualifier_data): Change qualifier data + corresponding to AARCH64_OPND_QLF_S_4B qualifier. + 2018-10-10 Jan Beulich * i386-gen.c (opcode_modifiers): Drop Size16, Size32, and diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c index 5cd57d5966e..44d2ca66fab 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)) -- 2.30.2