From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id 0C8B23857BB4 for ; Sun, 26 Jun 2022 10:59:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0C8B23857BB4 Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id C4D5D300089; Sun, 26 Jun 2022 10:58:56 +0000 (UTC) Message-ID: <5268a477-5e94-a098-0edc-f0130e8ae28c@irq.a4lg.com> Date: Sun, 26 Jun 2022 19:58:53 +0900 Mime-Version: 1.0 Subject: Re: [RFC PATCH v2] RISC-V: Add Zawrs ISA extension support Content-Language: en-US To: Christoph Muellner , Binutils References: <20220623152842.1606740-1-christoph.muellner@vrull.eu> From: Tsukasa OI In-Reply-To: <20220623152842.1606740-1-christoph.muellner@vrull.eu> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, 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: Sun, 26 Jun 2022 10:59:02 -0000 Functionally, LGTM. Comments below are non-functional but related to a coding standard and cleanliness. On 2022/06/24 0:28, Christoph Muellner wrote: > From: Christoph Müllner > > This patch adds support for the Zawrs ISA extension > ("wrs.nto" and "wrs.sto" instructions). > > The specification can be found here: > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc > > Note, that the Zawrs extension is not frozen or ratified yet. > Therefore this patch is an RFC and not intended to get merged. > > Changes since v1: > * Adjustments according to a specification change > > Signed-off-by: Christoph Müllner > --- > bfd/elfxx-riscv.c | 5 +++++ > gas/testsuite/gas/riscv/zawrs-32.d | 11 +++++++++++ > gas/testsuite/gas/riscv/zawrs.d | 11 +++++++++++ > gas/testsuite/gas/riscv/zawrs.s | 3 +++ > include/opcode/riscv-opc.h | 8 ++++++++ > include/opcode/riscv.h | 1 + > opcodes/riscv-opc.c | 4 ++++ > 7 files changed, 43 insertions(+) > create mode 100644 gas/testsuite/gas/riscv/zawrs-32.d > create mode 100644 gas/testsuite/gas/riscv/zawrs.d > create mode 100644 gas/testsuite/gas/riscv/zawrs.s > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > index f920e0ce9ff..597bdb0ee5e 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 }, > + {"zawrs", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 }, As per current coding standard: /* The standard extensions must be added in canonical order. */ ... this entry must be placed just before "zfh". Note that this is not a functional requirement (as list order does not affect the program) but I think this rule is a coding standard. > {NULL, 0, 0, 0, 0} > }; > > @@ -2294,6 +2295,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps, > { > case INSN_CLASS_I: > return riscv_subset_supports (rps, "i"); > + case INSN_CLASS_ZAWRS: > + return riscv_subset_supports (rps, "zawrs"); I prefer placing this case right after INSN_CLASS_A (not even a coding standard but just a preference). > case INSN_CLASS_ZICBOM: > return riscv_subset_supports (rps, "zicbom"); > case INSN_CLASS_ZICBOP: > @@ -2410,6 +2413,8 @@ riscv_multi_subset_supports_ext (riscv_parse_subset_t *rps, > { > case INSN_CLASS_I: > return "i"; > + case INSN_CLASS_ZAWRS: > + return "zawrs"; Same as above. > case INSN_CLASS_ZICSR: > return "zicsr"; > case INSN_CLASS_ZIFENCEI: > diff --git a/gas/testsuite/gas/riscv/zawrs-32.d b/gas/testsuite/gas/riscv/zawrs-32.d > new file mode 100644 > index 00000000000..32e3a07fb3a > --- /dev/null > +++ b/gas/testsuite/gas/riscv/zawrs-32.d > @@ -0,0 +1,11 @@ > +#as: -march=rv32i_zawrs > +#source: zawrs.s > +#objdump: -dr > + > +.*:[ ]+file format .* > + > +Disassembly of section .text: > + > +0+000 : > +[ ]+[0-9a-f]+:[ ]+00d00073[ ]+wrs.nto > +[ ]+[0-9a-f]+:[ ]+01d00073[ ]+wrs.sto > diff --git a/gas/testsuite/gas/riscv/zawrs.d b/gas/testsuite/gas/riscv/zawrs.d > new file mode 100644 > index 00000000000..9fe44f7e359 > --- /dev/null > +++ b/gas/testsuite/gas/riscv/zawrs.d > @@ -0,0 +1,11 @@ > +#as: -march=rv64i_zawrs > +#source: zawrs.s > +#objdump: -dr > + > +.*:[ ]+file format .* > + > +Disassembly of section .text: > + > +0+000 : > +[ ]+[0-9a-f]+:[ ]+00d00073[ ]+wrs.nto > +[ ]+[0-9a-f]+:[ ]+01d00073[ ]+wrs.sto > diff --git a/gas/testsuite/gas/riscv/zawrs.s b/gas/testsuite/gas/riscv/zawrs.s > new file mode 100644 > index 00000000000..138b7b5ca77 > --- /dev/null > +++ b/gas/testsuite/gas/riscv/zawrs.s > @@ -0,0 +1,3 @@ > +target: > + wrs.nto > + wrs.sto > diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h > index 207215b79fc..bc80a1624a6 100644 > --- a/include/opcode/riscv-opc.h > +++ b/include/opcode/riscv-opc.h > @@ -2113,6 +2113,11 @@ > #define MASK_CBO_INVAL 0xfff07fff > #define MATCH_CBO_ZERO 0x40200f > #define MASK_CBO_ZERO 0xfff07fff > +/* Zawrs intructions. */ > +#define MATCH_WRS_NTO 0x00d00073 > +#define MASK_WRS_NTO 0xffffffff > +#define MATCH_WRS_STO 0x01d00073 > +#define MASK_WRS_STO 0xffffffff > /* Unprivileged Counter/Timers CSR addresses. */ > #define CSR_CYCLE 0xc00 > #define CSR_TIME 0xc01 > @@ -2795,6 +2800,9 @@ DECLARE_INSN(cbo_clean, MATCH_CBO_CLEAN, MASK_CBO_CLEAN); > DECLARE_INSN(cbo_flush, MATCH_CBO_FLUSH, MASK_CBO_FLUSH); > DECLARE_INSN(cbo_inval, MATCH_CBO_INVAL, MASK_CBO_INVAL); > DECLARE_INSN(cbo_zero, MATCH_CBO_ZERO, MASK_CBO_ZERO); > +/* Zawrs instructions. */ > +DECLARE_INSN(wrs_nto, MATCH_WRS_NTO, MASK_WRS_NTO) > +DECLARE_INSN(wrs_sto, MATCH_WRS_STO, MASK_WRS_STO) > #endif /* DECLARE_INSN */ > #ifdef DECLARE_CSR > /* Unprivileged Counter/Timers CSRs. */ > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h > index 808f05f3d7a..a65b47740b8 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_ZAWRS, > }; > > /* This structure holds information for a particular instruction. */ > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c > index d5cedbe176c..1f80f090bb2 100644 > --- a/opcodes/riscv-opc.c > +++ b/opcodes/riscv-opc.c > @@ -915,6 +915,10 @@ const struct riscv_opcode riscv_opcodes[] = > {"sfence.vma", 0, INSN_CLASS_I, "s,t", MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 }, > {"wfi", 0, INSN_CLASS_I, "", MATCH_WFI, MASK_WFI, match_opcode, 0 }, > > +/* Zawrs instructions. */ > +{"wrs.nto", 0, INSN_CLASS_ZAWRS, "", MATCH_WRS_NTO, MASK_WRS_NTO, match_opcode, 0 }, > +{"wrs.sto", 0, INSN_CLASS_ZAWRS, "", MATCH_WRS_STO, MASK_WRS_STO, match_opcode, 0 }, Same as above (I prefer these lines after Zicbom and Zicboz). > + > /* Zicbom and Zicboz instructions. */ > {"cbo.clean", 0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_CLEAN, MASK_CBO_CLEAN, match_opcode, 0 }, > {"cbo.flush", 0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_FLUSH, MASK_CBO_FLUSH, match_opcode, 0 },