From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x734.google.com (mail-qk1-x734.google.com [IPv6:2607:f8b0:4864:20::734]) by sourceware.org (Postfix) with ESMTPS id 2B35D3850862 for ; Sun, 26 Jun 2022 11:22:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2B35D3850862 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu Received: by mail-qk1-x734.google.com with SMTP id z7so5145880qko.8 for ; Sun, 26 Jun 2022 04:22:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PYK1mZOVvSUqOOPLesJzjl19MMmfRyiNoUHAvTAWZ94=; b=Q9ZNcZjffdblwUrPkx9DHW6ElNag2RWuCI6XuEOIIaocvZSOgB3BUHPG8UlyKuhQlF r8jhqWj6bAVwSPZRfIHgQBDD3oDmW+i70xI8KErBQKZga23X+Yw3m98SMETjVutAyccE JN4Nehx82LmskWPUOB5Ocuakck/isGAEuzYqR4ipCIk7aToBn9CbcLRutyANB1zI1qup oRE+Bn+UDMc1zg08ACFGmTc0jF/V7e09DF96d5QQtJ7YzQGAKSJPHmMrt+r7MhvJRJyX Q8AOHJDchfHxLHdXmJwjyQ8zFTl8fuTBcoi0IaodpTI/eVzrOD5wGQHyGk4jSAT2NJxO j87g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=PYK1mZOVvSUqOOPLesJzjl19MMmfRyiNoUHAvTAWZ94=; b=m8ITgPoC99h84yDPQcdQ/x+2jFE2xB1Gawfg7tc30iziBW/7pFslKosrxcbIzYpkyR WAyojKIgkNruoV82mu1pk+tMkuCwp1zWU2te4TleiSGKZIhGyQ7VErHgMTcgEXQn+qc3 fxgtTcH3sLP52qqA1N3xArrlrmT3ctY9+hRm+h53y6AlboiJ5TsK8pJCO6TP2n9L1tQ+ 3UywMipqm+fpT/p2vFcK60IN7+CN9z73ZSgpjJQfeYJkosG15sxne/fOly66ofgJ/4OD uGDQsBoHkzECQi0RRHcXnOmTtbGNszEglKIniEcuaSvZWwLKr0OrKyBWRqaRGGD/upa5 e7ow== X-Gm-Message-State: AJIora9tVF14gBn2KI1lwOLHm+ietwthCMQkVrsqDElGB/NslySIK1qm XlvSifgQNu803bFFDCf8ovkLZS/xD0KgvmG9ro019BXe1XM= X-Google-Smtp-Source: AGRyM1v1LJ61w4hO6UNilLByvbRLj0i5AwFuL82XuTNWoO2YvSvKT5G7ZLEizOKPtYUPF64kAO8d13blEjS/5alTK4U= X-Received: by 2002:a05:620a:570:b0:6a6:ccb6:3083 with SMTP id p16-20020a05620a057000b006a6ccb63083mr4916310qkp.408.1656242555532; Sun, 26 Jun 2022 04:22:35 -0700 (PDT) MIME-Version: 1.0 References: <20220623152842.1606740-1-christoph.muellner@vrull.eu> <5268a477-5e94-a098-0edc-f0130e8ae28c@irq.a4lg.com> In-Reply-To: <5268a477-5e94-a098-0edc-f0130e8ae28c@irq.a4lg.com> From: =?UTF-8?Q?Christoph_M=C3=BCllner?= Date: Sun, 26 Jun 2022 13:22:23 +0200 Message-ID: Subject: Re: [RFC PATCH v2] RISC-V: Add Zawrs ISA extension support To: Tsukasa OI Cc: Binutils X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, HTML_MESSAGE, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_NONE, 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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 11:22:38 -0000 On Sun, Jun 26, 2022 at 12:59 PM Tsukasa OI wrote: > Functionally, LGTM. > Comments below are non-functional but related to a coding standard and > cleanliness. > Thanks for the review! I did indeed forget to fix the locations where I added the new code. All mentioned changes will be included in a new version of the patch (will most likely be sent out once Zawrs gets frozen or if it gets changed again). > > On 2022/06/24 0:28, Christoph Muellner wrote: > > From: Christoph M=C3=BCllner > > > > 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=C3=BCllner > > --- > > 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[] =3D > > {"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. > Ok. > > > {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). > Ok. > > > 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. > Ok. > > > 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=3Drv32i_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=3Drv64i_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[] =3D > > {"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). > Ok. > > > + > > /* 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 }, >