public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v1] RISC-V: Support XVentanaCondOps extension
@ 2022-01-09 19:29 Philipp Tomsich
  2022-01-17 12:57 ` Philipp Tomsich
  2022-04-20 11:37 ` Philipp Tomsich
  0 siblings, 2 replies; 18+ messages in thread
From: Philipp Tomsich @ 2022-01-09 19:29 UTC (permalink / raw)
  To: binutils
  Cc: Kito Cheng, Nelson Chu, Greg Favor, Christoph Muellner, 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.

---

 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                  |  7 +++++++
 include/opcode/riscv.h                      |  1 +
 opcodes/riscv-opc.c                         |  4 ++++
 7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -1241,6 +1241,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,
@@ -1248,6 +1255,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
 };
 
@@ -1513,8 +1521,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;
     }
@@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
--- a/include/opcode/riscv-opc.h
+++ b/include/opcode/riscv-opc.h
@@ -2029,6 +2029,11 @@
 #define MASK_HSV_W 0xfe007fff
 #define MATCH_HSV_D 0x6e004073
 #define MASK_HSV_D 0xfe007fff
+/* 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
@@ -2628,6 +2633,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
 /* Privileged CSRs.  */
diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
index 048ab0a5d68..9384c6eb84b 100644
--- a/include/opcode/riscv.h
+++ b/include/opcode/riscv.h
@@ -388,6 +388,7 @@ enum riscv_insn_class
   INSN_CLASS_V,
   INSN_CLASS_ZVEF,
   INSN_CLASS_SVINVAL,
+  INSN_CLASS_XVENTANACONDOPS,
 };
 
 /* This structure holds information for a particular instruction.  */
diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
index 2da0f7cf0a4..6c70c5b99f3 100644
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -1753,6 +1753,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.33.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-01-09 19:29 [PATCH v1] RISC-V: Support XVentanaCondOps extension Philipp Tomsich
@ 2022-01-17 12:57 ` Philipp Tomsich
  2022-04-20 11:37 ` Philipp Tomsich
  1 sibling, 0 replies; 18+ messages in thread
From: Philipp Tomsich @ 2022-01-17 12:57 UTC (permalink / raw)
  To: binutils; +Cc: Kito Cheng, Nelson Chu, Greg Favor, Christoph Muellner

Kito & Nelson,

On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
wrote:

> 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.


Given that this doesn't have any wider effects, I would like to merge this
for the next release.
Any feedback on this patch?

Thanks,
Philipp.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-01-09 19:29 [PATCH v1] RISC-V: Support XVentanaCondOps extension Philipp Tomsich
  2022-01-17 12:57 ` Philipp Tomsich
@ 2022-04-20 11:37 ` Philipp Tomsich
  2022-04-20 12:42   ` Kito Cheng
  1 sibling, 1 reply; 18+ messages in thread
From: Philipp Tomsich @ 2022-04-20 11:37 UTC (permalink / raw)
  To: binutils; +Cc: Kito Cheng, Nelson Chu, Greg Favor, Christoph Müllner

Kito & Nelson,

What is the status on this one?
Let me know if it is approved for master, so I can rebase and commit.

Thanks,
Philipp.

On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
wrote:

> 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.
>
> ---
>
>  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                  |  7 +++++++
>  include/opcode/riscv.h                      |  1 +
>  opcodes/riscv-opc.c                         |  4 ++++
>  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -1241,6 +1241,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,
> @@ -1248,6 +1255,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
>  };
>
> @@ -1513,8 +1521,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;
>      }
> @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> --- a/include/opcode/riscv-opc.h
> +++ b/include/opcode/riscv-opc.h
> @@ -2029,6 +2029,11 @@
>  #define MASK_HSV_W 0xfe007fff
>  #define MATCH_HSV_D 0x6e004073
>  #define MASK_HSV_D 0xfe007fff
> +/* 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
> @@ -2628,6 +2633,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
>  /* Privileged CSRs.  */
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index 048ab0a5d68..9384c6eb84b 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -388,6 +388,7 @@ enum riscv_insn_class
>    INSN_CLASS_V,
>    INSN_CLASS_ZVEF,
>    INSN_CLASS_SVINVAL,
> +  INSN_CLASS_XVENTANACONDOPS,
>  };
>
>  /* This structure holds information for a particular instruction.  */
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 2da0f7cf0a4..6c70c5b99f3 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -1753,6 +1753,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.33.1
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-20 11:37 ` Philipp Tomsich
@ 2022-04-20 12:42   ` Kito Cheng
  2022-04-22 10:37     ` Philipp Tomsich
  2023-04-25 10:48     ` Philipp Tomsich
  0 siblings, 2 replies; 18+ messages in thread
From: Kito Cheng @ 2022-04-20 12:42 UTC (permalink / raw)
  To: Philipp Tomsich
  Cc: Binutils, Kito Cheng, Greg Favor, Palmer Dabbelt, Nelson Chu, Jim Wilson

Hi Philipp:

I believe we have a consensus among most GNU toolchain maintainers for
accepting vendor extension to upstream / master branch,
so I am +1 for this patch, but I think we still need Nelson, Palmer or
Jim to give something LGTM here since I am not a binutils maintainer
:)

On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> Kito & Nelson,
>
> What is the status on this one?
> Let me know if it is approved for master, so I can rebase and commit.
>
> Thanks,
> Philipp.
>
> On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
> wrote:
>
> > 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.
> >
> > ---
> >
> >  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                  |  7 +++++++
> >  include/opcode/riscv.h                      |  1 +
> >  opcodes/riscv-opc.c                         |  4 ++++
> >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> > --- a/bfd/elfxx-riscv.c
> > +++ b/bfd/elfxx-riscv.c
> > @@ -1241,6 +1241,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,
> > @@ -1248,6 +1255,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
> >  };
> >
> > @@ -1513,8 +1521,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;
> >      }
> > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> > --- a/include/opcode/riscv-opc.h
> > +++ b/include/opcode/riscv-opc.h
> > @@ -2029,6 +2029,11 @@
> >  #define MASK_HSV_W 0xfe007fff
> >  #define MATCH_HSV_D 0x6e004073
> >  #define MASK_HSV_D 0xfe007fff
> > +/* 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
> > @@ -2628,6 +2633,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
> >  /* Privileged CSRs.  */
> > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > index 048ab0a5d68..9384c6eb84b 100644
> > --- a/include/opcode/riscv.h
> > +++ b/include/opcode/riscv.h
> > @@ -388,6 +388,7 @@ enum riscv_insn_class
> >    INSN_CLASS_V,
> >    INSN_CLASS_ZVEF,
> >    INSN_CLASS_SVINVAL,
> > +  INSN_CLASS_XVENTANACONDOPS,
> >  };
> >
> >  /* This structure holds information for a particular instruction.  */
> > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> > index 2da0f7cf0a4..6c70c5b99f3 100644
> > --- a/opcodes/riscv-opc.c
> > +++ b/opcodes/riscv-opc.c
> > @@ -1753,6 +1753,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.33.1
> >
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-20 12:42   ` Kito Cheng
@ 2022-04-22 10:37     ` Philipp Tomsich
  2022-04-23  1:42       ` Kito Cheng
  2023-04-25 10:48     ` Philipp Tomsich
  1 sibling, 1 reply; 18+ messages in thread
From: Philipp Tomsich @ 2022-04-22 10:37 UTC (permalink / raw)
  To: Kito Cheng
  Cc: Binutils, Kito Cheng, Greg Favor, Palmer Dabbelt, Nelson Chu, Jim Wilson

Nelson,

Can we make some progress on this, please?
There are already some dependent/similar changes in the queue (e.g.
xtheadcmo)...

Philipp.

On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:

> Hi Philipp:
>
> I believe we have a consensus among most GNU toolchain maintainers for
> accepting vendor extension to upstream / master branch,
> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> Jim to give something LGTM here since I am not a binutils maintainer
> :)
>
> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > Kito & Nelson,
> >
> > What is the status on this one?
> > Let me know if it is approved for master, so I can rebase and commit.
> >
> > Thanks,
> > Philipp.
> >
> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
> > wrote:
> >
> > > 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.
> > >
> > > ---
> > >
> > >  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                  |  7 +++++++
> > >  include/opcode/riscv.h                      |  1 +
> > >  opcodes/riscv-opc.c                         |  4 ++++
> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> > > --- a/bfd/elfxx-riscv.c
> > > +++ b/bfd/elfxx-riscv.c
> > > @@ -1241,6 +1241,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,
> > > @@ -1248,6 +1255,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
> > >  };
> > >
> > > @@ -1513,8 +1521,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;
> > >      }
> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> > > --- a/include/opcode/riscv-opc.h
> > > +++ b/include/opcode/riscv-opc.h
> > > @@ -2029,6 +2029,11 @@
> > >  #define MASK_HSV_W 0xfe007fff
> > >  #define MATCH_HSV_D 0x6e004073
> > >  #define MASK_HSV_D 0xfe007fff
> > > +/* 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
> > > @@ -2628,6 +2633,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
> > >  /* Privileged CSRs.  */
> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > > index 048ab0a5d68..9384c6eb84b 100644
> > > --- a/include/opcode/riscv.h
> > > +++ b/include/opcode/riscv.h
> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> > >    INSN_CLASS_V,
> > >    INSN_CLASS_ZVEF,
> > >    INSN_CLASS_SVINVAL,
> > > +  INSN_CLASS_XVENTANACONDOPS,
> > >  };
> > >
> > >  /* This structure holds information for a particular instruction.  */
> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> > > --- a/opcodes/riscv-opc.c
> > > +++ b/opcodes/riscv-opc.c
> > > @@ -1753,6 +1753,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.33.1
> > >
> > >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-22 10:37     ` Philipp Tomsich
@ 2022-04-23  1:42       ` Kito Cheng
  2022-04-25  9:38         ` Nelson Chu
  0 siblings, 1 reply; 18+ messages in thread
From: Kito Cheng @ 2022-04-23  1:42 UTC (permalink / raw)
  To: Philipp Tomsich
  Cc: Kito Cheng, Binutils, Greg Favor, Palmer Dabbelt, Nelson Chu, Jim Wilson

Hi Philipp:

I guess we can settle down Conventions for vendor extension[1] before
merging this?
We are pretty close to a consensus, no objection for the generally idea,
just remaining minor stuffs on the prefix naming scheme,
so I believe that could resolve soon.

[1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17

On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> Nelson,
>
> Can we make some progress on this, please?
> There are already some dependent/similar changes in the queue (e.g. xtheadcmo)...
>
> Philipp.
>
> On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
>>
>> Hi Philipp:
>>
>> I believe we have a consensus among most GNU toolchain maintainers for
>> accepting vendor extension to upstream / master branch,
>> so I am +1 for this patch, but I think we still need Nelson, Palmer or
>> Jim to give something LGTM here since I am not a binutils maintainer
>> :)
>>
>> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
>> <philipp.tomsich@vrull.eu> wrote:
>> >
>> > Kito & Nelson,
>> >
>> > What is the status on this one?
>> > Let me know if it is approved for master, so I can rebase and commit.
>> >
>> > Thanks,
>> > Philipp.
>> >
>> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
>> > wrote:
>> >
>> > > 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.
>> > >
>> > > ---
>> > >
>> > >  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                  |  7 +++++++
>> > >  include/opcode/riscv.h                      |  1 +
>> > >  opcodes/riscv-opc.c                         |  4 ++++
>> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
>> > > --- a/bfd/elfxx-riscv.c
>> > > +++ b/bfd/elfxx-riscv.c
>> > > @@ -1241,6 +1241,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,
>> > > @@ -1248,6 +1255,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
>> > >  };
>> > >
>> > > @@ -1513,8 +1521,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;
>> > >      }
>> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
>> > > --- a/include/opcode/riscv-opc.h
>> > > +++ b/include/opcode/riscv-opc.h
>> > > @@ -2029,6 +2029,11 @@
>> > >  #define MASK_HSV_W 0xfe007fff
>> > >  #define MATCH_HSV_D 0x6e004073
>> > >  #define MASK_HSV_D 0xfe007fff
>> > > +/* 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
>> > > @@ -2628,6 +2633,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
>> > >  /* Privileged CSRs.  */
>> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
>> > > index 048ab0a5d68..9384c6eb84b 100644
>> > > --- a/include/opcode/riscv.h
>> > > +++ b/include/opcode/riscv.h
>> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
>> > >    INSN_CLASS_V,
>> > >    INSN_CLASS_ZVEF,
>> > >    INSN_CLASS_SVINVAL,
>> > > +  INSN_CLASS_XVENTANACONDOPS,
>> > >  };
>> > >
>> > >  /* This structure holds information for a particular instruction.  */
>> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>> > > index 2da0f7cf0a4..6c70c5b99f3 100644
>> > > --- a/opcodes/riscv-opc.c
>> > > +++ b/opcodes/riscv-opc.c
>> > > @@ -1753,6 +1753,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.33.1
>> > >
>> > >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-23  1:42       ` Kito Cheng
@ 2022-04-25  9:38         ` Nelson Chu
  2022-05-12 19:59           ` Philipp Tomsich
  0 siblings, 1 reply; 18+ messages in thread
From: Nelson Chu @ 2022-04-25  9:38 UTC (permalink / raw)
  To: Kito Cheng
  Cc: Philipp Tomsich, Kito Cheng, Binutils, Greg Favor,
	Palmer Dabbelt, Jim Wilson

On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
>
> Hi Philipp:
>
> I guess we can settle down Conventions for vendor extension[1] before
> merging this?
> We are pretty close to a consensus, no objection for the generally idea,
> just remaining minor stuffs on the prefix naming scheme,
> so I believe that could resolve soon.
>
> [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17

Once the PR is merged and most of the people agree with the rules
there, I will say LGTM for the vendor extensions in general.

Thanks
Nelson

> On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > Nelson,
> >
> > Can we make some progress on this, please?
> > There are already some dependent/similar changes in the queue (e.g. xtheadcmo)...
> >
> > Philipp.
> >
> > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
> >>
> >> Hi Philipp:
> >>
> >> I believe we have a consensus among most GNU toolchain maintainers for
> >> accepting vendor extension to upstream / master branch,
> >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> >> Jim to give something LGTM here since I am not a binutils maintainer
> >> :)
> >>
> >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> >> <philipp.tomsich@vrull.eu> wrote:
> >> >
> >> > Kito & Nelson,
> >> >
> >> > What is the status on this one?
> >> > Let me know if it is approved for master, so I can rebase and commit.
> >> >
> >> > Thanks,
> >> > Philipp.
> >> >
> >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
> >> > wrote:
> >> >
> >> > > 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.
> >> > >
> >> > > ---
> >> > >
> >> > >  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                  |  7 +++++++
> >> > >  include/opcode/riscv.h                      |  1 +
> >> > >  opcodes/riscv-opc.c                         |  4 ++++
> >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> >> > > --- a/bfd/elfxx-riscv.c
> >> > > +++ b/bfd/elfxx-riscv.c
> >> > > @@ -1241,6 +1241,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,
> >> > > @@ -1248,6 +1255,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
> >> > >  };
> >> > >
> >> > > @@ -1513,8 +1521,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;
> >> > >      }
> >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> >> > > --- a/include/opcode/riscv-opc.h
> >> > > +++ b/include/opcode/riscv-opc.h
> >> > > @@ -2029,6 +2029,11 @@
> >> > >  #define MASK_HSV_W 0xfe007fff
> >> > >  #define MATCH_HSV_D 0x6e004073
> >> > >  #define MASK_HSV_D 0xfe007fff
> >> > > +/* 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
> >> > > @@ -2628,6 +2633,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
> >> > >  /* Privileged CSRs.  */
> >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> >> > > index 048ab0a5d68..9384c6eb84b 100644
> >> > > --- a/include/opcode/riscv.h
> >> > > +++ b/include/opcode/riscv.h
> >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> >> > >    INSN_CLASS_V,
> >> > >    INSN_CLASS_ZVEF,
> >> > >    INSN_CLASS_SVINVAL,
> >> > > +  INSN_CLASS_XVENTANACONDOPS,
> >> > >  };
> >> > >
> >> > >  /* This structure holds information for a particular instruction.  */
> >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> >> > > --- a/opcodes/riscv-opc.c
> >> > > +++ b/opcodes/riscv-opc.c
> >> > > @@ -1753,6 +1753,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.33.1
> >> > >
> >> > >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-25  9:38         ` Nelson Chu
@ 2022-05-12 19:59           ` Philipp Tomsich
  2022-05-12 22:23             ` Palmer Dabbelt
  0 siblings, 1 reply; 18+ messages in thread
From: Philipp Tomsich @ 2022-05-12 19:59 UTC (permalink / raw)
  To: Nelson Chu
  Cc: Kito Cheng, Kito Cheng, Binutils, Greg Favor, Palmer Dabbelt, Jim Wilson

Nelson,

The PR on the toolchain conventions was merged last week after weeks of
discussions and everyone agreeing to these.
Can we move this forward now?

Thanks,
Philipp.

On Mon, 25 Apr 2022 at 11:38, Nelson Chu <nelson.chu@sifive.com> wrote:

> On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> >
> > Hi Philipp:
> >
> > I guess we can settle down Conventions for vendor extension[1] before
> > merging this?
> > We are pretty close to a consensus, no objection for the generally idea,
> > just remaining minor stuffs on the prefix naming scheme,
> > so I believe that could resolve soon.
> >
> > [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17
>
> Once the PR is merged and most of the people agree with the rules
> there, I will say LGTM for the vendor extensions in general.
>
> Thanks
> Nelson
>
> > On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
> > <philipp.tomsich@vrull.eu> wrote:
> > >
> > > Nelson,
> > >
> > > Can we make some progress on this, please?
> > > There are already some dependent/similar changes in the queue (e.g.
> xtheadcmo)...
> > >
> > > Philipp.
> > >
> > > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
> > >>
> > >> Hi Philipp:
> > >>
> > >> I believe we have a consensus among most GNU toolchain maintainers for
> > >> accepting vendor extension to upstream / master branch,
> > >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> > >> Jim to give something LGTM here since I am not a binutils maintainer
> > >> :)
> > >>
> > >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> > >> <philipp.tomsich@vrull.eu> wrote:
> > >> >
> > >> > Kito & Nelson,
> > >> >
> > >> > What is the status on this one?
> > >> > Let me know if it is approved for master, so I can rebase and
> commit.
> > >> >
> > >> > Thanks,
> > >> > Philipp.
> > >> >
> > >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <
> philipp.tomsich@vrull.eu>
> > >> > wrote:
> > >> >
> > >> > > 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.
> > >> > >
> > >> > > ---
> > >> > >
> > >> > >  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                  |  7 +++++++
> > >> > >  include/opcode/riscv.h                      |  1 +
> > >> > >  opcodes/riscv-opc.c                         |  4 ++++
> > >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> > >> > > --- a/bfd/elfxx-riscv.c
> > >> > > +++ b/bfd/elfxx-riscv.c
> > >> > > @@ -1241,6 +1241,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,
> > >> > > @@ -1248,6 +1255,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
> > >> > >  };
> > >> > >
> > >> > > @@ -1513,8 +1521,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;
> > >> > >      }
> > >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> > >> > > --- a/include/opcode/riscv-opc.h
> > >> > > +++ b/include/opcode/riscv-opc.h
> > >> > > @@ -2029,6 +2029,11 @@
> > >> > >  #define MASK_HSV_W 0xfe007fff
> > >> > >  #define MATCH_HSV_D 0x6e004073
> > >> > >  #define MASK_HSV_D 0xfe007fff
> > >> > > +/* 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
> > >> > > @@ -2628,6 +2633,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
> > >> > >  /* Privileged CSRs.  */
> > >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > >> > > index 048ab0a5d68..9384c6eb84b 100644
> > >> > > --- a/include/opcode/riscv.h
> > >> > > +++ b/include/opcode/riscv.h
> > >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> > >> > >    INSN_CLASS_V,
> > >> > >    INSN_CLASS_ZVEF,
> > >> > >    INSN_CLASS_SVINVAL,
> > >> > > +  INSN_CLASS_XVENTANACONDOPS,
> > >> > >  };
> > >> > >
> > >> > >  /* This structure holds information for a particular
> instruction.  */
> > >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> > >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> > >> > > --- a/opcodes/riscv-opc.c
> > >> > > +++ b/opcodes/riscv-opc.c
> > >> > > @@ -1753,6 +1753,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.33.1
> > >> > >
> > >> > >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-05-12 19:59           ` Philipp Tomsich
@ 2022-05-12 22:23             ` Palmer Dabbelt
  2022-05-13  7:55               ` Philipp Tomsich
  2022-05-26 19:50               ` Philipp Tomsich
  0 siblings, 2 replies; 18+ messages in thread
From: Palmer Dabbelt @ 2022-05-12 22:23 UTC (permalink / raw)
  To: philipp.tomsich
  Cc: Nelson Chu, kito.cheng, Kito Cheng, binutils, gfavor, Jim Wilson

On Thu, 12 May 2022 12:59:58 PDT (-0700), philipp.tomsich@vrull.eu wrote:
> Nelson,
>
> The PR on the toolchain conventions was merged last week after weeks of
> discussions and everyone agreeing to these.
> Can we move this forward now?

This has been a persistent problem with the RISC-V foundation.  These 
RISC-V ports are very small parts of much larger community-oriented 
projects with well established communication mechanisms, and trying to 
claim that everyone was involved in discussions that happened inside the 
RISC-V foundation just isn't accurate.  We consistently get very 
valuable feedback from upstream contributors who chose to stick to 
upstream-focused forums for discussion, pretending that feedback doesn't 
exist just leads to unnecessary friction.

There's another thread started that includes the various GNU toolchain 
projects and proposes taking support for vendor extensions.  That is the 
result of a handful of discussions, the RISC-V naming conventions are 
part of that but there's a lot more than how to name extensions that's 
proposed.  What there is the best I could come up with after talking to 
a handful of people, happy to discuss things further.  Those discussions 
really need to happen in the relevant forums, though, as this is a 
really big policy change that has wide-reaching ramifications.

> Thanks,
> Philipp.
>
> On Mon, 25 Apr 2022 at 11:38, Nelson Chu <nelson.chu@sifive.com> wrote:
>
>> On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
>> >
>> > Hi Philipp:
>> >
>> > I guess we can settle down Conventions for vendor extension[1] before
>> > merging this?
>> > We are pretty close to a consensus, no objection for the generally idea,
>> > just remaining minor stuffs on the prefix naming scheme,
>> > so I believe that could resolve soon.
>> >
>> > [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17
>>
>> Once the PR is merged and most of the people agree with the rules
>> there, I will say LGTM for the vendor extensions in general.
>>
>> Thanks
>> Nelson
>>
>> > On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
>> > <philipp.tomsich@vrull.eu> wrote:
>> > >
>> > > Nelson,
>> > >
>> > > Can we make some progress on this, please?
>> > > There are already some dependent/similar changes in the queue (e.g.
>> xtheadcmo)...
>> > >
>> > > Philipp.
>> > >
>> > > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
>> > >>
>> > >> Hi Philipp:
>> > >>
>> > >> I believe we have a consensus among most GNU toolchain maintainers for
>> > >> accepting vendor extension to upstream / master branch,
>> > >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
>> > >> Jim to give something LGTM here since I am not a binutils maintainer
>> > >> :)
>> > >>
>> > >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
>> > >> <philipp.tomsich@vrull.eu> wrote:
>> > >> >
>> > >> > Kito & Nelson,
>> > >> >
>> > >> > What is the status on this one?
>> > >> > Let me know if it is approved for master, so I can rebase and
>> commit.
>> > >> >
>> > >> > Thanks,
>> > >> > Philipp.
>> > >> >
>> > >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <
>> philipp.tomsich@vrull.eu>
>> > >> > wrote:
>> > >> >
>> > >> > > 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.
>> > >> > >
>> > >> > > ---
>> > >> > >
>> > >> > >  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                  |  7 +++++++
>> > >> > >  include/opcode/riscv.h                      |  1 +
>> > >> > >  opcodes/riscv-opc.c                         |  4 ++++
>> > >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
>> > >> > > --- a/bfd/elfxx-riscv.c
>> > >> > > +++ b/bfd/elfxx-riscv.c
>> > >> > > @@ -1241,6 +1241,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,
>> > >> > > @@ -1248,6 +1255,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
>> > >> > >  };
>> > >> > >
>> > >> > > @@ -1513,8 +1521,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;
>> > >> > >      }
>> > >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
>> > >> > > --- a/include/opcode/riscv-opc.h
>> > >> > > +++ b/include/opcode/riscv-opc.h
>> > >> > > @@ -2029,6 +2029,11 @@
>> > >> > >  #define MASK_HSV_W 0xfe007fff
>> > >> > >  #define MATCH_HSV_D 0x6e004073
>> > >> > >  #define MASK_HSV_D 0xfe007fff
>> > >> > > +/* 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
>> > >> > > @@ -2628,6 +2633,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
>> > >> > >  /* Privileged CSRs.  */
>> > >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
>> > >> > > index 048ab0a5d68..9384c6eb84b 100644
>> > >> > > --- a/include/opcode/riscv.h
>> > >> > > +++ b/include/opcode/riscv.h
>> > >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
>> > >> > >    INSN_CLASS_V,
>> > >> > >    INSN_CLASS_ZVEF,
>> > >> > >    INSN_CLASS_SVINVAL,
>> > >> > > +  INSN_CLASS_XVENTANACONDOPS,
>> > >> > >  };
>> > >> > >
>> > >> > >  /* This structure holds information for a particular
>> instruction.  */
>> > >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>> > >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
>> > >> > > --- a/opcodes/riscv-opc.c
>> > >> > > +++ b/opcodes/riscv-opc.c
>> > >> > > @@ -1753,6 +1753,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.33.1
>> > >> > >
>> > >> > >
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-05-12 22:23             ` Palmer Dabbelt
@ 2022-05-13  7:55               ` Philipp Tomsich
  2022-05-26 19:50               ` Philipp Tomsich
  1 sibling, 0 replies; 18+ messages in thread
From: Philipp Tomsich @ 2022-05-13  7:55 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Nelson Chu, kito.cheng, Kito Cheng, binutils, gfavor, Jim Wilson

On Fri, 13 May 2022 at 00:23, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 12 May 2022 12:59:58 PDT (-0700), philipp.tomsich@vrull.eu wrote:
> > Nelson,
> >
> > The PR on the toolchain conventions was merged last week after weeks of
> > discussions and everyone agreeing to these.
> > Can we move this forward now?
>
> This has been a persistent problem with the RISC-V foundation.  These
> RISC-V ports are very small parts of much larger community-oriented
> projects with well established communication mechanisms, and trying to
> claim that everyone was involved in discussions that happened inside the
> RISC-V foundation just isn't accurate.  We consistently get very
> valuable feedback from upstream contributors who chose to stick to
> upstream-focused forums for discussion, pretending that feedback doesn't
> exist just leads to unnecessary friction.

Don't get me wrong here (I am merely trying to be pragmatic): projects
don't have to adopt these conventions if they disagree. But: we had
representation from the Binutils (nelson and andrew), GCC (kito), LLVM
(asb), and QEMU (alistair) maintainers working on the (now merged)
draft document. The expectation was that these same maintainers would
represent their respective projects and then adopt a subset or
superset of these conventions.
Note that QEMU posed some additional requirements on top of these
conventions (mainly how the vendor-specific extensions should be
interfaced/factored out from the standard extensions—e.g., to make
removing them in the future easier).  I hope that anyone working on
the software ecosystem as part of the RISC-V Foundation would
earnestly believe that they can make the rules for upstream projects…

What happens inside of the RISC-V Foundation only governs its
membership. However, we need to provide some general guidelines (as
in: "If you don't have at least these minimal requirements covered,
expect to be loudly ignored.") for said membership on what it takes to
get vendor-defined extensions upstream. The conventions give a small,
pragmatic, consensus-driven minimum set of requirements (again:
upstream projects can choose to require more or less than this): (a)
publicly available documentation and the (b) subdivision of the
mnemonics namespace.  I would imagine that some projects will have
additional requirements (e.g., Should qemu-support be a requirement
for acceptance into gcc or linux? There will be different opinions,
and I don't want to presume what the consensus there will be…). In
contrast, others are waiving some of these (e.g., OpenSSL doesn't care
for mnemonics and accepts hex-encoded instructions…).

The procedural question to get vendor-defined extensions (that don't
require vendor-specific relocations) flowing into binutils is:
1. We had representation from RISC-V maintainers for binutils (I
realise that neither you nor Jim commented, but would hope that you
had been made aware of this — of not, we'll need to figure that issue
out separately…) on these conventions: Are we going to adopt them for
the RISC-V backend in binutils?
2. And of course: are there additional requirements we want to pose
(e.g. introduce a dependency on QEMU; require the vendor to list
someone as a maintainer for the relevant extension)?

We will be asking ourselves similar questions for GCC, once we submit
the respective series for GCC (where XVentanaCondOps support lives in
a separate .md-file).

Just as an aside: I didn't expect all of this to be easy and
straightforward; this is exactly the reason why I picked
XVentanaCondOps (custom instruction opcode space, just 2 instructions,
no relocations…) to get the discussions moving forward.  As soon as
something more involved gets submitted, we will have to have some
follow-on discussions and define additional rules (e.g., when
vendor-defined relocations are required).

> There's another thread started that includes the various GNU toolchain
> projects and proposes taking support for vendor extensions.  That is the
> result of a handful of discussions, the RISC-V naming conventions are
> part of that but there's a lot more than how to name extensions that's
> proposed.  What there is the best I could come up with after talking to
> a handful of people, happy to discuss things further.

Could you point me to the thread you refer to?  There have been
multiple threads in multiple forums, and I don't know which one is the
leading one.

Thanks,
Philipp.

> Those discussions
> really need to happen in the relevant forums, though, as this is a
> really big policy change that has wide-reaching ramifications.
>
> > Thanks,
> > Philipp.
> >
> > On Mon, 25 Apr 2022 at 11:38, Nelson Chu <nelson.chu@sifive.com> wrote:
> >
> >> On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> >> >
> >> > Hi Philipp:
> >> >
> >> > I guess we can settle down Conventions for vendor extension[1] before
> >> > merging this?
> >> > We are pretty close to a consensus, no objection for the generally idea,
> >> > just remaining minor stuffs on the prefix naming scheme,
> >> > so I believe that could resolve soon.
> >> >
> >> > [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17
> >>
> >> Once the PR is merged and most of the people agree with the rules
> >> there, I will say LGTM for the vendor extensions in general.
> >>
> >> Thanks
> >> Nelson
> >>
> >> > On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
> >> > <philipp.tomsich@vrull.eu> wrote:
> >> > >
> >> > > Nelson,
> >> > >
> >> > > Can we make some progress on this, please?
> >> > > There are already some dependent/similar changes in the queue (e.g.
> >> xtheadcmo)...
> >> > >
> >> > > Philipp.
> >> > >
> >> > > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
> >> > >>
> >> > >> Hi Philipp:
> >> > >>
> >> > >> I believe we have a consensus among most GNU toolchain maintainers for
> >> > >> accepting vendor extension to upstream / master branch,
> >> > >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> >> > >> Jim to give something LGTM here since I am not a binutils maintainer
> >> > >> :)
> >> > >>
> >> > >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> >> > >> <philipp.tomsich@vrull.eu> wrote:
> >> > >> >
> >> > >> > Kito & Nelson,
> >> > >> >
> >> > >> > What is the status on this one?
> >> > >> > Let me know if it is approved for master, so I can rebase and
> >> commit.
> >> > >> >
> >> > >> > Thanks,
> >> > >> > Philipp.
> >> > >> >
> >> > >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <
> >> philipp.tomsich@vrull.eu>
> >> > >> > wrote:
> >> > >> >
> >> > >> > > 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.
> >> > >> > >
> >> > >> > > ---
> >> > >> > >
> >> > >> > >  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                  |  7 +++++++
> >> > >> > >  include/opcode/riscv.h                      |  1 +
> >> > >> > >  opcodes/riscv-opc.c                         |  4 ++++
> >> > >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> >> > >> > > --- a/bfd/elfxx-riscv.c
> >> > >> > > +++ b/bfd/elfxx-riscv.c
> >> > >> > > @@ -1241,6 +1241,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,
> >> > >> > > @@ -1248,6 +1255,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
> >> > >> > >  };
> >> > >> > >
> >> > >> > > @@ -1513,8 +1521,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;
> >> > >> > >      }
> >> > >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> >> > >> > > --- a/include/opcode/riscv-opc.h
> >> > >> > > +++ b/include/opcode/riscv-opc.h
> >> > >> > > @@ -2029,6 +2029,11 @@
> >> > >> > >  #define MASK_HSV_W 0xfe007fff
> >> > >> > >  #define MATCH_HSV_D 0x6e004073
> >> > >> > >  #define MASK_HSV_D 0xfe007fff
> >> > >> > > +/* 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
> >> > >> > > @@ -2628,6 +2633,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
> >> > >> > >  /* Privileged CSRs.  */
> >> > >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> >> > >> > > index 048ab0a5d68..9384c6eb84b 100644
> >> > >> > > --- a/include/opcode/riscv.h
> >> > >> > > +++ b/include/opcode/riscv.h
> >> > >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> >> > >> > >    INSN_CLASS_V,
> >> > >> > >    INSN_CLASS_ZVEF,
> >> > >> > >    INSN_CLASS_SVINVAL,
> >> > >> > > +  INSN_CLASS_XVENTANACONDOPS,
> >> > >> > >  };
> >> > >> > >
> >> > >> > >  /* This structure holds information for a particular
> >> instruction.  */
> >> > >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> >> > >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> >> > >> > > --- a/opcodes/riscv-opc.c
> >> > >> > > +++ b/opcodes/riscv-opc.c
> >> > >> > > @@ -1753,6 +1753,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.33.1
> >> > >> > >
> >> > >> > >
> >>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-05-12 22:23             ` Palmer Dabbelt
  2022-05-13  7:55               ` Philipp Tomsich
@ 2022-05-26 19:50               ` Philipp Tomsich
  2022-05-30 19:33                 ` Palmer Dabbelt
  1 sibling, 1 reply; 18+ messages in thread
From: Philipp Tomsich @ 2022-05-26 19:50 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Nelson Chu, kito.cheng, Kito Cheng, binutils, gfavor, Jim Wilson

On Fri, 13 May 2022 at 00:23, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 12 May 2022 12:59:58 PDT (-0700), philipp.tomsich@vrull.eu wrote:
> > Nelson,
> >
> > The PR on the toolchain conventions was merged last week after weeks of
> > discussions and everyone agreeing to these.
> > Can we move this forward now?
>
> This has been a persistent problem with the RISC-V foundation.  These
> RISC-V ports are very small parts of much larger community-oriented
> projects with well established communication mechanisms, and trying to
> claim that everyone was involved in discussions that happened inside the
> RISC-V foundation just isn't accurate.  We consistently get very
> valuable feedback from upstream contributors who chose to stick to
> upstream-focused forums for discussion, pretending that feedback doesn't
> exist just leads to unnecessary friction.
>
> There's another thread started that includes the various GNU toolchain
> projects and proposes taking support for vendor extensions.  That is the
> result of a handful of discussions, the RISC-V naming conventions are
> part of that but there's a lot more than how to name extensions that's
> proposed.  What there is the best I could come up with after talking to
> a handful of people, happy to discuss things further.  Those discussions
> really need to happen in the relevant forums, though, as this is a
> really big policy change that has wide-reaching ramifications.

After rereading the discussions on the other thread, I am not clear
what the next steps are for simple (as in: does not require any custom
relocations) vendor-defined extensions in the binutils context: are
the existing mechanisms (i.e., Tag_RISCV_arch and making sure that the
naming doesn't conflict) sufficient to move forward?  If not, could
you outline the next steps and how to move forward on this?

Philipp.

> > Thanks,
> > Philipp.
> >
> > On Mon, 25 Apr 2022 at 11:38, Nelson Chu <nelson.chu@sifive.com> wrote:
> >
> >> On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> >> >
> >> > Hi Philipp:
> >> >
> >> > I guess we can settle down Conventions for vendor extension[1] before
> >> > merging this?
> >> > We are pretty close to a consensus, no objection for the generally idea,
> >> > just remaining minor stuffs on the prefix naming scheme,
> >> > so I believe that could resolve soon.
> >> >
> >> > [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17
> >>
> >> Once the PR is merged and most of the people agree with the rules
> >> there, I will say LGTM for the vendor extensions in general.
> >>
> >> Thanks
> >> Nelson
> >>
> >> > On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
> >> > <philipp.tomsich@vrull.eu> wrote:
> >> > >
> >> > > Nelson,
> >> > >
> >> > > Can we make some progress on this, please?
> >> > > There are already some dependent/similar changes in the queue (e.g.
> >> xtheadcmo)...
> >> > >
> >> > > Philipp.
> >> > >
> >> > > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
> >> > >>
> >> > >> Hi Philipp:
> >> > >>
> >> > >> I believe we have a consensus among most GNU toolchain maintainers for
> >> > >> accepting vendor extension to upstream / master branch,
> >> > >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> >> > >> Jim to give something LGTM here since I am not a binutils maintainer
> >> > >> :)
> >> > >>
> >> > >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> >> > >> <philipp.tomsich@vrull.eu> wrote:
> >> > >> >
> >> > >> > Kito & Nelson,
> >> > >> >
> >> > >> > What is the status on this one?
> >> > >> > Let me know if it is approved for master, so I can rebase and
> >> commit.
> >> > >> >
> >> > >> > Thanks,
> >> > >> > Philipp.
> >> > >> >
> >> > >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <
> >> philipp.tomsich@vrull.eu>
> >> > >> > wrote:
> >> > >> >
> >> > >> > > 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.
> >> > >> > >
> >> > >> > > ---
> >> > >> > >
> >> > >> > >  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                  |  7 +++++++
> >> > >> > >  include/opcode/riscv.h                      |  1 +
> >> > >> > >  opcodes/riscv-opc.c                         |  4 ++++
> >> > >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> >> > >> > > --- a/bfd/elfxx-riscv.c
> >> > >> > > +++ b/bfd/elfxx-riscv.c
> >> > >> > > @@ -1241,6 +1241,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,
> >> > >> > > @@ -1248,6 +1255,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
> >> > >> > >  };
> >> > >> > >
> >> > >> > > @@ -1513,8 +1521,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;
> >> > >> > >      }
> >> > >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> >> > >> > > --- a/include/opcode/riscv-opc.h
> >> > >> > > +++ b/include/opcode/riscv-opc.h
> >> > >> > > @@ -2029,6 +2029,11 @@
> >> > >> > >  #define MASK_HSV_W 0xfe007fff
> >> > >> > >  #define MATCH_HSV_D 0x6e004073
> >> > >> > >  #define MASK_HSV_D 0xfe007fff
> >> > >> > > +/* 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
> >> > >> > > @@ -2628,6 +2633,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
> >> > >> > >  /* Privileged CSRs.  */
> >> > >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> >> > >> > > index 048ab0a5d68..9384c6eb84b 100644
> >> > >> > > --- a/include/opcode/riscv.h
> >> > >> > > +++ b/include/opcode/riscv.h
> >> > >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> >> > >> > >    INSN_CLASS_V,
> >> > >> > >    INSN_CLASS_ZVEF,
> >> > >> > >    INSN_CLASS_SVINVAL,
> >> > >> > > +  INSN_CLASS_XVENTANACONDOPS,
> >> > >> > >  };
> >> > >> > >
> >> > >> > >  /* This structure holds information for a particular
> >> instruction.  */
> >> > >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> >> > >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> >> > >> > > --- a/opcodes/riscv-opc.c
> >> > >> > > +++ b/opcodes/riscv-opc.c
> >> > >> > > @@ -1753,6 +1753,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.33.1
> >> > >> > >
> >> > >> > >
> >>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-05-26 19:50               ` Philipp Tomsich
@ 2022-05-30 19:33                 ` Palmer Dabbelt
  2022-05-30 20:09                   ` Philipp Tomsich
  0 siblings, 1 reply; 18+ messages in thread
From: Palmer Dabbelt @ 2022-05-30 19:33 UTC (permalink / raw)
  To: philipp.tomsich
  Cc: Nelson Chu, kito.cheng, Kito Cheng, binutils, gfavor, Jim Wilson

On Thu, 26 May 2022 12:50:30 PDT (-0700), philipp.tomsich@vrull.eu wrote:
> On Fri, 13 May 2022 at 00:23, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Thu, 12 May 2022 12:59:58 PDT (-0700), philipp.tomsich@vrull.eu wrote:
>> > Nelson,
>> >
>> > The PR on the toolchain conventions was merged last week after weeks of
>> > discussions and everyone agreeing to these.
>> > Can we move this forward now?
>>
>> This has been a persistent problem with the RISC-V foundation.  These
>> RISC-V ports are very small parts of much larger community-oriented
>> projects with well established communication mechanisms, and trying to
>> claim that everyone was involved in discussions that happened inside the
>> RISC-V foundation just isn't accurate.  We consistently get very
>> valuable feedback from upstream contributors who chose to stick to
>> upstream-focused forums for discussion, pretending that feedback doesn't
>> exist just leads to unnecessary friction.
>>
>> There's another thread started that includes the various GNU toolchain
>> projects and proposes taking support for vendor extensions.  That is the
>> result of a handful of discussions, the RISC-V naming conventions are
>> part of that but there's a lot more than how to name extensions that's
>> proposed.  What there is the best I could come up with after talking to
>> a handful of people, happy to discuss things further.  Those discussions
>> really need to happen in the relevant forums, though, as this is a
>> really big policy change that has wide-reaching ramifications.
>
> After rereading the discussions on the other thread, I am not clear
> what the next steps are for simple (as in: does not require any custom
> relocations) vendor-defined extensions in the binutils context: are
> the existing mechanisms (i.e., Tag_RISCV_arch and making sure that the
> naming doesn't conflict) sufficient to move forward?  If not, could
> you outline the next steps and how to move forward on this?

I think that's because we didn't really finish the discussion.  I was 
intending on letting things cool off a bit, but we ended up in another 
round of silliness a few days ago so I guess it'll take a bit longer.  
This is a really important policy decision, skipping the discussion 
because we're unable to have it without devolving into a bunch of 
personal attacks against various long-term upstream contributors is the 
wrong way to do things.

As far as I can tell there's still no hardware, and given that this is 
really the poster child for breaking compatibility really don't see any 
rush -- there's certainly loads of hardware that's actually in users' 
hands that we're not supporting, that worries me way more than this.  
There's a whole host of techniques that could be used to avoid 
compatibility breaks here, that or we just agree to give up -- 
either way, that needs a proper discussion.

>
> Philipp.
>
>> > Thanks,
>> > Philipp.
>> >
>> > On Mon, 25 Apr 2022 at 11:38, Nelson Chu <nelson.chu@sifive.com> wrote:
>> >
>> >> On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
>> >> >
>> >> > Hi Philipp:
>> >> >
>> >> > I guess we can settle down Conventions for vendor extension[1] before
>> >> > merging this?
>> >> > We are pretty close to a consensus, no objection for the generally idea,
>> >> > just remaining minor stuffs on the prefix naming scheme,
>> >> > so I believe that could resolve soon.
>> >> >
>> >> > [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17
>> >>
>> >> Once the PR is merged and most of the people agree with the rules
>> >> there, I will say LGTM for the vendor extensions in general.
>> >>
>> >> Thanks
>> >> Nelson
>> >>
>> >> > On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
>> >> > <philipp.tomsich@vrull.eu> wrote:
>> >> > >
>> >> > > Nelson,
>> >> > >
>> >> > > Can we make some progress on this, please?
>> >> > > There are already some dependent/similar changes in the queue (e.g.
>> >> xtheadcmo)...
>> >> > >
>> >> > > Philipp.
>> >> > >
>> >> > > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
>> >> > >>
>> >> > >> Hi Philipp:
>> >> > >>
>> >> > >> I believe we have a consensus among most GNU toolchain maintainers for
>> >> > >> accepting vendor extension to upstream / master branch,
>> >> > >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
>> >> > >> Jim to give something LGTM here since I am not a binutils maintainer
>> >> > >> :)
>> >> > >>
>> >> > >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
>> >> > >> <philipp.tomsich@vrull.eu> wrote:
>> >> > >> >
>> >> > >> > Kito & Nelson,
>> >> > >> >
>> >> > >> > What is the status on this one?
>> >> > >> > Let me know if it is approved for master, so I can rebase and
>> >> commit.
>> >> > >> >
>> >> > >> > Thanks,
>> >> > >> > Philipp.
>> >> > >> >
>> >> > >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <
>> >> philipp.tomsich@vrull.eu>
>> >> > >> > wrote:
>> >> > >> >
>> >> > >> > > 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.
>> >> > >> > >
>> >> > >> > > ---
>> >> > >> > >
>> >> > >> > >  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                  |  7 +++++++
>> >> > >> > >  include/opcode/riscv.h                      |  1 +
>> >> > >> > >  opcodes/riscv-opc.c                         |  4 ++++
>> >> > >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
>> >> > >> > > --- a/bfd/elfxx-riscv.c
>> >> > >> > > +++ b/bfd/elfxx-riscv.c
>> >> > >> > > @@ -1241,6 +1241,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,
>> >> > >> > > @@ -1248,6 +1255,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
>> >> > >> > >  };
>> >> > >> > >
>> >> > >> > > @@ -1513,8 +1521,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;
>> >> > >> > >      }
>> >> > >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
>> >> > >> > > --- a/include/opcode/riscv-opc.h
>> >> > >> > > +++ b/include/opcode/riscv-opc.h
>> >> > >> > > @@ -2029,6 +2029,11 @@
>> >> > >> > >  #define MASK_HSV_W 0xfe007fff
>> >> > >> > >  #define MATCH_HSV_D 0x6e004073
>> >> > >> > >  #define MASK_HSV_D 0xfe007fff
>> >> > >> > > +/* 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
>> >> > >> > > @@ -2628,6 +2633,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
>> >> > >> > >  /* Privileged CSRs.  */
>> >> > >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
>> >> > >> > > index 048ab0a5d68..9384c6eb84b 100644
>> >> > >> > > --- a/include/opcode/riscv.h
>> >> > >> > > +++ b/include/opcode/riscv.h
>> >> > >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
>> >> > >> > >    INSN_CLASS_V,
>> >> > >> > >    INSN_CLASS_ZVEF,
>> >> > >> > >    INSN_CLASS_SVINVAL,
>> >> > >> > > +  INSN_CLASS_XVENTANACONDOPS,
>> >> > >> > >  };
>> >> > >> > >
>> >> > >> > >  /* This structure holds information for a particular
>> >> instruction.  */
>> >> > >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>> >> > >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
>> >> > >> > > --- a/opcodes/riscv-opc.c
>> >> > >> > > +++ b/opcodes/riscv-opc.c
>> >> > >> > > @@ -1753,6 +1753,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.33.1
>> >> > >> > >
>> >> > >> > >
>> >>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-05-30 19:33                 ` Palmer Dabbelt
@ 2022-05-30 20:09                   ` Philipp Tomsich
  0 siblings, 0 replies; 18+ messages in thread
From: Philipp Tomsich @ 2022-05-30 20:09 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Nelson Chu, kito.cheng, Kito Cheng, binutils, gfavor, Jim Wilson

On Mon, 30 May 2022 at 21:33, Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 26 May 2022 12:50:30 PDT (-0700), philipp.tomsich@vrull.eu wrote:
> > On Fri, 13 May 2022 at 00:23, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >>
> >> On Thu, 12 May 2022 12:59:58 PDT (-0700), philipp.tomsich@vrull.eu wrote:
> >> > Nelson,
> >> >
> >> > The PR on the toolchain conventions was merged last week after weeks of
> >> > discussions and everyone agreeing to these.
> >> > Can we move this forward now?
> >>
> >> This has been a persistent problem with the RISC-V foundation.  These
> >> RISC-V ports are very small parts of much larger community-oriented
> >> projects with well established communication mechanisms, and trying to
> >> claim that everyone was involved in discussions that happened inside the
> >> RISC-V foundation just isn't accurate.  We consistently get very
> >> valuable feedback from upstream contributors who chose to stick to
> >> upstream-focused forums for discussion, pretending that feedback doesn't
> >> exist just leads to unnecessary friction.
> >>
> >> There's another thread started that includes the various GNU toolchain
> >> projects and proposes taking support for vendor extensions.  That is the
> >> result of a handful of discussions, the RISC-V naming conventions are
> >> part of that but there's a lot more than how to name extensions that's
> >> proposed.  What there is the best I could come up with after talking to
> >> a handful of people, happy to discuss things further.  Those discussions
> >> really need to happen in the relevant forums, though, as this is a
> >> really big policy change that has wide-reaching ramifications.
> >
> > After rereading the discussions on the other thread, I am not clear
> > what the next steps are for simple (as in: does not require any custom
> > relocations) vendor-defined extensions in the binutils context: are
> > the existing mechanisms (i.e., Tag_RISCV_arch and making sure that the
> > naming doesn't conflict) sufficient to move forward?  If not, could
> > you outline the next steps and how to move forward on this?
>
> I think that's because we didn't really finish the discussion.  I was
> intending on letting things cool off a bit, but we ended up in another
> round of silliness a few days ago so I guess it'll take a bit longer.
> This is a really important policy decision, skipping the discussion
> because we're unable to have it without devolving into a bunch of
> personal attacks against various long-term upstream contributors is the
> wrong way to do things.

I would regard the silliness (and also the somewhat uncivilized tone
in some of the discussions) as orthogonal to this patch.

My question at this stage is whether the patch at hand (which is
defining an X-extension with 2 instructions) is acceptable — or what
prerequisites you see necessary..
I gathered from the discussion you started on policy questions for X,
that you were (generally speaking) in favor of moving X extension
support forward.
As getting X extensions supported across the ecosystem will be a
multi-faceted project, I would gravitate towards taking it
step-by-step.  So here's starting with binutils-gdb and the
non-contentious (i.e. no relocations involved, opcode-space for custom
opcodes) cases.  After that, we will start to build on XVentanaCondOps
in GCC (we'll then have to discuss/establish ground rules) and some
next-stage policy discussion will follow naturally.  Keeping these
discussions focused on small steps seems like a worthwhile attempt to
keep tempers cool.

That said: Are there any opens—other than cross-project policy
questions—related to XVentanaCondOps and binutils-gdb that you would
like to see discussed/addressed?

> As far as I can tell there's still no hardware, and given that this is
> really the poster child for breaking compatibility really don't see any
> rush -- there's certainly loads of hardware that's actually in users'
> hands that we're not supporting, that worries me way more than this.
> There's a whole host of techniques that could be used to avoid
> compatibility breaks here, that or we just agree to give up --
> either way, that needs a proper discussion.

XVentanaCondOps shouldn't break compatibility unless I am overlooking
something obvious.
readelf clearly knows that binaries built with it will require the extension:
>
> Attribute Section: riscv
> File Attributes
>   Tag_RISCV_stack_align: 16-bytes
>   Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0_zba1p0_zbb1p0_zbc1p0_zbs1p0_xventanacondops1p0"
>   Tag_RISCV_unaligned_access: Unaligned access

While I am intentionally trying to keep this focused narrowly on
XVentanaCondOps, I should point out that there's some momentum to
enable the X-extensions from Alibaba T-Head as well.  At least,
whether there's hardware available for these will be a clear-cut case.

Thanks,
Philipp.

>
> >
> > Philipp.
> >
> >> > Thanks,
> >> > Philipp.
> >> >
> >> > On Mon, 25 Apr 2022 at 11:38, Nelson Chu <nelson.chu@sifive.com> wrote:
> >> >
> >> >> On Sat, Apr 23, 2022 at 9:42 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> >> >> >
> >> >> > Hi Philipp:
> >> >> >
> >> >> > I guess we can settle down Conventions for vendor extension[1] before
> >> >> > merging this?
> >> >> > We are pretty close to a consensus, no objection for the generally idea,
> >> >> > just remaining minor stuffs on the prefix naming scheme,
> >> >> > so I believe that could resolve soon.
> >> >> >
> >> >> > [1] https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17
> >> >>
> >> >> Once the PR is merged and most of the people agree with the rules
> >> >> there, I will say LGTM for the vendor extensions in general.
> >> >>
> >> >> Thanks
> >> >> Nelson
> >> >>
> >> >> > On Fri, Apr 22, 2022 at 6:37 PM Philipp Tomsich
> >> >> > <philipp.tomsich@vrull.eu> wrote:
> >> >> > >
> >> >> > > Nelson,
> >> >> > >
> >> >> > > Can we make some progress on this, please?
> >> >> > > There are already some dependent/similar changes in the queue (e.g.
> >> >> xtheadcmo)...
> >> >> > >
> >> >> > > Philipp.
> >> >> > >
> >> >> > > On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
> >> >> > >>
> >> >> > >> Hi Philipp:
> >> >> > >>
> >> >> > >> I believe we have a consensus among most GNU toolchain maintainers for
> >> >> > >> accepting vendor extension to upstream / master branch,
> >> >> > >> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> >> >> > >> Jim to give something LGTM here since I am not a binutils maintainer
> >> >> > >> :)
> >> >> > >>
> >> >> > >> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> >> >> > >> <philipp.tomsich@vrull.eu> wrote:
> >> >> > >> >
> >> >> > >> > Kito & Nelson,
> >> >> > >> >
> >> >> > >> > What is the status on this one?
> >> >> > >> > Let me know if it is approved for master, so I can rebase and
> >> >> commit.
> >> >> > >> >
> >> >> > >> > Thanks,
> >> >> > >> > Philipp.
> >> >> > >> >
> >> >> > >> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <
> >> >> philipp.tomsich@vrull.eu>
> >> >> > >> > wrote:
> >> >> > >> >
> >> >> > >> > > 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.
> >> >> > >> > >
> >> >> > >> > > ---
> >> >> > >> > >
> >> >> > >> > >  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                  |  7 +++++++
> >> >> > >> > >  include/opcode/riscv.h                      |  1 +
> >> >> > >> > >  opcodes/riscv-opc.c                         |  4 ++++
> >> >> > >> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> >> >> > >> > > --- a/bfd/elfxx-riscv.c
> >> >> > >> > > +++ b/bfd/elfxx-riscv.c
> >> >> > >> > > @@ -1241,6 +1241,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,
> >> >> > >> > > @@ -1248,6 +1255,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
> >> >> > >> > >  };
> >> >> > >> > >
> >> >> > >> > > @@ -1513,8 +1521,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;
> >> >> > >> > >      }
> >> >> > >> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> >> >> > >> > > --- a/include/opcode/riscv-opc.h
> >> >> > >> > > +++ b/include/opcode/riscv-opc.h
> >> >> > >> > > @@ -2029,6 +2029,11 @@
> >> >> > >> > >  #define MASK_HSV_W 0xfe007fff
> >> >> > >> > >  #define MATCH_HSV_D 0x6e004073
> >> >> > >> > >  #define MASK_HSV_D 0xfe007fff
> >> >> > >> > > +/* 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
> >> >> > >> > > @@ -2628,6 +2633,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
> >> >> > >> > >  /* Privileged CSRs.  */
> >> >> > >> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> >> >> > >> > > index 048ab0a5d68..9384c6eb84b 100644
> >> >> > >> > > --- a/include/opcode/riscv.h
> >> >> > >> > > +++ b/include/opcode/riscv.h
> >> >> > >> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> >> >> > >> > >    INSN_CLASS_V,
> >> >> > >> > >    INSN_CLASS_ZVEF,
> >> >> > >> > >    INSN_CLASS_SVINVAL,
> >> >> > >> > > +  INSN_CLASS_XVENTANACONDOPS,
> >> >> > >> > >  };
> >> >> > >> > >
> >> >> > >> > >  /* This structure holds information for a particular
> >> >> instruction.  */
> >> >> > >> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> >> >> > >> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> >> >> > >> > > --- a/opcodes/riscv-opc.c
> >> >> > >> > > +++ b/opcodes/riscv-opc.c
> >> >> > >> > > @@ -1753,6 +1753,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.33.1
> >> >> > >> > >
> >> >> > >> > >
> >> >>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-20 12:42   ` Kito Cheng
  2022-04-22 10:37     ` Philipp Tomsich
@ 2023-04-25 10:48     ` Philipp Tomsich
  2023-04-26 20:34       ` Jeff Law
  1 sibling, 1 reply; 18+ messages in thread
From: Philipp Tomsich @ 2023-04-25 10:48 UTC (permalink / raw)
  To: Kito Cheng
  Cc: Binutils, Kito Cheng, Greg Favor, Palmer Dabbelt, Nelson Chu, Jim Wilson

+Jeff Law (who needs to update his email-address in binutils/MAINTAINERS).

This patch is still waiting for a LGTM (or specific changes requested
during a review).
Given that Ventana's chip is announced, can we close on this now?

Thanks,
Philipp.

On Wed, 20 Apr 2022 at 14:42, Kito Cheng <kito.cheng@gmail.com> wrote:
>
> Hi Philipp:
>
> I believe we have a consensus among most GNU toolchain maintainers for
> accepting vendor extension to upstream / master branch,
> so I am +1 for this patch, but I think we still need Nelson, Palmer or
> Jim to give something LGTM here since I am not a binutils maintainer
> :)
>
> On Wed, Apr 20, 2022 at 7:38 PM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > Kito & Nelson,
> >
> > What is the status on this one?
> > Let me know if it is approved for master, so I can rebase and commit.
> >
> > Thanks,
> > Philipp.
> >
> > On Sun, 9 Jan 2022 at 20:29, Philipp Tomsich <philipp.tomsich@vrull.eu>
> > wrote:
> >
> > > 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.
> > >
> > > ---
> > >
> > >  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                  |  7 +++++++
> > >  include/opcode/riscv.h                      |  1 +
> > >  opcodes/riscv-opc.c                         |  4 ++++
> > >  7 files changed, 59 insertions(+), 2 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 8409c0254e5..39fc1e9911b 100644
> > > --- a/bfd/elfxx-riscv.c
> > > +++ b/bfd/elfxx-riscv.c
> > > @@ -1241,6 +1241,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,
> > > @@ -1248,6 +1255,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
> > >  };
> > >
> > > @@ -1513,8 +1521,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;
> > >      }
> > > @@ -2406,6 +2413,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 be9c1148355..1e24053cbdc 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 0b8cc6c7ddb..07c613163b7 100644
> > > --- a/include/opcode/riscv-opc.h
> > > +++ b/include/opcode/riscv-opc.h
> > > @@ -2029,6 +2029,11 @@
> > >  #define MASK_HSV_W 0xfe007fff
> > >  #define MATCH_HSV_D 0x6e004073
> > >  #define MASK_HSV_D 0xfe007fff
> > > +/* 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
> > > @@ -2628,6 +2633,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
> > >  /* Privileged CSRs.  */
> > > diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> > > index 048ab0a5d68..9384c6eb84b 100644
> > > --- a/include/opcode/riscv.h
> > > +++ b/include/opcode/riscv.h
> > > @@ -388,6 +388,7 @@ enum riscv_insn_class
> > >    INSN_CLASS_V,
> > >    INSN_CLASS_ZVEF,
> > >    INSN_CLASS_SVINVAL,
> > > +  INSN_CLASS_XVENTANACONDOPS,
> > >  };
> > >
> > >  /* This structure holds information for a particular instruction.  */
> > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> > > index 2da0f7cf0a4..6c70c5b99f3 100644
> > > --- a/opcodes/riscv-opc.c
> > > +++ b/opcodes/riscv-opc.c
> > > @@ -1753,6 +1753,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.33.1
> > >
> > >

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2023-04-25 10:48     ` Philipp Tomsich
@ 2023-04-26 20:34       ` Jeff Law
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff Law @ 2023-04-26 20:34 UTC (permalink / raw)
  To: binutils



On 4/25/23 04:48, Philipp Tomsich wrote:
> +Jeff Law (who needs to update his email-address in binutils/MAINTAINERS).
> 
> This patch is still waiting for a LGTM (or specific changes requested
> during a review).
> Given that Ventana's chip is announced, can we close on this now?
I took care of final review & pushing this to the binutils tree.

I'll take care of my address separately :-)

jeff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-20 22:15   ` Jim Wilson
@ 2022-04-20 23:17     ` lkcl
  0 siblings, 0 replies; 18+ messages in thread
From: lkcl @ 2022-04-20 23:17 UTC (permalink / raw)
  To: Jim Wilson; +Cc: binutils



On April 20, 2022 10:15:55 PM UTC, Jim Wilson <jim.wilson.gcc@gmail.com> wrote:
>On Wed, Apr 20, 2022 at 10:55 AM lkcl via Binutils
><binutils@sourceware.org>
>wrote:
>
>> my understanding of custom extensions is that they are intended
>never,
>> under any circumstances, to hit "upstream", due to the risk of
>creating
>> massive conflict and confusion due to dominance of one extension over
>> another, particularly if that custom extension achieves extremely
>> commonly-used and high profile public status.
>>
>
>The RISC-V ISA has always planned for support for custom extensions. 

yes. as isolated non-public usage.  for example, Western Digital's private usage within their SSD, HDD and USB Flash products.  such private usage is non-disruptive to the overall ecosystem.

>We
>have a part of the opcode space reserved for custom extensions,

i know [others on this list may not]. i followed RISC-V development for over 18 months.

>Custom extensions are a fact of life.  All major RISC-V vendors have
>them.
>You are suggesting that each vendor should maintain their own toolchain
>source tree.  But if they all do that, then there is a risk that none
>will
>contribute patches upstream. 

it not a risk that it *doesn't* happen, it's a risk if it *does* happen.

conflicting public usage of the exact same opcodes results in utter destruction of confidence in the entire ISA.  many people including ARM have warned the RISCV community about this.

one binary compiled for one vendor is absolutely impossible to run on another vendor's hardware.

and that starts the moment that any two uses of the exact same opcode become public.  [this was the altivec nightmare of powerpc, 20 years ago].

if they are *private* then that risk of the destruction of public confidence in the entire ISA is mitigated.

>If you don't want custom extensions, it is easy enough to avoid them by
>not
>enabling them in the arch string.  I expect that vendor independent
>linux
>distros will be built without any vendor custom extensions enabled.

mhm.  and what happens when people complain, "these public common vendor independent releases are S***! they're SO SLOW?"

and people investigate, and find that it's because the base RV64GC (common, independent) variant is missing 50% of the opcodes that were added *specifically* by the Alibaba Group to compensate for the lacklustre performance of the standard authorised RV64GC?

https://news.ycombinator.com/item?id=24459041

at that point, to ensure users do not abandon their product in droves, the sheer overwhelming number of user requests will compel distros to add support for the (faster) rogue, unauthorised custom instructions (in direct violation of the Trademark and Compliance Certification), and it quickly goes to hell in a handbasket from there.

to help the RISC-V ecosystem i did explain a way to avoid this mess: i went to a lot of trouble to document and propose it.  the response to those efforts was so shockingly abusive that several people contacted me privately to apologise.

now it is too late, the damage is done. the opportunity to mitigate the public ISA conflict scenario is literally unfolding and cannot be retroactively corrected.  the mechanism to solve this had to have been put in place *before* so many vendors started publicly competing for the same conflicting opcodes, *before* they committed tens to hundreds of millions of dollars in silicon product.

l.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
  2022-04-20 17:55 ` lkcl
@ 2022-04-20 22:15   ` Jim Wilson
  2022-04-20 23:17     ` lkcl
  0 siblings, 1 reply; 18+ messages in thread
From: Jim Wilson @ 2022-04-20 22:15 UTC (permalink / raw)
  To: lkcl; +Cc: binutils

On Wed, Apr 20, 2022 at 10:55 AM lkcl via Binutils <binutils@sourceware.org>
wrote:

> my understanding of custom extensions is that they are intended never,
> under any circumstances, to hit "upstream", due to the risk of creating
> massive conflict and confusion due to dominance of one extension over
> another, particularly if that custom extension achieves extremely
> commonly-used and high profile public status.
>

The RISC-V ISA has always planned for support for custom extensions.  We
have a part of the opcode space reserved for custom extensions, and we have
an arch string syntax for specifying custom extensions.   We are also
trying to standardize the naming conventions for custom extensions in
riscv-toolchain-conventions to avoid conflict.  See pull requests #17 and
#19.

Custom extensions are a fact of life.  All major RISC-V vendors have them.
You are suggesting that each vendor should maintain their own toolchain
source tree.  But if they all do that, then there is a risk that none will
contribute patches upstream.  We are better off if we allow vendor custom
extensions upstream, and that will encourage vendors to work upstream.  We
already have support for SiFive custom extensions on
the users/riscv/binutils-integration-branch.  The plan was to move that to
the master branch after the last release, but Nelson has been busy and
hasn't gotten around to it yet.  Ventana custom extensions are just more of
the same.  Hopefully we can also get Alibaba/T-Head and others to add their
custom extensions upstream too.

If you don't want custom extensions, it is easy enough to avoid them by not
enabling them in the arch string.  I expect that vendor independent linux
distros will be built without any vendor custom extensions enabled.

Jim

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
       [not found] <mailman.26390.1650459569.2100866.binutils@sourceware.org>
@ 2022-04-20 17:55 ` lkcl
  2022-04-20 22:15   ` Jim Wilson
  0 siblings, 1 reply; 18+ messages in thread
From: lkcl @ 2022-04-20 17:55 UTC (permalink / raw)
  To: binutils, binutils-request

sorry this is out of replyto because i am subscribed digest.

> Subject:	Re: [PATCH v1] RISC-V: Support XVentanaCondOps extension
> Hi Philipp:

> I believe we have a consensus among most GNU toolchain maintainers for
> accepting vendor extension to upstream / master branch,

my understanding of custom extensions is that they are intended never, under any circumstances, to hit "upstream", due to the risk of creating massive conflict and confusion due to dominance of one extension over another, particularly if that custom extension achieves extremely commonly-used and high profile public status.

[a dynamic plugin system on the other hand is a completely different matter]

i warned the RISC-V Foundation about this scenario.  they ignored my warnings.  two years later as a recent article online outlines, the fragmentation has already occurred because this 3rd category (abuse of custom extension opcode space for *high profile* common public usage) has, as i warned would happen, in fact occurred.

custom extensions were *supposed* to be hard-forks of the entire toolchain, maintained by a proprietary vendor, at their cost and expense, for their benefit and their benefit only.

[a dynamic plugin system helps such proprietary vendors to reduce costs of maintaining such private hard forks]

if you accept proprietary (rogue, unauthorized) custom plugins into the toolchain you risk destruction of the entire ecosystem for everyone.

[by complete contrast a dynamic plugin system helps keep such rogue extensions "at bay", crucially *without* giving the false and misleading impression that the extensions have been "authorised" through upstream acceptance]

if unfamiliar with this scenario recall the nightmare opcode conflict of altivec for powerpc, of 20 years ago.  ask around.

l.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-04-26 20:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-09 19:29 [PATCH v1] RISC-V: Support XVentanaCondOps extension Philipp Tomsich
2022-01-17 12:57 ` Philipp Tomsich
2022-04-20 11:37 ` Philipp Tomsich
2022-04-20 12:42   ` Kito Cheng
2022-04-22 10:37     ` Philipp Tomsich
2022-04-23  1:42       ` Kito Cheng
2022-04-25  9:38         ` Nelson Chu
2022-05-12 19:59           ` Philipp Tomsich
2022-05-12 22:23             ` Palmer Dabbelt
2022-05-13  7:55               ` Philipp Tomsich
2022-05-26 19:50               ` Philipp Tomsich
2022-05-30 19:33                 ` Palmer Dabbelt
2022-05-30 20:09                   ` Philipp Tomsich
2023-04-25 10:48     ` Philipp Tomsich
2023-04-26 20:34       ` Jeff Law
     [not found] <mailman.26390.1650459569.2100866.binutils@sourceware.org>
2022-04-20 17:55 ` lkcl
2022-04-20 22:15   ` Jim Wilson
2022-04-20 23:17     ` lkcl

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).