From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id F0D383858434 for ; Wed, 13 Jul 2022 03:42:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F0D383858434 Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 571E6300089; Wed, 13 Jul 2022 03:42:25 +0000 (UTC) Message-ID: Date: Wed, 13 Jul 2022 12:42:24 +0900 Mime-Version: 1.0 Subject: Re: [PATCH] RISC-V: Support Zmmul extension Content-Language: en-US To: shihua@iscas.ac.cn References: <20220711073042.2009-1-shihua@iscas.ac.cn> From: Tsukasa OI Cc: Binutils In-Reply-To: <20220711073042.2009-1-shihua@iscas.ac.cn> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jul 2022 03:42:29 -0000 廖先生, I submitted a patchset for pretty much the same purpose: 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. On 2022/07/11 16:30, shihua@iscas.ac.cn wrote: > From: LiaoShihua > > 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 },