From: 廖仕华 <shihua@iscas.ac.cn>
To: "Tsukasa OI" <research_trasio@irq.a4lg.com>
Cc: binutils@sourceware.org
Subject: Re: Re: [PATCH] RISC-V: Support Zmmul extension
Date: Wed, 13 Jul 2022 16:01:15 +0800 (GMT+08:00) [thread overview]
Message-ID: <6d85b0d2.6975d.181f6939440.Coremail.shihua@iscas.ac.cn> (raw)
In-Reply-To: <f37d5a60-e345-1e8f-f6bd-25d996de13f9@irq.a4lg.com>
Hi, Tsukasa OI
I have seen your patch, and I think your patch is better than mine.
So, I tend to use the patch that you submitted.
> -----原始邮件-----
> 发件人: "Tsukasa OI" <research_trasio@irq.a4lg.com>
> 发送时间: 2022-07-13 11:42:24 (星期三)
> 收件人: shihua@iscas.ac.cn
> 抄送: Binutils <binutils@sourceware.org>
> 主题: Re: [PATCH] RISC-V: Support Zmmul extension
>
> 廖先生,
>
> I submitted a patchset for pretty much the same purpose:
> <https: sourceware.org="" pipermail="" binutils="" 2022-july="" 121685.html="">
> but my testcase part could coexist with yours.
>
> Note that my and your patchset has a key technical difference:
> - In Tsukasa OI's patch, "M" implies "Zmmul" but
> - in LIAO Shihua's patch, it does not.
>
> I have no (or a little) preference here as long as you clean the patch a
> bit.
>
My thinking is that if "M" implies "Zmmul", "M" is actually split into two parts, "Zmmul and "Zmdiv". But we didn't define "zmdiv".
So I prefer to define the relationship between the two just like "Zbb" and "Zbkb".
Thanks,
Liao Shihua
>
>
> On 2022/07/11 16:30, shihua@iscas.ac.cn wrote:
> > From: LiaoShihua <shihua@iscas.ac.cn>
> >
> > Zmmul extension is Multiply only extension for RISC-V.It implements the multiplication subset of the M extension.
> > The encodings are identical to those of the corresponding M-extension instructions.
> >
> > bfd\ChangeLog:
>
> A backslash seems odd here. Just for curiosity, is it what you get when
> you run contrib/mklog.py on Windows?
>
> >
> > * elfxx-riscv.c (riscv_multi_subset_supports):Add support for Zmmul extension
> > (riscv_multi_subset_supports_ext):Ditto.
> >
> > include\ChangeLog:
> >
> > * opcode/riscv.h (enum riscv_insn_class):Ditto.
> >
> > opcodes\ChangeLog:
> >
> > * riscv-opc.c:Ditto.
> > ---
> > bfd/elfxx-riscv.c | 6 ++++++
> > include/opcode/riscv.h | 1 +
> > opcodes/riscv-opc.c | 10 +++++-----
> > 3 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > index bf7dc20e892..3c9c961352a 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1226,6 +1226,7 @@ static struct riscv_supported_ext riscv_supported_std_z_ext[] =
> > {"zvl16384b", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 },
> > {"zvl32768b", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 },
> > {"zvl65536b", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 },
> > + {"zmmul", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 },
>
> Although complying the rule:
>
> > /* The standard extensions must be added in canonical order. */
>
> is not strictly necessary (in fact, some Zvl* extensions are not
> canonically ordered), it would be good to have added in pretty much
> canonical order (put "zmmul" after "zihintpause").
>
> > {NULL, 0, 0, 0, 0}
> > };
> >
> > @@ -2395,6 +2396,9 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
> > return riscv_subset_supports (rps, "svinval");
> > case INSN_CLASS_H:
> > return riscv_subset_supports (rps, "h");
> > + case INSN_CLASS_M_OR_ZMMUL:
> > + return (riscv_subset_supports (rps, "m")
> > + || riscv_subset_supports (rps, "zmmul"));
>
> How about putting INSN_CLASS_M_OR_ZMMUL just after INSN_CLASS_M?
>
> > default:
> > rps->error_handler
> > (_("internal: unreachable INSN_CLASS_*"));
> > @@ -2500,6 +2504,8 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps,
> > return _("('q' and 'zfh') or 'zhinx");
> > case INSN_CLASS_H:
> > return _("h");
> > + case INSN_CLASS_M_OR_ZMMUL:
> > + return _("m' or `zmmul");
>
> Likewise.
>
> > default:
> > rps->error_handler
> > (_("internal: unreachable INSN_CLASS_*"));
> > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > index 808f05f3d7a..0021b7434ea 100644
> > --- a/include/opcode/riscv.h
> > +++ b/include/opcode/riscv.h
> > @@ -396,6 +396,7 @@ enum riscv_insn_class
> > INSN_CLASS_ZICBOP,
> > INSN_CLASS_ZICBOZ,
> > INSN_CLASS_H,
> > + INSN_CLASS_M_OR_ZMMUL,
>
> Because switch case ordering above is not the same as riscv_insn_class
> ordering, we would have multiple candidates. I chose to place
> INSN_CLASS_ZMMUL right after INSN_CLASS_ZIHINTPAUSE but putting
> INSN_CLASS_M_OR_ZMMUL right after INSN_CLASS_M seems equivalently good.
>
>
> Thanks,
> Tsukasa
>
> > };
> >
> > /* This structure holds information for a particular instruction. */
> > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> > index d5cedbe176c..958364fd5c0 100644
> > --- a/opcodes/riscv-opc.c
> > +++ b/opcodes/riscv-opc.c
> > @@ -558,15 +558,15 @@ const struct riscv_opcode riscv_opcodes[] =
> > {"amominu.d.aqrl", 64, INSN_CLASS_A, "d,t,0(s)", MATCH_AMOMINU_D|MASK_AQRL, MASK_AMOMINU_D|MASK_AQRL, match_opcode, INSN_DREF|INSN_8_BYTE },
> >
> > /* Multiply/Divide instruction subset. */
> > -{"mul", 0, INSN_CLASS_M, "d,s,t", MATCH_MUL, MASK_MUL, match_opcode, 0 },
> > -{"mulh", 0, INSN_CLASS_M, "d,s,t", MATCH_MULH, MASK_MULH, match_opcode, 0 },
> > -{"mulhu", 0, INSN_CLASS_M, "d,s,t", MATCH_MULHU, MASK_MULHU, match_opcode, 0 },
> > -{"mulhsu", 0, INSN_CLASS_M, "d,s,t", MATCH_MULHSU, MASK_MULHSU, match_opcode, 0 },
> > +{"mul", 0, INSN_CLASS_M_OR_ZMMUL, "d,s,t", MATCH_MUL, MASK_MUL, match_opcode, 0 },
> > +{"mulh", 0, INSN_CLASS_M_OR_ZMMUL, "d,s,t", MATCH_MULH, MASK_MULH, match_opcode, 0 },
> > +{"mulhu", 0, INSN_CLASS_M_OR_ZMMUL, "d,s,t", MATCH_MULHU, MASK_MULHU, match_opcode, 0 },
> > +{"mulhsu", 0, INSN_CLASS_M_OR_ZMMUL, "d,s,t", MATCH_MULHSU, MASK_MULHSU, match_opcode, 0 },
> > {"div", 0, INSN_CLASS_M, "d,s,t", MATCH_DIV, MASK_DIV, match_opcode, 0 },
> > {"divu", 0, INSN_CLASS_M, "d,s,t", MATCH_DIVU, MASK_DIVU, match_opcode, 0 },
> > {"rem", 0, INSN_CLASS_M, "d,s,t", MATCH_REM, MASK_REM, match_opcode, 0 },
> > {"remu", 0, INSN_CLASS_M, "d,s,t", MATCH_REMU, MASK_REMU, match_opcode, 0 },
> > -{"mulw", 64, INSN_CLASS_M, "d,s,t", MATCH_MULW, MASK_MULW, match_opcode, 0 },
> > +{"mulw", 64, INSN_CLASS_M_OR_ZMMUL, "d,s,t", MATCH_MULW, MASK_MULW, match_opcode, 0 },
> > {"divw", 64, INSN_CLASS_M, "d,s,t", MATCH_DIVW, MASK_DIVW, match_opcode, 0 },
> > {"divuw", 64, INSN_CLASS_M, "d,s,t", MATCH_DIVUW, MASK_DIVUW, match_opcode, 0 },
> > {"remw", 64, INSN_CLASS_M, "d,s,t", MATCH_REMW, MASK_REMW, match_opcode, 0 },
</shihua@iscas.ac.cn></https:></binutils@sourceware.org></research_trasio@irq.a4lg.com>
prev parent reply other threads:[~2022-07-13 8:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-11 7:30 shihua
2022-07-13 3:42 ` Tsukasa OI
2022-07-13 8:01 ` 廖仕华 [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=6d85b0d2.6975d.181f6939440.Coremail.shihua@iscas.ac.cn \
--to=shihua@iscas.ac.cn \
--cc=binutils@sourceware.org \
--cc=research_trasio@irq.a4lg.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).