From: "Christoph Müllner" <cmuellner@gcc.gnu.org>
To: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Nelson Chu <nelson.chu@sifive.com>,
Binutils <binutils@sourceware.org>,
Kito Cheng <kito.cheng@sifive.com>,
Jim Wilson <jim.wilson.gcc@gmail.com>,
Heiko Stuebner <heiko.stuebner@vrull.eu>,
"Patrick O'Neill" <patrick@rivosinc.com>,
C-SKY <lifang_xia@c-sky.com>, Jojo R <rjiejie@linux.alibaba.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Christoph Muellner <christoph.muellner@vrull.eu>
Subject: Re: [PATCH 1/2] RISC-V: Support XVentanaCondOps extension
Date: Mon, 25 Apr 2022 15:37:11 +0200 [thread overview]
Message-ID: <CAHB2gtTbQKijAaNy_fAg=Vc0Yok8epav6CWRPr=d9a0WDfdrzg@mail.gmail.com> (raw)
In-Reply-To: <CAAeLtUB7sdimUi2e9nZLyG0fPnCRy3RRRVLtC=W6U9g340irrA@mail.gmail.com>
On Mon, Apr 25, 2022 at 2:15 PM Philipp Tomsich <philipp.tomsich@vrull.eu>
wrote:
> On Mon, 25 Apr 2022 at 11:54, Nelson Chu <nelson.chu@sifive.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 10:56 PM Christoph Muellner
> > <cmuellner@gcc.gnu.org> wrote:
> > >
> > > From: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > >
> > > 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 <philipp.tomsich@vrull.eu>
> > > ---
> > > 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 <target>:
> > > +[ ]+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...
>
Yes, that's a rebase artifact.
I intend to rebase and resend once riscv-toolchain-convention#17 gets merged
unless Nelson wants to handle this directly.
>
> > 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
> > >
>
next prev parent reply other threads:[~2022-04-25 13:37 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-20 14:56 [PATCH 0/2] Add RISC-V vendor extensions: XVentanaCondOps and XTheadCmo Christoph Muellner
2022-04-20 14:56 ` [PATCH 1/2] RISC-V: Support XVentanaCondOps extension Christoph Muellner
2022-04-25 9:54 ` Nelson Chu
2022-04-25 12:15 ` Philipp Tomsich
2022-04-25 13:37 ` Christoph Müllner [this message]
2022-04-25 14:55 ` Palmer Dabbelt
2022-04-25 13:41 ` C-SKY
2022-04-20 14:56 ` [PATCH 2/2] RISC-V: Add T-Head CMO vendor extension Christoph Muellner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHB2gtTbQKijAaNy_fAg=Vc0Yok8epav6CWRPr=d9a0WDfdrzg@mail.gmail.com' \
--to=cmuellner@gcc.gnu.org \
--cc=binutils@sourceware.org \
--cc=christoph.muellner@vrull.eu \
--cc=heiko.stuebner@vrull.eu \
--cc=jim.wilson.gcc@gmail.com \
--cc=kito.cheng@sifive.com \
--cc=lifang_xia@c-sky.com \
--cc=nelson.chu@sifive.com \
--cc=palmer@dabbelt.com \
--cc=patrick@rivosinc.com \
--cc=philipp.tomsich@vrull.eu \
--cc=rjiejie@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).