From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x435.google.com (mail-pf1-x435.google.com [IPv6:2607:f8b0:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id 85AA13858D28 for ; Mon, 25 Apr 2022 14:55:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 85AA13858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com Received: by mail-pf1-x435.google.com with SMTP id b15so15019919pfm.5 for ; Mon, 25 Apr 2022 07:55:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20210112.gappssmtp.com; s=20210112; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=kPJwQo+UH+VwIE6UMyFDgRHS7HZuS0zwX98CPTA2Dcw=; b=bxVXksgX9sxyaJ02IehUP8I6cmSvLEHJGiCTTztzMw/HeUcHZCzx8AkpVXnoRrd1D6 d+oU2n4iHfTGI1Aq7cMp3q8y+cN4XVGcxhFtO6cg4Hoj6slISQBMEwe5gEL2RIWBGjB5 l5ZNNifP4UWEhgDLgH3yujr0I/dV3gqHKWu3dvw57lJfjUrXcV4z1ZQIzQUw1YbuP6jG nRIilqZDdLD9NffDQlJ0r+ntJL58FGvdGGhRsi9yULOZKYmbckOje/gCGRUGs/ggHRqC PObi4lugE3jOm4zyGZo6M9c5szWFmum5zi7swYE4ovzVOkTonKvu4BNNTKAP5nEg4JuW 7vng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=kPJwQo+UH+VwIE6UMyFDgRHS7HZuS0zwX98CPTA2Dcw=; b=g6zDLsvZPeKdT1eu7YlWZxPidQkDw5j/amU6UYPcI885KTEltGNdnSHjhpmj5pP5AQ 1RAe7pIMWqamiQBr5Jw+34euL0QuTZdrVgzv2ehoaymryOIhRgHa4g3DWcZY/HD9G1HS iUiMI5h/Tj6wlz0qG2hjvjZ1/2LbPAwYhdwHkAeMrOlixZTGKMuR6e0S1EEPBUiA0sWM MomEU++RWMkE26COpBWEousaOy7Lp5YN7v6g83QYy6H7TfAtjsq8DXKiQQUUTPIQFiMg 2n42hTcC8HObrqwmQvdlrSnEY/HJtcYyT8O7LdPpuuP5L/ETlJc+9R4TeLI1f7j7qJd7 p4vg== X-Gm-Message-State: AOAM531n44FZhSl+Et7l4fA/GJ79Nsv0/EqyvUIfAAOlAhPVpsh7qjvR LLPjT7Gp/wZyY0J25YKyFQtfMw== X-Google-Smtp-Source: ABdhPJzjQhSzk8nc2MAubquyVkf93epp8ZHr9zIZmVQPtl5Xvy8gkGdDc6AfDP0NAYWZbuQZru/1jg== X-Received: by 2002:a63:8848:0:b0:3ab:2967:df83 with SMTP id l69-20020a638848000000b003ab2967df83mr5558957pgd.77.1650898536306; Mon, 25 Apr 2022 07:55:36 -0700 (PDT) Received: from localhost (76-210-143-223.lightspeed.sntcca.sbcglobal.net. [76.210.143.223]) by smtp.gmail.com with ESMTPSA id c13-20020a655a8d000000b003aae0fb5668sm7872036pgt.42.2022.04.25.07.55.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Apr 2022 07:55:35 -0700 (PDT) Date: Mon, 25 Apr 2022 07:55:35 -0700 (PDT) X-Google-Original-Date: Mon, 25 Apr 2022 07:52:24 PDT (-0700) Subject: Re: [PATCH 1/2] RISC-V: Support XVentanaCondOps extension In-Reply-To: CC: Nelson Chu , cmuellner@gcc.gnu.org, binutils@sourceware.org, kito.cheng@sifive.com, Jim Wilson , heiko.stuebner@vrull.com, Patrick O'Neill , lifang_xia@c-sky.com, rjiejie@linux.alibaba.com, christoph.muellner@vrull.com From: Palmer Dabbelt To: philipp.tomsich@vrull.eu Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, 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 14:55:40 -0000 On Mon, 25 Apr 2022 05:15:32 PDT (-0700), philipp.tomsich@vrull.eu wrote: > 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). , which was pointed out on Patrick's version of the patch . Generally we go with the patch that was posted first, I can understand if you guys felt like we stepped on your toes by writing the code when you said you were going to so I'm fine dropping Patrick's patch, but Lifang's was sent many months ago. > 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). The patch already has these split up, at least a bit. I definately agree that whatever we have should match the ISA manual, so if that means updating the ISA manual that works for me. > > 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 >> >