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

Mon Jan 10 10:32:03 GMT 2022

Hello,

> 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.

I see, good catch. You are right, although +sme-f64 allows sme instructions these
clearly do not belong to this test case file.

When I find an empty slot, I will clean this up.

> > --- /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.

Yes, same here. Cheers!

> Jan

Kind regards, 
Przemyslaw