Re: [PATCH][GAS][2/8] aarch64: [SME] Add SME instructions

[Jan Beulich <jbeulich@suse.com>]

On 25.10.2021 23:08, Przemyslaw Wirkus via Binutils wrote:
> Hi,
> 
> Patch is adding new SME matrix instructions. Please note additional
> instructions will be added in following patches.
> 
> OK for maste?
> 
> gas/ChangeLog:
> 
> 	* config/tc-aarch64.c (parse_sme_zada_operand):
> 	New parser.
> 	* config/tc-aarch64.c (parse_reg_with_qual):
> 	New reg parser.
> 	* config/tc-aarch64.c (R_ZA): New egister type.
> 	(parse_operands): New parser.
> 	* testsuite/gas/aarch64/sme-illegal.d: New test.
> 	* testsuite/gas/aarch64/sme-illegal.l: New test.
> 	* testsuite/gas/aarch64/sme-illegal.s: New test.
> 	* testsuite/gas/aarch64/sme.d: New test.
> 	* testsuite/gas/aarch64/sme.s: New test.
> 	* testsuite/gas/aarch64/sme-f64.d: New test.
> 	* testsuite/gas/aarch64/sme-f64.s: New test.
> 	* testsuite/gas/aarch64/sme-i64.d: New test.
> 	* testsuite/gas/aarch64/sme-i64.s: New test.
> 
> include/ChangeLog:
> 
> 	* opcode/aarch64.h (enum aarch64_opnd): New operands
> 	AARCH64_OPND_SME_ZAda_2b, AARCH64_OPND_SME_ZAda_3b and
> 	AARCH64_OPND_SME_Pm.
> 	(enum aarch64_insn_class): New instruction class sme_misc.
> 
> opcodes/ChangeLog:
> 
> 	* aarch64-opc.c (aarch64_print_operand):
> 	Print OPND_SME_ZAda_2b and OPND_SME_ZAda_3b operands.
> 	(verify_constraints): Handle OPND_SME_Pm.
> 	* aarch64-opc.h (enum aarch64_field_kind):
> 	New bit fields FLD_SME_ZAda_2b, FLD_SME_ZAda_3b and FLD_SME_Pm.
> 	* aarch64-tbl.h (OP_SME_ZADA_PN_PM_ZN_S): New qualifier set.
> 	(OP_SME_ZADA_PN_PM_ZN_D): New qualifier.
> 	(OP_SME_ZADA_PN_PM_ZN_ZM): New qualifier.
> 	(OP_SME_ZADA_S_PM_PM_S_S): New qualifier.
> 	(OP_SME_ZADA_D_PM_PM_D_D): New qualifier.
> 	(OP_SME_ZADA_S_PM_PM_H_H): New qualifier.
> 	(OP_SME_ZADA_S_PM_PM_B_B): New qualifier.
> 	(OP_SME_ZADA_D_PM_PM_H_H): New qualifier.
> 	(SME_INSN): New instruction macro.
> 	(SME_F64_INSN): New instruction macro.
> 	(SME_I64_INSN): New instruction macro.
> 	(SME_INSNC): New instruction macro.
> 	(struct aarch64_opcode): New SME instructions.

The patch being present only as attachment makes it hard to comment.
Nevertheless two remarks on the test cases:

> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sme-f64.s
> @@ -0,0 +1,32 @@
> +/* Scalable Matrix Extension (SME F64).  */
> +
> +/* FMOPA (non-widening), double-precision.  */
> +fmopa za0.d, p0/m, p1/m, z1.d, z8.d
> +fmopa za1.d, p2/m, p3/m, z2.d, z7.d
> +fmopa za2.d, p4/m, p5/m, z3.d, z6.d
> +fmopa za3.d, p6/m, p7/m, z4.d, z5.d
> +fmopa za4.d, p1/m, p0/m, z5.d, z4.d
> +fmopa za5.d, p3/m, p2/m, z6.d, z3.d
> +fmopa za6.d, p5/m, p4/m, z7.d, z2.d
> +fmopa za7.d, p7/m, p6/m, z8.d, z1.d
> +fmopa za4.d, p7/m, p0/m, z5.d, z4.d
> +fmopa za5.d, p6/m, p1/m, z6.d, z3.d
> +fmopa za6.d, p5/m, p2/m, z7.d, z2.d
> +fmopa za7.d, p4/m, p3/m, z8.d, z1.d
> +
> +/* FMOPS (non-widening), double-precision.  */
> +fmops za0.d, p0/m, p1/m, z1.d, z8.d
> +fmops za1.d, p2/m, p3/m, z2.d, z7.d
> +fmops za2.d, p4/m, p5/m, z3.d, z6.d
> +fmops za3.d, p6/m, p7/m, z4.d, z5.d
> +fmops za4.d, p1/m, p0/m, z5.d, z4.d
> +fmops za5.d, p3/m, p2/m, z6.d, z3.d
> +fmops za6.d, p5/m, p4/m, z7.d, z2.d
> +fmops za7.d, p7/m, p6/m, z8.d, z1.d

While up to here it's all f64 insns, ...

> +/* Register aliases.  */
> +foo .req za3 
> +bar .req z0
> +
> +fmopa foo.s, p6/m, p7/m, bar.h, z1.h
> +fmops foo.s, p6/m, p7/m, bar.s, z1.s

... these aren't afaict, and hence wouldn't seem to belong in this
test case. A similar issue appears to exist in i64's "Register
aliases" portion of that test case.

> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/sme.s
> @@ -0,0 +1,123 @@
> +/* Scalable Matrix Extension (SME).  */
> +
> +/* ADDHA 32-bit variant.  */
> +addha za0.s, p0/m, p1/m, z1.s
> +addha za1.s, p2/m, p3/m, z2.s
> +addha za2.s, p4/m, p5/m, z3.s
> +addha za3.s, p6/m, p7/m, z4.s
> +
> +/* ADDVA 32-bit variant.  */
> +addva za0.s, p0/m, p1/m, z1.s
> +addva za1.s, p2/m, p3/m, z2.s
> +addva za2.s, p4/m, p5/m, z3.s
> +addva za3.s, p6/m, p7/m, z4.s
> +
> +/* BFMOPA.  */
> +bfmopa za0.s, p0/m, p1/m, z1.h, z4.h
> +bfmopa za1.s, p2/m, p3/m, z2.h, z3.h
> +bfmopa za2.s, p4/m, p5/m, z3.h, z2.h
> +bfmopa za3.s, p6/m, p7/m, z4.h, z1.h
> +
> +/* BFMOPS.  */
> +bfmops za0.s, p0/m, p1/m, z1.h, z4.h
> +bfmops za1.s, p2/m, p3/m, z2.h, z3.h
> +bfmops za2.s, p4/m, p5/m, z3.h, z2.h
> +bfmops za3.s, p6/m, p7/m, z4.h, z1.h
> +
> +/* FMOPA (non-widening), single-precision.  */
> +fmopa za0.s, p0/m, p1/m, z1.s, z4.s
> +fmopa za1.s, p2/m, p3/m, z2.s, z3.s
> +fmopa za2.s, p4/m, p5/m, z3.s, z2.s
> +fmopa za3.s, p6/m, p7/m, z4.s, z1.s
> +
> +/* FMOPA (widening)  */
> +fmopa za0.s, p0/m, p1/m, z1.h, z4.h
> +fmopa za1.s, p2/m, p3/m, z2.h, z3.h
> +fmopa za2.s, p4/m, p5/m, z3.h, z2.h
> +fmopa za3.s, p6/m, p7/m, z4.h, z1.h
> +
> +/* FMOPS (non-widening), single-precision.  */
> +fmops za0.s, p0/m, p1/m, z1.s, z4.s
> +fmops za1.s, p2/m, p3/m, z2.s, z3.s
> +fmops za2.s, p4/m, p5/m, z3.s, z2.s
> +fmops za3.s, p6/m, p7/m, z4.s, z1.s
> +fmops za0.s, p7/m, p0/m, z1.s, z4.s
> +fmops za1.s, p6/m, p1/m, z2.s, z3.s
> +fmops za2.s, p5/m, p2/m, z3.s, z2.s
> +fmops za3.s, p4/m, p3/m, z4.s, z1.s

Up the here comments and insns match up, ...

> +/* FMOPS (widening)  */
> +fmops za0.s, p0/m, p1/m, z1.s, z4.s
> +fmops za1.s, p2/m, p3/m, z2.s, z3.s
> +fmops za2.s, p4/m, p5/m, z3.s, z2.s
> +fmops za3.s, p6/m, p7/m, z4.s, z1.s

... but here I think source registers want to use .h suffixes.

Jan