This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH][Binutils][AArch64] Add verifier for By elem Single and Double sized instructions.
- From: Tamar Christina <Tamar dot Christina at arm dot com>
- To: "nickc at redhat dot com" <nickc at redhat dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, nd <nd at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>
- Date: Thu, 7 Feb 2019 15:26:12 +0000
- Subject: Re: [PATCH][Binutils][AArch64] Add verifier for By elem Single and Double sized instructions.
- References: <20190205162455.GA6701@arm.com> <d733010b-f64c-683c-4bca-103d5492ddd8@redhat.com>
Hi Nick,
Attached the updated patch.
Ok for master?
Thanks,
Tamar
The 02/07/2019 14:46, Nick Clifton wrote:
> Hi Tamar,
>
> > gas/ChangeLog:
> > 2019-02-05 Tamar Christina <tamar.christina@arm.com>
> >
> > PR binutils/23212
> > * testsuite/gas/aarch64/undefined_by_elem_sz_l.s: New test.
> > * testsuite/gas/aarch64/undefined_by_elem_sz_l.d: New test.
> >
> > opcodes/ChangeLog:
> > 2019-02-05 Tamar Christina <tamar.christina@arm.com>
> >
> > PR binutils/23212
> > * aarch64-opc.h (enum aarch64_field_kind): Add FLD_sz.
> > * aarch64-opc.c (verify_elem_sd): New.
> > (fields): Add FLD_sz entr.
> > * aarch64-tbl.h (_SIMD_INSN): New.
> > (aarch64_opcode_table): Add elem_sd verifier to fmla, fmls, fmul and
> > fmulx scalar and vector by element isns.
>
> Could you make a few changes please:
>
> +/* Verifier for vector by element 3 operands functions where the
> + conditions `if sz:L == 11 then UNDEFINED` holds.
> +*/
>
> Minor formatting nit. The comment closing sequence should be on the
> same line as the end of the comment. Ie:
>
> +/* Verifier for vector by element 3 operands functions where the
> + conditions `if sz:L == 11 then UNDEFINED` holds. */
>
> Also:
>
> +{
> + assert (inst->opcode);
> + assert (inst->opcode->operands[2] == AARCH64_OPND_Em);
> + const aarch64_insn undef_pattern = 0b11;
> + aarch64_insn value = encoding ? inst->value : insn;
>
> You have code before variable declarations. (Which we do have in other
> areas, but I am trying to avoid unless it is really helpful for code
> clarity).
>
> Plus (and this is the important one) I think that the binary constant
> prefix (0b) is a GCC-ism rather than something that is supported by ISO-C.
>
> Cheers
> Nick
--
diff --git a/gas/testsuite/gas/aarch64/undefined_by_elem_sz_l.d b/gas/testsuite/gas/aarch64/undefined_by_elem_sz_l.d
new file mode 100644
index 0000000000000000000000000000000000000000..41afa67d2b636b4c4fcb7f5166462ae5e2a6f127
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/undefined_by_elem_sz_l.d
@@ -0,0 +1,40 @@
+#as: -march=armv8.4-a
+#objdump: -dr
+
+.*: file format .*
+
+Disassembly of section \.text:
+
+0+ <.*>:
+[^:]+:\s+5f909000 fmul s0, s0, v16.s\[0\]
+[^:]+:\s+5ff09000 .inst 0x5ff09000 ; undefined
+[^:]+:\s+5f901000 fmla s0, s0, v16.s\[0\]
+[^:]+:\s+5ff01000 .inst 0x5ff01000 ; undefined
+[^:]+:\s+5f905000 fmls s0, s0, v16.s\[0\]
+[^:]+:\s+5ff05000 .inst 0x5ff05000 ; undefined
+[^:]+:\s+7f909000 fmulx s0, s0, v16.s\[0\]
+[^:]+:\s+7ff09000 .inst 0x7ff09000 ; undefined
+[^:]+:\s+5fd09000 fmul d0, d0, v16.d\[0\]
+[^:]+:\s+5ff09000 .inst 0x5ff09000 ; undefined
+[^:]+:\s+5fd01000 fmla d0, d0, v16.d\[0\]
+[^:]+:\s+5ff01000 .inst 0x5ff01000 ; undefined
+[^:]+:\s+5fd05000 fmls d0, d0, v16.d\[0\]
+[^:]+:\s+5ff05000 .inst 0x5ff05000 ; undefined
+[^:]+:\s+7fd09000 fmulx d0, d0, v16.d\[0\]
+[^:]+:\s+7ff09000 .inst 0x7ff09000 ; undefined
+[^:]+:\s+4f909000 fmul v0.4s, v0.4s, v16.s\[0\]
+[^:]+:\s+4ff09000 .inst 0x4ff09000 ; undefined
+[^:]+:\s+4f901000 fmla v0.4s, v0.4s, v16.s\[0\]
+[^:]+:\s+4ff01000 .inst 0x4ff01000 ; undefined
+[^:]+:\s+4f905000 fmls v0.4s, v0.4s, v16.s\[0\]
+[^:]+:\s+4ff05000 .inst 0x4ff05000 ; undefined
+[^:]+:\s+6f909000 fmulx v0.4s, v0.4s, v16.s\[0\]
+[^:]+:\s+6ff09000 .inst 0x6ff09000 ; undefined
+[^:]+:\s+4fd09000 fmul v0.2d, v0.2d, v16.d\[0\]
+[^:]+:\s+4ff09000 .inst 0x4ff09000 ; undefined
+[^:]+:\s+4fd01000 fmla v0.2d, v0.2d, v16.d\[0\]
+[^:]+:\s+4ff01000 .inst 0x4ff01000 ; undefined
+[^:]+:\s+4fd05000 fmls v0.2d, v0.2d, v16.d\[0\]
+[^:]+:\s+4ff05000 .inst 0x4ff05000 ; undefined
+[^:]+:\s+6fd09000 fmulx v0.2d, v0.2d, v16.d\[0\]
+[^:]+:\s+6ff09000 .inst 0x6ff09000 ; undefined
diff --git a/gas/testsuite/gas/aarch64/undefined_by_elem_sz_l.s b/gas/testsuite/gas/aarch64/undefined_by_elem_sz_l.s
new file mode 100644
index 0000000000000000000000000000000000000000..19e93b035f01f40019d6360480b37af229cb8040
--- /dev/null
+++ b/gas/testsuite/gas/aarch64/undefined_by_elem_sz_l.s
@@ -0,0 +1,55 @@
+# Generates tests to see if setting bit 22 (sz) and 21 (L) together correctly
+# marks the instruction as undefined. This pattern can't be created by the
+# assembler so instead manually encode it.
+.macro gen_insns opc
+ .inst \opc
+ .inst (\opc | 0x600000)
+.endm
+
+# fmul s0, s0, v16.s[0]
+gen_insns 0x5f909000
+
+# fmla s0, s0, v16.s[0]
+gen_insns 0x5f901000
+
+# fmls s0, s0, v16.s[0]
+gen_insns 0x5f905000
+
+# fmulx s0, s0, v16.s[0]
+gen_insns 0x7f909000
+
+# fmul d0, d0, v16.d[0]
+gen_insns 0x5fd09000
+
+# fmla d0, d0, v16.d[0]
+gen_insns 0x5fd01000
+
+# fmls d0, d0, v16.d[0]
+gen_insns 0x5fd05000
+
+# fmulx d0, d0, v16.d[0]
+gen_insns 0x7fd09000
+
+# fmul v0.4s, v0.4s, v16.s[0]
+gen_insns 0x4f909000
+
+# fmla v0.4s, v0.4s, v16.s[0]
+gen_insns 0x4f901000
+
+# fmls v0.4s, v0.4s, v16.s[0]
+gen_insns 0x4f905000
+
+# fmulx v0.4s, v0.4s, v16.s[0]
+gen_insns 0x6f909000
+
+# fmul v0.2d, v0.2d, v16.d[0]
+gen_insns 0x4fd09000
+
+# fmla v0.2d, v0.2d, v16.d[0]
+gen_insns 0x4fd01000
+
+# fmls v0.2d, v0.2d, v16.d[0]
+gen_insns 0x4fd05000
+
+# fmulx v0.2d, v0.2d, v16.d[0]
+gen_insns 0x6fd09000
diff --git a/opcodes/aarch64-opc.h b/opcodes/aarch64-opc.h
index ffb3b8317c83d0ebfb8b34ab485b9b2464aa4032..f6c506d04486fbe885569a741e68d0e83efdfeb0 100644
--- a/opcodes/aarch64-opc.h
+++ b/opcodes/aarch64-opc.h
@@ -146,7 +146,8 @@ enum aarch64_field_kind
FLD_rotate1,
FLD_rotate2,
FLD_rotate3,
- FLD_SM3_imm2
+ FLD_SM3_imm2,
+ FLD_sz
};
/* Field description. */
diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
index 22839ca72451152db3950b6d9b298a76ece234c4..a17411622cbc4b8bd29f4a8e9d5ef48029af09d1 100644
--- a/opcodes/aarch64-opc.c
+++ b/opcodes/aarch64-opc.c
@@ -320,6 +320,7 @@ const aarch64_field fields[] =
{ 13, 2 }, /* rotate2: Indexed element FCMLA immediate rotate. */
{ 12, 1 }, /* rotate3: FCADD immediate rotate. */
{ 12, 2 }, /* SM3: Indexed element SM3 2 bits index immediate. */
+ { 22, 1 }, /* sz: 1-bit element size select. */
};
enum aarch64_operand_class
@@ -4728,6 +4729,29 @@ verify_ldpsw (const struct aarch64_inst *inst ATTRIBUTE_UNUSED,
return ERR_OK;
}
+/* Verifier for vector by element 3 operands functions where the
+ conditions `if sz:L == 11 then UNDEFINED` holds. */
+
+static enum err_type
+verify_elem_sd (const struct aarch64_inst *inst, const aarch64_insn insn,
+ bfd_vma pc ATTRIBUTE_UNUSED, bfd_boolean encoding,
+ aarch64_operand_error *mismatch_detail ATTRIBUTE_UNUSED,
+ aarch64_instr_sequence *insn_sequence ATTRIBUTE_UNUSED)
+{
+ const aarch64_insn undef_pattern = 0x3;
+ aarch64_insn value;
+
+ assert (inst->opcode);
+ assert (inst->opcode->operands[2] == AARCH64_OPND_Em);
+ value = encoding ? inst->value : insn;
+ assert (value);
+
+ if (undef_pattern == extract_fields (value, 0, 2, FLD_sz, FLD_L))
+ return ERR_UND;
+
+ return ERR_OK;
+}
+
/* Initialize an instruction sequence insn_sequence with the instruction INST.
If INST is NULL the given insn_sequence is cleared and the sequence is left
uninitialized. */
diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
index ac05aae582c56e3f04c8183ac440c62b54bb1b49..e0c3903cd6224053e2615f5436c8382e9aa0013e 100644
--- a/opcodes/aarch64-tbl.h
+++ b/opcodes/aarch64-tbl.h
@@ -2239,6 +2239,8 @@ static const aarch64_feature_set aarch64_feature_memtag =
{ NAME, OPCODE, MASK, CLASS, OP, FP, OPS, QUALS, FLAGS, 0, 0, NULL }
#define SIMD_INSN(NAME,OPCODE,MASK,CLASS,OP,OPS,QUALS,FLAGS) \
{ NAME, OPCODE, MASK, CLASS, OP, SIMD, OPS, QUALS, FLAGS, 0, 0, NULL }
+#define _SIMD_INSN(NAME,OPCODE,MASK,CLASS,OP,OPS,QUALS,FLAGS,VERIFIER) \
+ { NAME, OPCODE, MASK, CLASS, OP, SIMD, OPS, QUALS, FLAGS, 0, 0, VERIFIER }
#define CRYP_INSN(NAME,OPCODE,MASK,CLASS,OPS,QUALS,FLAGS) \
{ NAME, OPCODE, MASK, CLASS, 0, CRYPTO, OPS, QUALS, FLAGS, 0, 0, NULL }
#define _CRC_INSN(NAME,OPCODE,MASK,CLASS,OPS,QUALS,FLAGS) \
@@ -2420,11 +2422,11 @@ struct aarch64_opcode aarch64_opcode_table[] =
SIMD_INSN ("sqdmull2",0x4f00b000, 0xff00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT_L2, F_SIZEQ),
SIMD_INSN ("sqdmulh", 0x0f00c000, 0xbf00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT, F_SIZEQ),
SIMD_INSN ("sqrdmulh",0x0f00d000, 0xbf00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT, F_SIZEQ),
- SIMD_INSN ("fmla", 0x0f801000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ),
+ _SIMD_INSN ("fmla", 0x0f801000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ, VERIFIER (elem_sd)),
SF16_INSN ("fmla", 0x0f001000, 0xbfc0f400, asimdelem, OP3 (Vd, Vn, Em16), QL_ELEMENT_FP_H, F_SIZEQ),
- SIMD_INSN ("fmls", 0x0f805000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ),
+ _SIMD_INSN ("fmls", 0x0f805000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ, VERIFIER (elem_sd)),
SF16_INSN ("fmls", 0x0f005000, 0xbfc0f400, asimdelem, OP3 (Vd, Vn, Em16), QL_ELEMENT_FP_H, F_SIZEQ),
- SIMD_INSN ("fmul", 0x0f809000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ),
+ _SIMD_INSN ("fmul", 0x0f809000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ, VERIFIER (elem_sd)),
SF16_INSN ("fmul", 0x0f009000, 0xbfc0f400, asimdelem, OP3 (Vd, Vn, Em16), QL_ELEMENT_FP_H, F_SIZEQ),
SIMD_INSN ("mla", 0x2f000000, 0xbf00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT, F_SIZEQ),
SIMD_INSN ("umlal", 0x2f002000, 0xff00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT_L, F_SIZEQ),
@@ -2434,7 +2436,7 @@ struct aarch64_opcode aarch64_opcode_table[] =
SIMD_INSN ("umlsl2", 0x6f006000, 0xff00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT_L2, F_SIZEQ),
SIMD_INSN ("umull", 0x2f00a000, 0xff00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT_L, F_SIZEQ),
SIMD_INSN ("umull2", 0x6f00a000, 0xff00f400, asimdelem, 0, OP3 (Vd, Vn, Em16), QL_ELEMENT_L2, F_SIZEQ),
- SIMD_INSN ("fmulx", 0x2f809000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ),
+ _SIMD_INSN ("fmulx", 0x2f809000, 0xbf80f400, asimdelem, 0, OP3 (Vd, Vn, Em), QL_ELEMENT_FP, F_SIZEQ, VERIFIER (elem_sd)),
SF16_INSN ("fmulx", 0x2f009000, 0xbfc0f400, asimdelem, OP3 (Vd, Vn, Em16), QL_ELEMENT_FP_H, F_SIZEQ),
RDMA_INSN ("sqrdmlah",0x2f00d000, 0xbf00f400, asimdelem, OP3 (Vd, Vn, Em16), QL_ELEMENT, F_SIZEQ),
RDMA_INSN ("sqrdmlsh",0x2f00f000, 0xbf00f400, asimdelem, OP3 (Vd, Vn, Em16), QL_ELEMENT, F_SIZEQ),
@@ -2748,13 +2750,13 @@ struct aarch64_opcode aarch64_opcode_table[] =
SIMD_INSN ("sqdmull", 0x5f00b000, 0xff00f400, asisdelem, 0, OP3 (Sd, Sn, Em16), QL_SISDL_HS, F_SSIZE),
SIMD_INSN ("sqdmulh", 0x5f00c000, 0xff00f400, asisdelem, 0, OP3 (Sd, Sn, Em16), QL_SISD_HS, F_SSIZE),
SIMD_INSN ("sqrdmulh", 0x5f00d000, 0xff00f400, asisdelem, 0, OP3 (Sd, Sn, Em16), QL_SISD_HS, F_SSIZE),
- SIMD_INSN ("fmla", 0x5f801000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE),
+ _SIMD_INSN ("fmla", 0x5f801000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE, VERIFIER (elem_sd)),
SF16_INSN ("fmla", 0x5f001000, 0xffc0f400, asisdelem, OP3 (Sd, Sn, Em16), QL_FP3_H, F_SSIZE),
- SIMD_INSN ("fmls", 0x5f805000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE),
+ _SIMD_INSN ("fmls", 0x5f805000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE, VERIFIER (elem_sd)),
SF16_INSN ("fmls", 0x5f005000, 0xffc0f400, asisdelem, OP3 (Sd, Sn, Em16), QL_FP3_H, F_SSIZE),
- SIMD_INSN ("fmul", 0x5f809000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE),
+ _SIMD_INSN ("fmul", 0x5f809000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE, VERIFIER (elem_sd)),
SF16_INSN ("fmul", 0x5f009000, 0xffc0f400, asisdelem, OP3 (Sd, Sn, Em16), QL_FP3_H, F_SSIZE),
- SIMD_INSN ("fmulx", 0x7f809000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE),
+ _SIMD_INSN ("fmulx", 0x7f809000, 0xff80f400, asisdelem, 0, OP3 (Sd, Sn, Em), QL_FP3, F_SSIZE, VERIFIER (elem_sd)),
SF16_INSN ("fmulx", 0x7f009000, 0xffc0f400, asisdelem, OP3 (Sd, Sn, Em16), QL_FP3_H, F_SSIZE),
RDMA_INSN ("sqrdmlah", 0x7f00d000, 0xff00f400, asisdelem, OP3 (Sd, Sn, Em16), QL_SISD_HS, F_SSIZE),
RDMA_INSN ("sqrdmlsh", 0x7f00f000, 0xff00f400, asisdelem, OP3 (Sd, Sn, Em16), QL_SISD_HS, F_SSIZE),