From: Przemyslaw Wirkus <Przemyslaw.Wirkus@arm.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>,
Richard Sandiford <Richard.Sandiford@arm.com>,
Marcus Shawcroft <Marcus.Shawcroft@arm.com>,
Binutils <binutils@sourceware.org>
Subject: RE: [PATCH][GAS][2/8] aarch64: [SME] Add SME instructions
Date: Mon, 10 Jan 2022 10:32:03 +0000 [thread overview]
Message-ID: <DB9PR08MB6618B3C0A870DB0A5FBE45FFE4509@DB9PR08MB6618.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <93eff524-92ba-aea6-df01-27febed2cc31@suse.com>
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
prev parent reply other threads:[~2022-01-10 10:32 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-25 21:08 Przemyslaw Wirkus
2022-01-06 12:59 ` Jan Beulich
2022-01-10 10:32 ` Przemyslaw Wirkus [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB9PR08MB6618B3C0A870DB0A5FBE45FFE4509@DB9PR08MB6618.eurprd08.prod.outlook.com \
--to=przemyslaw.wirkus@arm.com \
--cc=Marcus.Shawcroft@arm.com \
--cc=Richard.Earnshaw@arm.com \
--cc=Richard.Sandiford@arm.com \
--cc=binutils@sourceware.org \
--cc=jbeulich@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).