From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 6FE9C385843E for ; Mon, 25 Apr 2022 12:15:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6FE9C385843E 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-wr1-x429.google.com with SMTP id d5so5345431wrb.6 for ; Mon, 25 Apr 2022 05:15:48 -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=pLIqQkiv4/3ALiIVhqs761jMNPLsjuHF4oiLWkzBHL4=; b=LxJOTweQFl782gb4XMWeT6k5lldRDtmMNbdvhABSEe3Eo/7OYboslmG8DKDYnuHf3R 0/tyGsctJbBAz++o/tt4Dw7NVitA1zYSZJJHrH1ogZC0GZD50BAygNi0DcFVPCHsu8pr 22csTmOEKurO6+4/ZeeWlXgP2VDO+c6Bb8Mjkb2v1DCKVg4sH+/TgtRTp48zkOVD53VX fx7xS9/thdCpDFO+nIdMotnKFX+cZL6BhhzFXCODiZWRFOFHb3AHCQjpdOb3mCiYOq56 prF/l+bKFopAkmGJnHZ1A5Ix0x1n7NtKKkLH5/e1Iw6COs4NzhTIG3WsnJMDG5VOuAcr GZfQ== 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=pLIqQkiv4/3ALiIVhqs761jMNPLsjuHF4oiLWkzBHL4=; b=2qkKWkOkYwPVBPRF4kf7JFifNUm7Lzq6CKK3Trh+DgE8loylIVoosoTAlyuiTT+f36 A5bo88Vx1SfEEELdIoP4f0z/sxiyL93C4fEtN+SPeZoWGm3707DYJKCGfy/y3Qx6216j AgYL2ZLfCntageC7LL+S9jej2/FY6vpThloLOemPd7TDNp1Qu27BvjdzGVPJ3u1Qv9tf k7yvM4xgDQRCett9O6l+YFNQ+0ewh/UIYYiJuXcM94R/JxUcnuVJVDYg+g+hlyxhONhd 8AwFRknT9MivJDt7obMliqx0O9T05vBHzXTk8K1DMXBsbMIt5zJfUmhJJHbwVfHs8tIu +KKg== X-Gm-Message-State: AOAM533k02B43PM++pjoN7intSYSwb04UtuTa6pE9l6Ew/IQ29emV1FF aSO6dCj+qZLXNN1MZlEhYowh+WCh3b0F2s7z+AhTzA== X-Google-Smtp-Source: ABdhPJwOrVFeknDDz/VorT/hNiucfJG7IhoBk3cSuWFnvt8M26YO7aZcQiilHLYTbsJOvBIND3OvTf8mRyxXkqiBE4c= X-Received: by 2002:a5d:6d0d:0:b0:20a:d741:6949 with SMTP id e13-20020a5d6d0d000000b0020ad7416949mr6847683wrq.312.1650888946966; Mon, 25 Apr 2022 05:15:46 -0700 (PDT) MIME-Version: 1.0 References: <20220420145620.1034899-1-cmuellner@gcc.gnu.org> <20220420145620.1034899-2-cmuellner@gcc.gnu.org> In-Reply-To: From: Philipp Tomsich Date: Mon, 25 Apr 2022 14:15:32 +0200 Message-ID: Subject: Re: [PATCH 1/2] RISC-V: Support XVentanaCondOps extension To: Nelson Chu Cc: Christoph Muellner , Binutils , Kito Cheng , Jim Wilson , Heiko Stuebner , "Patrick O'Neill" , C-SKY , Jojo R , Palmer Dabbelt , Christoph Muellner Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Mon, 25 Apr 2022 12:15:52 -0000 On Mon, 25 Apr 2022 at 11:54, Nelson Chu wrote: > > On Wed, Apr 20, 2022 at 10:56 PM Christoph Muellner > wrote: > > > > From: Philipp Tomsich > > > > Ventana Micro has published the specification for their > > XVentanaCondOps ("conditional ops") extension at > > https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf > > which contains two new instructions > > - vt.maskc > > - vt.maskcn > > that can be used in constructing branchless sequences for > > various conditional-arithmetic, conditional-logical, and > > conditional-select operations. > > > > To support such vendor-defined instructions in the mainline binutils, > > this change also adds a riscv_supported_vendor_x_ext secondary > > dispatch table (but also keeps the behaviour of allowing any unknow > > X-extension to be specified in addition to the known ones from this > > table). > > > > As discussed, this change already includes the planned/agreed future > > requirements for X-extensions (which are likely to be captured in the > > riscv-toolchain-conventions repository): > > - a public specification document is available (see above) and is > > referenced from the gas-documentation > > - the naming follows chapter 27 of the RISC-V ISA specification > > - instructions are prefixed by a vendor-prefix (vt for Ventana) > > to ensure that they neither conflict with future standard > > extensions nor clash with other vendors > > > > bfd/ChangeLog: > > > > * elfxx-riscv.c (riscv_get_default_ext_version): Add riscv_supported_vendor_x_ext. > > (riscv_multi_subset_supports): Recognize INSN_CLASS_XVENTANACONDOPS. > > > > gas/ChangeLog: > > > > * doc/c-riscv.texi: Add section to list custom extensions and > > their documentation URLs. > > * testsuite/gas/riscv/x-ventana-condops.d: New test. > > * testsuite/gas/riscv/x-ventana-condops.s: New test. > > > > include/ChangeLog: > > > > * opcode/riscv-opc.h Add vt.maskc and vt.maskcn. > > * opcode/riscv.h (enum riscv_insn_class): Add INSN_CLASS_XVENTANACONDOPS. > > > > opcodes/ChangeLog: > > > > * riscv-opc.c: Add vt.maskc and vt.maskcn. > > > > v2: > > - Rebase (no changes requested for v1; see > > https://sourceware.org/pipermail/binutils/2022-January/119236.html) > > > > Signed-off-by: Philipp Tomsich > > --- > > bfd/elfxx-riscv.c | 13 +++++++++++-- > > gas/doc/c-riscv.texi | 20 ++++++++++++++++++++ > > gas/testsuite/gas/riscv/x-ventana-condops.d | 12 ++++++++++++ > > gas/testsuite/gas/riscv/x-ventana-condops.s | 4 ++++ > > include/opcode/riscv-opc.h | 17 ++++++++++++++++- > > include/opcode/riscv.h | 1 + > > opcodes/riscv-opc.c | 4 ++++ > > 7 files changed, 68 insertions(+), 3 deletions(-) > > create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops.d > > create mode 100644 gas/testsuite/gas/riscv/x-ventana-condops.s > > > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > > index cb2cc146c04..723b30ddbfc 100644 > > --- a/bfd/elfxx-riscv.c > > +++ b/bfd/elfxx-riscv.c > > @@ -1237,6 +1237,13 @@ static struct riscv_supported_ext riscv_supported_std_zxm_ext[] = > > {NULL, 0, 0, 0, 0} > > }; > > > > +static struct riscv_supported_ext riscv_supported_vendor_x_ext[] = > > +{ > > + /* XVentanaCondOps: https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf */ > > + {"xventanacondops", ISA_SPEC_CLASS_DRAFT, 1, 0, 0 }, > > + {NULL, 0, 0, 0, 0} > > +}; > > + > > const struct riscv_supported_ext *riscv_all_supported_ext[] = > > { > > riscv_supported_std_ext, > > @@ -1244,6 +1251,7 @@ const struct riscv_supported_ext *riscv_all_supported_ext[] = > > riscv_supported_std_s_ext, > > riscv_supported_std_h_ext, > > riscv_supported_std_zxm_ext, > > + riscv_supported_vendor_x_ext, > > NULL > > }; > > > > @@ -1504,8 +1512,7 @@ riscv_get_default_ext_version (enum riscv_spec_class *default_isa_spec, > > case RV_ISA_CLASS_Z: table = riscv_supported_std_z_ext; break; > > case RV_ISA_CLASS_S: table = riscv_supported_std_s_ext; break; > > case RV_ISA_CLASS_H: table = riscv_supported_std_h_ext; break; > > - case RV_ISA_CLASS_X: > > - break; > > + case RV_ISA_CLASS_X: table = riscv_supported_vendor_x_ext; break; > > default: > > table = riscv_supported_std_ext; > > } > > @@ -2402,6 +2409,8 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps, > > || riscv_subset_supports (rps, "zve32f")); > > case INSN_CLASS_SVINVAL: > > return riscv_subset_supports (rps, "svinval"); > > + case INSN_CLASS_XVENTANACONDOPS: > > + return riscv_subset_supports (rps, "xventanacondops"); > > default: > > rps->error_handler > > (_("internal: unreachable INSN_CLASS_*")); > > diff --git a/gas/doc/c-riscv.texi b/gas/doc/c-riscv.texi > > index 21d867e9cf0..c75a5ad5a08 100644 > > --- a/gas/doc/c-riscv.texi > > +++ b/gas/doc/c-riscv.texi > > @@ -20,6 +20,7 @@ > > * RISC-V-Modifiers:: RISC-V Assembler Modifiers > > * RISC-V-Formats:: RISC-V Instruction Formats > > * RISC-V-ATTRIBUTE:: RISC-V Object Attribute > > +* RISC-V-CustomExts:: RISC-V Custom (Vendor-Defined) Extensions > > @end menu > > > > @node RISC-V-Options > > @@ -692,3 +693,22 @@ the privileged specification. It will report errors if object files of > > different privileged specification versions are merged. > > > > @end table > > + > > +@node RISC-V-CustomExts > > +@section RISC-V Custom (Vendor-Defined) Extensions > > +@cindex custom (vendor-defined) extensions, RISC-V > > +@cindex RISC-V custom (vendor-defined) extensions > > + > > +The following table lists the custom (vendor-defined) RISC-V > > +extensions supported and provides the location of their > > +publicly-released documentation: > > + > > +@table @r > > +@item XVentanaCondOps > > +XVentanaCondOps extension provides instructions for branchless > > +sequences that perform conditional arithmetic, conditional > > +bitwise-logic, and conditional select operations. > > + > > +It is documented at @url{https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf}. > > + > > +@end table > > diff --git a/gas/testsuite/gas/riscv/x-ventana-condops.d b/gas/testsuite/gas/riscv/x-ventana-condops.d > > new file mode 100644 > > index 00000000000..cab0cc8dc12 > > --- /dev/null > > +++ b/gas/testsuite/gas/riscv/x-ventana-condops.d > > @@ -0,0 +1,12 @@ > > +#as: -march=rv64i_xventanacondops1p0 > > +#source: x-ventana-condops.s > > +#objdump: -d > > + > > +.*:[ ]+file format .* > > + > > + > > +Disassembly of section .text: > > + > > +0+000 : > > +[ ]+0:[ ]+00c5e57b[ ]+vt.maskc[ ]+a0,a1,a2 > > +[ ]+4:[ ]+00e6f57b[ ]+vt.maskcn[ ]+a0,a3,a4 > > diff --git a/gas/testsuite/gas/riscv/x-ventana-condops.s b/gas/testsuite/gas/riscv/x-ventana-condops.s > > new file mode 100644 > > index 00000000000..562cf7384f7 > > --- /dev/null > > +++ b/gas/testsuite/gas/riscv/x-ventana-condops.s > > @@ -0,0 +1,4 @@ > > +target: > > + vt.maskc a0, a1, a2 > > + vt.maskcn a0, a3, a4 > > + > > diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h > > index 3eea33a5dae..419ed538da9 100644 > > --- a/include/opcode/riscv-opc.h > > +++ b/include/opcode/riscv-opc.h > > @@ -2045,7 +2045,20 @@ > > #define MASK_CBO_INVAL 0xfff07fff > > #define MATCH_CBO_ZERO 0x40200f > > #define MASK_CBO_ZERO 0xfff07fff > > -/* Unprivileged Counter/Timers CSR addresses. */ > > +/* Vendor-specific (Ventana Microsystems) XVentanaCondOps instructions */ > > +#define MATCH_VT_MASKC 0x607b > > +#define MASK_VT_MASKC 0xfe00707f > > +#define MATCH_VT_MASKCN 0x707b > > +#define MASK_VT_MASKCN 0xfe00707f > > +/* Privileged CSR addresses. */ > > +#define CSR_USTATUS 0x0 > > +#define CSR_UIE 0x4 > > +#define CSR_UTVEC 0x5 > > +#define CSR_USCRATCH 0x40 > > +#define CSR_UEPC 0x41 > > +#define CSR_UCAUSE 0x42 > > +#define CSR_UTVAL 0x43 > > +#define CSR_UIP 0x44 > > These N-ext CSRs should be removed since priv 1.12 spec, so it seems > like they are added by accident here ;) As my original patch (https://sourceware.org/pipermail/binutils/2022-January/119236.html) didn't have these, I'd say that this must be an artifact from "a rebase gone wrong". I guess this comes from the rebase mentioned in Christoph's v2 changelog above... > I think it would be better to follow the rules in the PR for an > RISC-V vendor extension > (https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17). > So once the pr is approved and merged, I will say LGTM for this patch > and am happy to support Ventana's vendor extension in the master > branch. > > As for the t-head cache instruction, personally for me, it would be > better to move the whole support from the integration branch to > master, and keep the Alibaba's guys as the main author. If they agree > with the vendor rules in the PR above, and could help to rebase their > patch, it would be great :) Which integration branch are you referring to? AFAIK, there is no patch adding cache instructions floating around (which is the reason why we created a cleanroom implementation for these). We have buy-in on the vendor rules (after all, Chen Wei from T-Head is the vice-chair of the Software HC) from Alibaba, but you can expect things to take a while to propagate through into their own trees and documents. Note that in a separate effort, I am talking to T-Head to have their instructions grouped into multiple, separate extensions (and hopefully also have their documentation updated to reflect this in the same structure as we're using in the RISC-V documentation elsewhere). Cheers, Philipp. > Thanks > Nelson > > > #define CSR_CYCLE 0xc00 > > #define CSR_TIME 0xc01 > > #define CSR_INSTRET 0xc02 > > @@ -2720,6 +2733,8 @@ DECLARE_INSN(hsv_b, MATCH_HSV_B, MASK_HSV_B) > > DECLARE_INSN(hsv_h, MATCH_HSV_H, MASK_HSV_H) > > DECLARE_INSN(hsv_w, MATCH_HSV_W, MASK_HSV_W) > > DECLARE_INSN(hsv_d, MATCH_HSV_D, MASK_HSV_D) > > +DECLARE_INSN(vt_maskc, MATCH_VT_MASKC, MASK_VT_MASKC) > > +DECLARE_INSN(vt_maskcn, MATCH_VT_MASKCN, MASK_VT_MASKCN) > > #endif /* DECLARE_INSN */ > > #ifdef DECLARE_CSR > > /* Unprivileged Counter/Timers CSRs. */ > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h > > index b769769b4ec..3cbb68b5655 100644 > > --- a/include/opcode/riscv.h > > +++ b/include/opcode/riscv.h > > @@ -391,6 +391,7 @@ enum riscv_insn_class > > INSN_CLASS_ZICBOM, > > INSN_CLASS_ZICBOP, > > INSN_CLASS_ZICBOZ, > > + INSN_CLASS_XVENTANACONDOPS, > > }; > > > > /* This structure holds information for a particular instruction. */ > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c > > index 00108ff24ae..052209f6fe2 100644 > > --- a/opcodes/riscv-opc.c > > +++ b/opcodes/riscv-opc.c > > @@ -1762,6 +1762,10 @@ const struct riscv_opcode riscv_opcodes[] = > > {"hsv.w", 0, INSN_CLASS_I, "t,0(s)", MATCH_HSV_W, MASK_HSV_W, match_opcode, INSN_DREF|INSN_4_BYTE }, > > {"hsv.d", 64, INSN_CLASS_I, "t,0(s)", MATCH_HSV_D, MASK_HSV_D, match_opcode, INSN_DREF|INSN_8_BYTE }, > > > > +/* Vendor-specific (Ventana Microsystems) XVentanaCondOps instructions */ > > +{"vt.maskc", 0, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKC, MASK_VT_MASKC, match_opcode, 0 }, > > +{"vt.maskcn", 0, INSN_CLASS_XVENTANACONDOPS, "d,s,t", MATCH_VT_MASKCN, MASK_VT_MASKCN, match_opcode, 0 }, > > + > > /* Terminate the list. */ > > {0, 0, INSN_CLASS_NONE, 0, 0, 0, 0, 0} > > }; > > -- > > 2.35.1 > >