[PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Philipp Tomsich]

This series provides support for the Ventana VT1 (a 4-way superscalar
rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
for the supported instruction fusion patterns.

This includes the addition of the fusion-aware scheduling
infrastructure for RISC-V and implements idiom recognition for the
fusion patterns supported by VT1.

Note that we don't signal support for XVentanaCondOps at this point,
as the XVentanaCondOps support is in-flight separately.  Changing the
defaults for VT1 can happen late in the cycle, so no need to link the
two different changesets.

Changes in v2:
- Rebased and changed over to .rst-based documentation
- Updated to catch more fusion cases
- Signals support for Zifencei

Philipp Tomsich (2):
  RISC-V: Add basic support for the Ventana-VT1 core
  RISC-V: Add instruction fusion (for ventana-vt1)

 gcc/config/riscv/riscv-cores.def              |   3 +
 gcc/config/riscv/riscv-opts.h                 |   2 +-
 gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
 .../risc-v-options.rst                        |   5 +-
 4 files changed, 240 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] RISC-V: Add basic support for the Ventana-VT1 core

[Philipp Tomsich]

The Ventana-VT1 core is compatible with rv64gc, Zb[abcs], Zifenci and
XVentanaCondOps.
This introduces a placeholder -mcpu=ventana-vt1, so tooling and
scripts don't need to change once full support (pipeline, tuning,
etc.) will become public later.

gcc/ChangeLog:

	* config/riscv/riscv-cores.def (RISCV_TUNE): Add ventana-vt1.
	(RISCV_CORE): Ditto.
	* config/riscv/riscv-opts.h (enum riscv_microarchitecture_type): Ditto.
	* config/riscv/riscv.cc: Add tune_info for ventana-vt1.
	* config/riscv/riscv.md: Add ventana-vt1.
	* doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst:
	Document -mcpu= and -mtune with ventana-vt1.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

Changes in v2:
- Rebased and changed over to .rst-based documentation
- Updated to catch more fusion cases
- Signals support for Zifencei
- Rebase to master, adjusting for the new way to define cores.
- Change documentation to the new way (.rst)
- Include Zifencei in the VT1 definition.

 gcc/config/riscv/riscv-cores.def                   |  3 +++
 gcc/config/riscv/riscv-opts.h                      |  2 +-
 gcc/config/riscv/riscv.cc                          | 14 ++++++++++++++
 .../machine-dependent-options/risc-v-options.rst   |  5 +++--
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/gcc/config/riscv/riscv-cores.def b/gcc/config/riscv/riscv-cores.def
index 31ad34682c5..aef1e92ae24 100644
--- a/gcc/config/riscv/riscv-cores.def
+++ b/gcc/config/riscv/riscv-cores.def
@@ -38,6 +38,7 @@ RISCV_TUNE("sifive-3-series", generic, rocket_tune_info)
 RISCV_TUNE("sifive-5-series", generic, rocket_tune_info)
 RISCV_TUNE("sifive-7-series", sifive_7, sifive_7_tune_info)
 RISCV_TUNE("thead-c906", generic, thead_c906_tune_info)
+RISCV_TUNE("ventana-vt1", generic, ventana_vt1_tune_info)
 RISCV_TUNE("size", generic, optimize_size_tune_info)
 
 #undef RISCV_TUNE
@@ -73,4 +74,6 @@ RISCV_CORE("sifive-s76",      "rv64imafdc", "sifive-7-series")
 RISCV_CORE("sifive-u54",      "rv64imafdc", "sifive-5-series")
 RISCV_CORE("sifive-u74",      "rv64imafdc", "sifive-7-series")
 
+RISCV_CORE("ventana-vt1",     "rv64imafdc_zba_zbb_zbc_zbs_zifencei",	"ventana-vt1")
+
 #undef RISCV_CORE
diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index 84c987626bc..7962dbe5018 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -52,7 +52,7 @@ extern enum riscv_isa_spec_class riscv_isa_spec;
 /* Keep this list in sync with define_attr "tune" in riscv.md.  */
 enum riscv_microarchitecture_type {
   generic,
-  sifive_7
+  sifive_7,
 };
 extern enum riscv_microarchitecture_type riscv_microarchitecture;
 
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index c04e5db21df..31d651f8744 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -360,6 +360,20 @@ static const struct riscv_tune_param optimize_size_tune_info = {
   false,					/* slow_unaligned_access */
 };
 
+/* Costs to use when optimizing for Ventana Micro VT1.  */
+static const struct riscv_tune_param ventana_vt1_tune_info = {
+  {COSTS_N_INSNS (4), COSTS_N_INSNS (5)},	/* fp_add */
+  {COSTS_N_INSNS (4), COSTS_N_INSNS (5)},	/* fp_mul */
+  {COSTS_N_INSNS (20), COSTS_N_INSNS (20)},	/* fp_div */
+  {COSTS_N_INSNS (4), COSTS_N_INSNS (4)},	/* int_mul */
+  {COSTS_N_INSNS (6), COSTS_N_INSNS (6)},	/* int_div */
+  4,						/* issue_rate */
+  4,						/* branch_cost */
+  5,						/* memory_cost */
+  8,						/* fmv_cost */
+  false,					/* slow_unaligned_access */
+};
+
 static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
 static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
 
diff --git a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst
index 2b5167b56b2..5a0345ae2b3 100644
--- a/gcc/doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst
+++ b/gcc/doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst
@@ -95,14 +95,15 @@ These command-line options are defined for RISC-V targets:
   Permissible values for this option are: :samp:`sifive-e20`, :samp:`sifive-e21`,
   :samp:`sifive-e24`, :samp:`sifive-e31`, :samp:`sifive-e34`, :samp:`sifive-e76`,
   :samp:`sifive-s21`, :samp:`sifive-s51`, :samp:`sifive-s54`, :samp:`sifive-s76`,
-  :samp:`sifive-u54`, and :samp:`sifive-u74`.
+  :samp:`sifive-u54`, :samp:`sifive-u74`, and :samp:`ventana-vt1`.
 
 .. option:: -mtune={processor-string}
 
   Optimize the output for the given processor, specified by microarchitecture or
   particular CPU name.  Permissible values for this option are: :samp:`rocket`,
   :samp:`sifive-3-series`, :samp:`sifive-5-series`, :samp:`sifive-7-series`,
-  :samp:`thead-c906`, :samp:`size`, and all valid options for :option:`-mcpu=`.
+  :samp:`thead-c906`, :samp:`ventana-vt1`, and :samp:`size`, and all valid
+  options for :option:`-mcpu=`.
 
   When :option:`-mtune=` is not specified, use the setting from :option:`-mcpu`,
   the default is :samp:`rocket` if both are not specified.
-- 
2.34.1

Re: [PATCH v2 1/2] RISC-V: Add basic support for the Ventana-VT1 core

[Jeff Law]

On 11/13/22 13:48, Philipp Tomsich wrote:
> The Ventana-VT1 core is compatible with rv64gc, Zb[abcs], Zifenci and
> XVentanaCondOps.
> This introduces a placeholder -mcpu=ventana-vt1, so tooling and
> scripts don't need to change once full support (pipeline, tuning,
> etc.) will become public later.
>
> gcc/ChangeLog:
>
> 	* config/riscv/riscv-cores.def (RISCV_TUNE): Add ventana-vt1.
> 	(RISCV_CORE): Ditto.
> 	* config/riscv/riscv-opts.h (enum riscv_microarchitecture_type): Ditto.
> 	* config/riscv/riscv.cc: Add tune_info for ventana-vt1.
> 	* config/riscv/riscv.md: Add ventana-vt1.
> 	* doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst:
> 	Document -mcpu= and -mtune with ventana-vt1.

OK.


WRT the scheduler description.  I have one, but I think it's on the 
server at the vacation house which went offline a couple weeks ago and 
due to health reasons I haven't been up there to reset the internet 
connection.  Worst case I can just rebuild it from scratch, it's not 
that complex.

Jeff

Re: [PATCH v2 1/2] RISC-V: Add basic support for the Ventana-VT1 core

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 16:52, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/13/22 13:48, Philipp Tomsich wrote:
> > The Ventana-VT1 core is compatible with rv64gc, Zb[abcs], Zifenci and
> > XVentanaCondOps.
> > This introduces a placeholder -mcpu=ventana-vt1, so tooling and
> > scripts don't need to change once full support (pipeline, tuning,
> > etc.) will become public later.
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/riscv-cores.def (RISCV_TUNE): Add ventana-vt1.
> >       (RISCV_CORE): Ditto.
> >       * config/riscv/riscv-opts.h (enum riscv_microarchitecture_type): Ditto.
> >       * config/riscv/riscv.cc: Add tune_info for ventana-vt1.
> >       * config/riscv/riscv.md: Add ventana-vt1.
> >       * doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst:
> >       Document -mcpu= and -mtune with ventana-vt1.
>
> OK.
>
>
> WRT the scheduler description.  I have one, but I think it's on the
> server at the vacation house which went offline a couple weeks ago and
> due to health reasons I haven't been up there to reset the internet
> connection.  Worst case I can just rebuild it from scratch, it's not
> that complex.

This series is pointing 'ventana-vt1' back to 'generic', so we could
also add the pipeline description later in the release cycle...

Philipp.

Re: [PATCH v2 1/2] RISC-V: Add basic support for the Ventana-VT1 core

[Philipp Tomsich]

Applied to master. Thanks!

Philipp.

On Mon, 14 Nov 2022 at 16:52, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/13/22 13:48, Philipp Tomsich wrote:
> > The Ventana-VT1 core is compatible with rv64gc, Zb[abcs], Zifenci and
> > XVentanaCondOps.
> > This introduces a placeholder -mcpu=ventana-vt1, so tooling and
> > scripts don't need to change once full support (pipeline, tuning,
> > etc.) will become public later.
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/riscv-cores.def (RISCV_TUNE): Add ventana-vt1.
> >       (RISCV_CORE): Ditto.
> >       * config/riscv/riscv-opts.h (enum riscv_microarchitecture_type): Ditto.
> >       * config/riscv/riscv.cc: Add tune_info for ventana-vt1.
> >       * config/riscv/riscv.md: Add ventana-vt1.
> >       * doc/gcc/gcc-command-options/machine-dependent-options/risc-v-options.rst:
> >       Document -mcpu= and -mtune with ventana-vt1.
>
> OK.
>
>
> WRT the scheduler description.  I have one, but I think it's on the
> server at the vacation house which went offline a couple weeks ago and
> due to health reasons I haven't been up there to reset the internet
> connection.  Worst case I can just rebuild it from scratch, it's not
> that complex.
>
> Jeff

[PATCH v2 2/2] RISC-V: Add instruction fusion (for ventana-vt1)

[Philipp Tomsich]

The Ventana VT1 core supports quad-issue and instruction fusion.
This implemented TARGET_SCHED_MACRO_FUSION_P to keep fusible sequences
together and adds idiom matcheing for the supported fusion cases.

gcc/ChangeLog:

	* config/riscv/riscv.cc (enum riscv_fusion_pairs): Add symbolic
	constants to identify supported fusion patterns.
	(struct riscv_tune_param): Add fusible_op field.
	(riscv_macro_fusion_p): Implement.
	(riscv_fusion_enabled_p): Implement.
	(riscv_macro_fusion_pair_p): Implement and recoginze fusible
	idioms for Ventana VT1.
	(TARGET_SCHED_MACRO_FUSION_P): Point to riscv_macro_fusion_p.
	(TARGET_SCHED_MACRO_FUSION_PAIR_P): Point to riscv_macro_fusion_pair_p.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

Changes in v2:
- Update fusion patterns and catch some missing idioms/fusion pairs.

 gcc/config/riscv/riscv.cc | 219 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 219 insertions(+)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 31d651f8744..43ba520885c 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -215,6 +215,19 @@ struct riscv_integer_op {
    The worst case is LUI, ADDI, SLLI, ADDI, SLLI, ADDI, SLLI, ADDI.  */
 #define RISCV_MAX_INTEGER_OPS 8
 
+enum riscv_fusion_pairs
+{
+  RISCV_FUSE_NOTHING = 0,
+  RISCV_FUSE_ZEXTW = (1 << 0),
+  RISCV_FUSE_ZEXTH = (1 << 1),
+  RISCV_FUSE_ZEXTWS = (1 << 2),
+  RISCV_FUSE_LDINDEXED = (1 << 3),
+  RISCV_FUSE_LUI_ADDI = (1 << 4),
+  RISCV_FUSE_AUIPC_ADDI = (1 << 5),
+  RISCV_FUSE_LUI_LD = (1 << 6),
+  RISCV_FUSE_AUIPC_LD = (1 << 7),
+};
+
 /* Costs of various operations on the different architectures.  */
 
 struct riscv_tune_param
@@ -229,6 +242,7 @@ struct riscv_tune_param
   unsigned short memory_cost;
   unsigned short fmv_cost;
   bool slow_unaligned_access;
+  unsigned int fusible_ops;
 };
 
 /* Information about one micro-arch we know about.  */
@@ -316,6 +330,7 @@ static const struct riscv_tune_param rocket_tune_info = {
   5,						/* memory_cost */
   8,						/* fmv_cost */
   true,						/* slow_unaligned_access */
+  RISCV_FUSE_NOTHING,                           /* fusible_ops */
 };
 
 /* Costs to use when optimizing for Sifive 7 Series.  */
@@ -330,6 +345,7 @@ static const struct riscv_tune_param sifive_7_tune_info = {
   3,						/* memory_cost */
   8,						/* fmv_cost */
   true,						/* slow_unaligned_access */
+  RISCV_FUSE_NOTHING,                           /* fusible_ops */
 };
 
 /* Costs to use when optimizing for T-HEAD c906.  */
@@ -344,6 +360,7 @@ static const struct riscv_tune_param thead_c906_tune_info = {
   5,            /* memory_cost */
   8,		/* fmv_cost */
   false,            /* slow_unaligned_access */
+  RISCV_FUSE_NOTHING,                           /* fusible_ops */
 };
 
 /* Costs to use when optimizing for size.  */
@@ -358,6 +375,7 @@ static const struct riscv_tune_param optimize_size_tune_info = {
   2,						/* memory_cost */
   8,						/* fmv_cost */
   false,					/* slow_unaligned_access */
+  RISCV_FUSE_NOTHING,                           /* fusible_ops */
 };
 
 /* Costs to use when optimizing for Ventana Micro VT1.  */
@@ -372,6 +390,10 @@ static const struct riscv_tune_param ventana_vt1_tune_info = {
   5,						/* memory_cost */
   8,						/* fmv_cost */
   false,					/* slow_unaligned_access */
+  ( RISCV_FUSE_ZEXTW | RISCV_FUSE_ZEXTH |       /* fusible_ops */
+    RISCV_FUSE_ZEXTWS | RISCV_FUSE_LDINDEXED |
+    RISCV_FUSE_LUI_ADDI | RISCV_FUSE_AUIPC_ADDI |
+    RISCV_FUSE_LUI_LD | RISCV_FUSE_AUIPC_LD )
 };
 
 static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
@@ -5627,6 +5649,199 @@ riscv_issue_rate (void)
   return tune_param->issue_rate;
 }
 
+/* Implement TARGET_SCHED_MACRO_FUSION_P.  Return true if target supports
+   instruction fusion of some sort.  */
+
+static bool
+riscv_macro_fusion_p (void)
+{
+  return tune_param->fusible_ops != RISCV_FUSE_NOTHING;
+}
+
+/* Return true iff the instruction fusion described by OP is enabled.  */
+
+static bool
+riscv_fusion_enabled_p(enum riscv_fusion_pairs op)
+{
+  return tune_param->fusible_ops & op;
+}
+
+/* Implement TARGET_SCHED_MACRO_FUSION_PAIR_P.  Return true if PREV and CURR
+   should be kept together during scheduling.  */
+
+static bool
+riscv_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
+{
+  rtx prev_set = single_set (prev);
+  rtx curr_set = single_set (curr);
+  /* prev and curr are simple SET insns i.e. no flag setting or branching.  */
+  bool simple_sets_p = prev_set && curr_set && !any_condjump_p (curr);
+
+  if (!riscv_macro_fusion_p ())
+    return false;
+
+  if (simple_sets_p && (riscv_fusion_enabled_p (RISCV_FUSE_ZEXTW) ||
+			riscv_fusion_enabled_p (RISCV_FUSE_ZEXTH)))
+    {
+      /* We are trying to match the following:
+	   prev (slli) == (set (reg:DI rD)
+			       (ashift:DI (reg:DI rS) (const_int 32)))
+	   curr (slri) == (set (reg:DI rD)
+			       (lshiftrt:DI (reg:DI rD) (const_int <shift>)))
+	 with <shift> being either 32 for FUSE_ZEXTW, or
+			 `less than 32 for FUSE_ZEXTWS. */
+
+      if (GET_CODE (SET_SRC (prev_set)) == ASHIFT
+	  && GET_CODE (SET_SRC (curr_set)) == LSHIFTRT
+	  && REG_P (SET_DEST (prev_set))
+	  && REG_P (SET_DEST (curr_set))
+	  && REGNO (SET_DEST (prev_set)) == REGNO (SET_DEST (curr_set))
+	  && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO(SET_DEST (curr_set))
+	  && CONST_INT_P (XEXP (SET_SRC (prev_set), 1))
+	  && CONST_INT_P (XEXP (SET_SRC (curr_set), 1))
+	  && INTVAL (XEXP (SET_SRC (prev_set), 1)) == 32
+	  && (( INTVAL (XEXP (SET_SRC (curr_set), 1)) == 32
+		&& riscv_fusion_enabled_p(RISCV_FUSE_ZEXTW) )
+	      || ( INTVAL (XEXP (SET_SRC (curr_set), 1)) < 32
+		   && riscv_fusion_enabled_p(RISCV_FUSE_ZEXTWS))))
+	return true;
+    }
+
+  if (simple_sets_p && riscv_fusion_enabled_p (RISCV_FUSE_ZEXTH))
+    {
+      /* We are trying to match the following:
+	   prev (slli) == (set (reg:DI rD)
+			       (ashift:DI (reg:DI rS) (const_int 48)))
+	   curr (slri) == (set (reg:DI rD)
+			       (lshiftrt:DI (reg:DI rD) (const_int 48))) */
+
+      if (GET_CODE (SET_SRC (prev_set)) == ASHIFT
+	  && GET_CODE (SET_SRC (curr_set)) == LSHIFTRT
+	  && REG_P (SET_DEST (prev_set))
+	  && REG_P (SET_DEST (curr_set))
+	  && REGNO (SET_DEST (prev_set)) == REGNO (SET_DEST (curr_set))
+	  && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO(SET_DEST (curr_set))
+	  && CONST_INT_P (XEXP (SET_SRC (prev_set), 1))
+	  && CONST_INT_P (XEXP (SET_SRC (curr_set), 1))
+	  && INTVAL (XEXP (SET_SRC (prev_set), 1)) == 48
+	  && INTVAL (XEXP (SET_SRC (curr_set), 1)) == 48)
+	return true;
+    }
+
+  if (simple_sets_p && riscv_fusion_enabled_p (RISCV_FUSE_LDINDEXED))
+    {
+      /* We are trying to match the following:
+	   prev (add) == (set (reg:DI rD)
+			      (plus:DI (reg:DI rS1) (reg:DI rS2))
+	   curr (ld)  == (set (reg:DI rD)
+			      (mem:DI (reg:DI rD))) */
+
+      if (MEM_P (SET_SRC (curr_set))
+	  && REG_P (XEXP (SET_SRC (curr_set), 0))
+	  && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO (SET_DEST (prev_set))
+	  && GET_CODE (SET_SRC (prev_set)) == PLUS
+	  && REG_P (XEXP (SET_SRC (prev_set), 0))
+	  && REG_P (XEXP (SET_SRC (prev_set), 1)))
+	return true;
+
+      /* We are trying to match the following:
+	   prev (add) == (set (reg:DI rD)
+			      (plus:DI (reg:DI rS1) (reg:DI rS2)))
+	   curr (lw)  == (set (any_extend:DI (mem:SUBX (reg:DI rD)))) */
+
+      if ((GET_CODE (SET_SRC (curr_set)) == SIGN_EXTEND
+	   || (GET_CODE (SET_SRC (curr_set)) == ZERO_EXTEND))
+	  && MEM_P (XEXP (SET_SRC (curr_set), 0))
+	  && REG_P (XEXP (XEXP (SET_SRC (curr_set), 0), 0))
+	  && REGNO (XEXP (XEXP (SET_SRC (curr_set), 0), 0)) == REGNO (SET_DEST (prev_set))
+	  && GET_CODE (SET_SRC (prev_set)) == PLUS
+	  && REG_P (XEXP (SET_SRC (prev_set), 0))
+	  && REG_P (XEXP (SET_SRC (prev_set), 1)))
+	return true;
+    }
+
+  if (simple_sets_p && riscv_fusion_enabled_p (RISCV_FUSE_LUI_ADDI))
+    {
+      /* We are trying to match the following:
+	   prev (lui)  == (set (reg:DI rD) (const_int UPPER_IMM_20))
+	   curr (addi) == (set (reg:DI rD)
+			       (plus:DI (reg:DI rD) (const_int IMM12))) */
+
+      if ((GET_CODE (SET_SRC (curr_set)) == LO_SUM
+	   || (GET_CODE (SET_SRC (curr_set)) == PLUS
+	       && CONST_INT_P (XEXP (SET_SRC (curr_set), 1))
+	       && SMALL_OPERAND (INTVAL (XEXP (SET_SRC (curr_set), 1)))))
+	  && (GET_CODE (SET_SRC (prev_set)) == HIGH
+	      || (CONST_INT_P (SET_SRC (prev_set))
+		  && LUI_OPERAND (INTVAL (SET_SRC (prev_set))))))
+	return true;
+    }
+
+  if (simple_sets_p && riscv_fusion_enabled_p (RISCV_FUSE_AUIPC_ADDI))
+    {
+      /* We are trying to match the following:
+	   prev (auipc) == (set (reg:DI rD) (unspec:DI [...] UNSPEC_AUIPC))
+	   curr (addi)  == (set (reg:DI rD)
+				(plus:DI (reg:DI rD) (const_int IMM12)))
+	 and
+	   prev (auipc) == (set (reg:DI rD) (unspec:DI [...] UNSPEC_AUIPC))
+	   curr (addi)  == (set (reg:DI rD)
+				(lo_sum:DI (reg:DI rD) (const_int IMM12))) */
+
+      if (GET_CODE (SET_SRC (prev_set)) == UNSPEC
+	  && XINT (prev_set, 1) == UNSPEC_AUIPC
+	  && (GET_CODE (SET_SRC (curr_set)) == LO_SUM
+	      || (GET_CODE (SET_SRC (curr_set)) == PLUS
+		  && SMALL_OPERAND (INTVAL (XEXP (SET_SRC (curr_set), 1))))))
+
+	return true;
+    }
+
+  if (simple_sets_p && riscv_fusion_enabled_p (RISCV_FUSE_LUI_LD))
+    {
+      /* We are trying to match the following:
+	   prev (lui)  == (set (reg:DI rD) (const_int UPPER_IMM_20))
+	   curr (ld)  == (set (reg:DI rD)
+			      (mem:DI (plus:DI (reg:DI rD) (const_int IMM12)))) */
+
+      if (CONST_INT_P (SET_SRC (prev_set))
+	  && LUI_OPERAND (INTVAL (SET_SRC (prev_set)))
+	  && MEM_P (SET_SRC (curr_set))
+	  && GET_CODE (XEXP (SET_SRC (curr_set), 0)) == PLUS)
+	return true;
+
+      if (GET_CODE (SET_SRC (prev_set)) == HIGH
+	  && MEM_P (SET_SRC (curr_set))
+	  && GET_CODE (XEXP (SET_SRC (curr_set), 0)) == LO_SUM
+	  && REGNO (SET_DEST (prev_set)) == REGNO (XEXP (XEXP (SET_SRC (curr_set), 0), 0)))
+	return true;
+
+      if (GET_CODE (SET_SRC (prev_set)) == HIGH
+	  && (GET_CODE (SET_SRC (curr_set)) == SIGN_EXTEND
+	      || GET_CODE (SET_SRC (curr_set)) == ZERO_EXTEND)
+	  && MEM_P (XEXP (SET_SRC (curr_set), 0))
+	  && (GET_CODE (XEXP (XEXP (SET_SRC (curr_set), 0), 0)) == LO_SUM
+	      && REGNO (SET_DEST (prev_set)) == REGNO (XEXP (XEXP (XEXP (SET_SRC (curr_set), 0), 0), 0))))
+	return true;
+    }
+
+  if (simple_sets_p && riscv_fusion_enabled_p (RISCV_FUSE_AUIPC_LD))
+    {
+      /* We are trying to match the following:
+	   prev (auipc) == (set (reg:DI rD) (unspec:DI [...] UNSPEC_AUIPC))
+	   curr (ld)  == (set (reg:DI rD)
+			      (mem:DI (plus:DI (reg:DI rD) (const_int IMM12)))) */
+
+      if (GET_CODE (SET_SRC (prev_set)) == UNSPEC
+	  && XINT (prev_set, 1) == UNSPEC_AUIPC
+	  && MEM_P (SET_SRC (curr_set))
+	  && GET_CODE (XEXP (SET_SRC (curr_set), 0)) == PLUS)
+	return true;
+    }
+
+  return false;
+}
+
 /* Auxiliary function to emit RISC-V ELF attribute. */
 static void
 riscv_emit_attribute ()
@@ -6657,6 +6872,10 @@ riscv_shamt_matches_mask_p (int shamt, HOST_WIDE_INT mask)
 
 #undef TARGET_SCHED_ISSUE_RATE
 #define TARGET_SCHED_ISSUE_RATE riscv_issue_rate
+#undef TARGET_SCHED_MACRO_FUSION_P
+#define TARGET_SCHED_MACRO_FUSION_P riscv_macro_fusion_p
+#undef TARGET_SCHED_MACRO_FUSION_PAIR_P
+#define TARGET_SCHED_MACRO_FUSION_PAIR_P riscv_macro_fusion_pair_p
 
 #undef TARGET_FUNCTION_OK_FOR_SIBCALL
 #define TARGET_FUNCTION_OK_FOR_SIBCALL riscv_function_ok_for_sibcall
-- 
2.34.1

Re: [PATCH v2 2/2] RISC-V: Add instruction fusion (for ventana-vt1)

[Jeff Law]

On 11/13/22 13:48, Philipp Tomsich wrote:
> The Ventana VT1 core supports quad-issue and instruction fusion.
> This implemented TARGET_SCHED_MACRO_FUSION_P to keep fusible sequences
> together and adds idiom matcheing for the supported fusion cases.
>
> gcc/ChangeLog:
>
> 	* config/riscv/riscv.cc (enum riscv_fusion_pairs): Add symbolic
> 	constants to identify supported fusion patterns.
> 	(struct riscv_tune_param): Add fusible_op field.
> 	(riscv_macro_fusion_p): Implement.
> 	(riscv_fusion_enabled_p): Implement.
> 	(riscv_macro_fusion_pair_p): Implement and recoginze fusible
> 	idioms for Ventana VT1.
> 	(TARGET_SCHED_MACRO_FUSION_P): Point to riscv_macro_fusion_p.
> 	(TARGET_SCHED_MACRO_FUSION_PAIR_P): Point to riscv_macro_fusion_pair_p.

You know the fusion rules for VT1 better than I...  I'm happy to largely 
defer to you on this.

I do wonder if going forward hand matching RTL like this is going to be 
an unmaintainable mess and whether or not we would be better served 
using insn attributes to describe instruction fusion.



>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>
> Changes in v2:
> - Update fusion patterns and catch some missing idioms/fusion pairs.
>
>   gcc/config/riscv/riscv.cc | 219 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 219 insertions(+)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 31d651f8744..43ba520885c 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
>
> +static bool
> +riscv_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
> +{
> +  rtx prev_set = single_set (prev);
> +  rtx curr_set = single_set (curr);
> +  /* prev and curr are simple SET insns i.e. no flag setting or branching.  */
> +  bool simple_sets_p = prev_set && curr_set && !any_condjump_p (curr);
> +
> +  if (!riscv_macro_fusion_p ())
> +    return false;
> +
> +  if (simple_sets_p && (riscv_fusion_enabled_p (RISCV_FUSE_ZEXTW) ||
> +			riscv_fusion_enabled_p (RISCV_FUSE_ZEXTH)))

Formatting nit.  Bring the && down to a new line and if you still need a 
line break for the "||",  then the "||" should be on a new line as 
well.  Something like this...


if (simple_sets_p
       && (riscv_fusion_enabled_p (RISCV_FUSE_ZEXTW

           || riscv_fusion_enabled_p (RISCV_FUSE_ZEXTH)))


> +	  && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO(SET_DEST (curr_set))

Space before open paren on this line.


>
> +	  && (( INTVAL (XEXP (SET_SRC (curr_set), 1)) == 32
> +		&& riscv_fusion_enabled_p(RISCV_FUSE_ZEXTW) )
> +	      || ( INTVAL (XEXP (SET_SRC (curr_set), 1)) < 32
> +		   && riscv_fusion_enabled_p(RISCV_FUSE_ZEXTWS))))

Extraneous spaces after the open parens before INTVALs above.


> +	  && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO(SET_DEST (curr_set))

Missing whitespace before open paren on this line.


OK with the nits fixed.


Jeff

Re: [PATCH v2 2/2] RISC-V: Add instruction fusion (for ventana-vt1)

[Jakub Jelinek]

On Mon, Nov 14, 2022 at 09:06:10AM -0700, Jeff Law via Gcc-patches wrote:
> 
> On 11/13/22 13:48, Philipp Tomsich wrote:
> > The Ventana VT1 core supports quad-issue and instruction fusion.
> > This implemented TARGET_SCHED_MACRO_FUSION_P to keep fusible sequences
> > together and adds idiom matcheing for the supported fusion cases.
> > 
> > gcc/ChangeLog:
> > 
> > 	* config/riscv/riscv.cc (enum riscv_fusion_pairs): Add symbolic
> > 	constants to identify supported fusion patterns.
> > 	(struct riscv_tune_param): Add fusible_op field.
> > 	(riscv_macro_fusion_p): Implement.
> > 	(riscv_fusion_enabled_p): Implement.
> > 	(riscv_macro_fusion_pair_p): Implement and recoginze fusible

s/recoginze/recognize/

> > 	idioms for Ventana VT1.
> > 	(TARGET_SCHED_MACRO_FUSION_P): Point to riscv_macro_fusion_p.
> > 	(TARGET_SCHED_MACRO_FUSION_PAIR_P): Point to riscv_macro_fusion_pair_p.
> 
> You know the fusion rules for VT1 better than I...  I'm happy to largely
> defer to you on this.
> 
> I do wonder if going forward hand matching RTL like this is going to be an
> unmaintainable mess and whether or not we would be better served using insn
> attributes to describe instruction fusion.

	Jakub

Re: [PATCH v2 2/2] RISC-V: Add instruction fusion (for ventana-vt1)

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 17:06, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/13/22 13:48, Philipp Tomsich wrote:
> > The Ventana VT1 core supports quad-issue and instruction fusion.
> > This implemented TARGET_SCHED_MACRO_FUSION_P to keep fusible sequences
> > together and adds idiom matcheing for the supported fusion cases.
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/riscv.cc (enum riscv_fusion_pairs): Add symbolic
> >       constants to identify supported fusion patterns.
> >       (struct riscv_tune_param): Add fusible_op field.
> >       (riscv_macro_fusion_p): Implement.
> >       (riscv_fusion_enabled_p): Implement.
> >       (riscv_macro_fusion_pair_p): Implement and recoginze fusible
> >       idioms for Ventana VT1.
> >       (TARGET_SCHED_MACRO_FUSION_P): Point to riscv_macro_fusion_p.
> >       (TARGET_SCHED_MACRO_FUSION_PAIR_P): Point to riscv_macro_fusion_pair_p.
>
> You know the fusion rules for VT1 better than I...  I'm happy to largely
> defer to you on this.
>
> I do wonder if going forward hand matching RTL like this is going to be
> an unmaintainable mess and whether or not we would be better served
> using insn attributes to describe instruction fusion.

I had thought about that, too.
In the end our team decided to stay away from it for the time being:
fusion frequently needs to look at second-level properties and whether
the first instruction's output register is overwritten by the second
instruction.  So we kept with the same stereotype of idiom-matching
that is also used for AArch64 today.

That said, both the RISC-V and the AArch64 implementations of this are
on my list of things to refactor in a quiet hour.

>
>
>
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > ---
> >
> > Changes in v2:
> > - Update fusion patterns and catch some missing idioms/fusion pairs.
> >
> >   gcc/config/riscv/riscv.cc | 219 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 219 insertions(+)
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 31d651f8744..43ba520885c 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> >
> > +static bool
> > +riscv_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
> > +{
> > +  rtx prev_set = single_set (prev);
> > +  rtx curr_set = single_set (curr);
> > +  /* prev and curr are simple SET insns i.e. no flag setting or branching.  */
> > +  bool simple_sets_p = prev_set && curr_set && !any_condjump_p (curr);
> > +
> > +  if (!riscv_macro_fusion_p ())
> > +    return false;
> > +
> > +  if (simple_sets_p && (riscv_fusion_enabled_p (RISCV_FUSE_ZEXTW) ||
> > +                     riscv_fusion_enabled_p (RISCV_FUSE_ZEXTH)))
>
> Formatting nit.  Bring the && down to a new line and if you still need a
> line break for the "||",  then the "||" should be on a new line as
> well.  Something like this...
>
>
> if (simple_sets_p
>        && (riscv_fusion_enabled_p (RISCV_FUSE_ZEXTW
>
>            || riscv_fusion_enabled_p (RISCV_FUSE_ZEXTH)))
>
>
> > +       && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO(SET_DEST (curr_set))
>
> Space before open paren on this line.
>
>
> >
> > +       && (( INTVAL (XEXP (SET_SRC (curr_set), 1)) == 32
> > +             && riscv_fusion_enabled_p(RISCV_FUSE_ZEXTW) )
> > +           || ( INTVAL (XEXP (SET_SRC (curr_set), 1)) < 32
> > +                && riscv_fusion_enabled_p(RISCV_FUSE_ZEXTWS))))
>
> Extraneous spaces after the open parens before INTVALs above.
>
>
> > +       && REGNO (XEXP (SET_SRC (curr_set), 0)) == REGNO(SET_DEST (curr_set))
>
> Missing whitespace before open paren on this line.
>
>
> OK with the nits fixed.

Applied to master with these fixes (and a fix for the typo in the
commit message that Jakub spotted).
Thanks!

Philipp.

Re: [PATCH v2 2/2] RISC-V: Add instruction fusion (for ventana-vt1)

[Jeff Law]

On 11/14/22 11:55, Philipp Tomsich wrote:
> On Mon, 14 Nov 2022 at 17:06, Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>> On 11/13/22 13:48, Philipp Tomsich wrote:
>>> The Ventana VT1 core supports quad-issue and instruction fusion.
>>> This implemented TARGET_SCHED_MACRO_FUSION_P to keep fusible sequences
>>> together and adds idiom matcheing for the supported fusion cases.
>>>
>>> gcc/ChangeLog:
>>>
>>>        * config/riscv/riscv.cc (enum riscv_fusion_pairs): Add symbolic
>>>        constants to identify supported fusion patterns.
>>>        (struct riscv_tune_param): Add fusible_op field.
>>>        (riscv_macro_fusion_p): Implement.
>>>        (riscv_fusion_enabled_p): Implement.
>>>        (riscv_macro_fusion_pair_p): Implement and recoginze fusible
>>>        idioms for Ventana VT1.
>>>        (TARGET_SCHED_MACRO_FUSION_P): Point to riscv_macro_fusion_p.
>>>        (TARGET_SCHED_MACRO_FUSION_PAIR_P): Point to riscv_macro_fusion_pair_p.
>> You know the fusion rules for VT1 better than I...  I'm happy to largely
>> defer to you on this.
>>
>> I do wonder if going forward hand matching RTL like this is going to be
>> an unmaintainable mess and whether or not we would be better served
>> using insn attributes to describe instruction fusion.
> I had thought about that, too.
> In the end our team decided to stay away from it for the time being:
> fusion frequently needs to look at second-level properties and whether
> the first instruction's output register is overwritten by the second
> instruction.  So we kept with the same stereotype of idiom-matching
> that is also used for AArch64 today.

Yea, we're still going to have to grub around to get the operands.  But 
we'd know the overall form of the insn and the types of its operands was 
right.  But it's still going to be clunky either way.  My worry with the 
attribute approach is we'll end up with a horrible mess of attributes 
due to multiple fusion implementations and that we'll need to split 
alternatives so that we can tag them more precisely, etc.

It feels like we almost need a DSL to specify this stuff, much like we 
have for scheduling models.


Jeff

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Palmer Dabbelt]

On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>
> This series provides support for the Ventana VT1 (a 4-way superscalar
> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> for the supported instruction fusion patterns.
>
> This includes the addition of the fusion-aware scheduling
> infrastructure for RISC-V and implements idiom recognition for the
> fusion patterns supported by VT1.
>
> Note that we don't signal support for XVentanaCondOps at this point,
> as the XVentanaCondOps support is in-flight separately.  Changing the
> defaults for VT1 can happen late in the cycle, so no need to link the
> two different changesets.
>
> Changes in v2:
> - Rebased and changed over to .rst-based documentation
> - Updated to catch more fusion cases
> - Signals support for Zifencei
>
> Philipp Tomsich (2):
>   RISC-V: Add basic support for the Ventana-VT1 core
>   RISC-V: Add instruction fusion (for ventana-vt1)
>
>  gcc/config/riscv/riscv-cores.def              |   3 +
>  gcc/config/riscv/riscv-opts.h                 |   2 +-
>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>  .../risc-v-options.rst                        |   5 +-
>  4 files changed, 240 insertions(+), 3 deletions(-)

I guess we never really properly talked about this on the GCC mailing 
lists, but IMO it's fine to start taking code for designs that have been 
announced under the assumption that if the hardware doesn't actually 
show up according to those timelines that it will be assumed to have 
never existed and thus be removed more quickly than usual.

That said, I can't find anything describing that the VT-1 exists aside 
from these patches.  Is there anything that describes this design and 
when it's expected to be available?

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> >
> > This series provides support for the Ventana VT1 (a 4-way superscalar
> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> > for the supported instruction fusion patterns.
> >
> > This includes the addition of the fusion-aware scheduling
> > infrastructure for RISC-V and implements idiom recognition for the
> > fusion patterns supported by VT1.
> >
> > Note that we don't signal support for XVentanaCondOps at this point,
> > as the XVentanaCondOps support is in-flight separately.  Changing the
> > defaults for VT1 can happen late in the cycle, so no need to link the
> > two different changesets.
> >
> > Changes in v2:
> > - Rebased and changed over to .rst-based documentation
> > - Updated to catch more fusion cases
> > - Signals support for Zifencei
> >
> > Philipp Tomsich (2):
> >   RISC-V: Add basic support for the Ventana-VT1 core
> >   RISC-V: Add instruction fusion (for ventana-vt1)
> >
> >  gcc/config/riscv/riscv-cores.def              |   3 +
> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> >  .../risc-v-options.rst                        |   5 +-
> >  4 files changed, 240 insertions(+), 3 deletions(-)
>
> I guess we never really properly talked about this on the GCC mailing
> lists, but IMO it's fine to start taking code for designs that have been
> announced under the assumption that if the hardware doesn't actually
> show up according to those timelines that it will be assumed to have
> never existed and thus be removed more quickly than usual.
>
> That said, I can't find anything describing that the VT-1 exists aside
> from these patches.  Is there anything that describes this design and
> when it's expected to be available?

I have to defer to Jeff on this one.

Philipp.

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Palmer Dabbelt]

On Mon, 14 Nov 2022 12:03:38 PST (-0800), philipp.tomsich@vrull.eu wrote:
> On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>>
>> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> >
>> > This series provides support for the Ventana VT1 (a 4-way superscalar
>> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
>> > for the supported instruction fusion patterns.
>> >
>> > This includes the addition of the fusion-aware scheduling
>> > infrastructure for RISC-V and implements idiom recognition for the
>> > fusion patterns supported by VT1.
>> >
>> > Note that we don't signal support for XVentanaCondOps at this point,
>> > as the XVentanaCondOps support is in-flight separately.  Changing the
>> > defaults for VT1 can happen late in the cycle, so no need to link the
>> > two different changesets.
>> >
>> > Changes in v2:
>> > - Rebased and changed over to .rst-based documentation
>> > - Updated to catch more fusion cases
>> > - Signals support for Zifencei
>> >
>> > Philipp Tomsich (2):
>> >   RISC-V: Add basic support for the Ventana-VT1 core
>> >   RISC-V: Add instruction fusion (for ventana-vt1)
>> >
>> >  gcc/config/riscv/riscv-cores.def              |   3 +
>> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
>> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>> >  .../risc-v-options.rst                        |   5 +-
>> >  4 files changed, 240 insertions(+), 3 deletions(-)
>>
>> I guess we never really properly talked about this on the GCC mailing
>> lists, but IMO it's fine to start taking code for designs that have been
>> announced under the assumption that if the hardware doesn't actually
>> show up according to those timelines that it will be assumed to have
>> never existed and thus be removed more quickly than usual.
>>
>> That said, I can't find anything describing that the VT-1 exists aside
>> from these patches.  Is there anything that describes this design and
>> when it's expected to be available?
>
> I have to defer to Jeff on this one.

Looks like you already committed it, though:

991cfe5b30c ("RISC-V: Add instruction fusion (for ventana-vt1)")
b4fca4fc70d ("RISC-V: Add basic support for the Ventana-VT1 core")

We talked about this multiple times and I thought you were on board with 
the proposed "hardware needs to be announced" changes, did I 
misunderstand that?

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 21:58, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Mon, 14 Nov 2022 12:03:38 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> >>
> >> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> >> >
> >> > This series provides support for the Ventana VT1 (a 4-way superscalar
> >> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> >> > for the supported instruction fusion patterns.
> >> >
> >> > This includes the addition of the fusion-aware scheduling
> >> > infrastructure for RISC-V and implements idiom recognition for the
> >> > fusion patterns supported by VT1.
> >> >
> >> > Note that we don't signal support for XVentanaCondOps at this point,
> >> > as the XVentanaCondOps support is in-flight separately.  Changing the
> >> > defaults for VT1 can happen late in the cycle, so no need to link the
> >> > two different changesets.
> >> >
> >> > Changes in v2:
> >> > - Rebased and changed over to .rst-based documentation
> >> > - Updated to catch more fusion cases
> >> > - Signals support for Zifencei
> >> >
> >> > Philipp Tomsich (2):
> >> >   RISC-V: Add basic support for the Ventana-VT1 core
> >> >   RISC-V: Add instruction fusion (for ventana-vt1)
> >> >
> >> >  gcc/config/riscv/riscv-cores.def              |   3 +
> >> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
> >> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> >> >  .../risc-v-options.rst                        |   5 +-
> >> >  4 files changed, 240 insertions(+), 3 deletions(-)
> >>
> >> I guess we never really properly talked about this on the GCC mailing
> >> lists, but IMO it's fine to start taking code for designs that have been
> >> announced under the assumption that if the hardware doesn't actually
> >> show up according to those timelines that it will be assumed to have
> >> never existed and thus be removed more quickly than usual.
> >>
> >> That said, I can't find anything describing that the VT-1 exists aside
> >> from these patches.  Is there anything that describes this design and
> >> when it's expected to be available?
> >
> > I have to defer to Jeff on this one.
>
> Looks like you already committed it, though:
>
> 991cfe5b30c ("RISC-V: Add instruction fusion (for ventana-vt1)")
> b4fca4fc70d ("RISC-V: Add basic support for the Ventana-VT1 core")
>
> We talked about this multiple times and I thought you were on board with
> the proposed "hardware needs to be announced" changes, did I
> misunderstand that?

Sorry — I had assumed that the "basic support" changes were agreed
upon between you and Jeff, given that Jeff had given the OK.

My position is still the same as discussed at LPC that "hardware needs
to be announced".

Thanks,
Philipp.

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Palmer Dabbelt]

[Trying to join the threads here.]

On Mon, 14 Nov 2022 13:28:23 PST (-0800), philipp.tomsich@vrull.eu wrote:
> Jeff,
>
> On Mon, 14 Nov 2022 at 22:23, Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>>
>> On 11/14/22 13:00, Palmer Dabbelt wrote:
>> > On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> >>
>> >> This series provides support for the Ventana VT1 (a 4-way superscalar
>> >> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
>> >> for the supported instruction fusion patterns.
>> >>
>> >> This includes the addition of the fusion-aware scheduling
>> >> infrastructure for RISC-V and implements idiom recognition for the
>> >> fusion patterns supported by VT1.
>> >>
>> >> Note that we don't signal support for XVentanaCondOps at this point,
>> >> as the XVentanaCondOps support is in-flight separately. Changing the
>> >> defaults for VT1 can happen late in the cycle, so no need to link the
>> >> two different changesets.
>> >>
>> >> Changes in v2:
>> >> - Rebased and changed over to .rst-based documentation
>> >> - Updated to catch more fusion cases
>> >> - Signals support for Zifencei
>> >>
>> >> Philipp Tomsich (2):
>> >>   RISC-V: Add basic support for the Ventana-VT1 core
>> >>   RISC-V: Add instruction fusion (for ventana-vt1)
>> >>
>> >>  gcc/config/riscv/riscv-cores.def              |   3 +
>> >>  gcc/config/riscv/riscv-opts.h                 |   2 +-
>> >>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>> >>  .../risc-v-options.rst                        |   5 +-
>> >>  4 files changed, 240 insertions(+), 3 deletions(-)
>> >
>> > I guess we never really properly talked about this on the GCC mailing
>> > lists, but IMO it's fine to start taking code for designs that have
>> > been announced under the assumption that if the hardware doesn't
>> > actually show up according to those timelines that it will be assumed
>> > to have never existed and thus be removed more quickly than usual.
>> Absolutely.   I have zero interest in carrying around code for
>> nonexistent or dead variants.
>> >
>> > That said, I can't find anything describing that the VT-1 exists aside
>> > from these patches.  Is there anything that describes this design and
>> > when it's expected to be available?
>>
>> What do you need?  I can give some broad overview information on the
>> design, but it would likely just mirror what's already been mentioned in
>> these patches.
>>
>>
>> As far as schedules.  I'm not sure what I can say.  I'll check on that.

I'm less worried about the "does this pipeline model match the HW" bits, 
at least until the HW is publicly available then all we can do is rely 
on the vendor (and even after the HW is public the vendor might be the 
only one who cares enough to figure things out, nothing we can really do 
upstream there).  We've had some issues with nobody caring enough about 
the C906 pipeline model to sort out whether some patches are a net win, 
but if nobody (including the vendor) cares about the HW enough to 
benchmark things then there's not much we can do.

My bigger worry is getting roped in to supporting a bunch of hardware 
that doesn't actually exist yet and may never make it outside some 
vendor's lab.  That can generally be a ton of work and filters 
throughout GCC, even outside of the RISC-V backend.  We've already got 
enough chaos just trying to follow the ISA, chasing down issues related 
to hardware that may not ever manifest is just going to lead to 
craziness.

So on my end the point of the schedule is to have something we can look 
at and determine that the hardware is somehow defunct.  The fairest way 
we could come up with was to tie it to some sort of company announcement 
of the hardware: obviously everyone knows their internal timelines, but 
that's not fair to companies that don't employ someone with commit 
access.  Requirement some sort of public announcement means everyone has 
the same rules to play by, IMO that's really important in RISC-V land as 
there's so many vendors.

>> It was never my intention to bypass any process/procedures here. So if I
>> did, my apologies.
>
> The controversial part is XVentanaCondOps (as it is a vendor-defined
> extension), so I'll certainly hold off on that until both you and
> Palmer are in agreement on how to proceed there.

The pipeline models are essentially in the same spot.  We've got a bit 
of a precedent there for taking them just based on an announcement, but 
there isn't one here.

[and the other side of the thread]

On Mon, 14 Nov 2022 13:14:35 PST (-0800), philipp.tomsich@vrull.eu wrote:
> On Mon, 14 Nov 2022 at 21:58, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>>
>> On Mon, 14 Nov 2022 12:03:38 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> > On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>> >>
>> >> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> >> >
>> >> > This series provides support for the Ventana VT1 (a 4-way superscalar
>> >> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
>> >> > for the supported instruction fusion patterns.
>> >> >
>> >> > This includes the addition of the fusion-aware scheduling
>> >> > infrastructure for RISC-V and implements idiom recognition for the
>> >> > fusion patterns supported by VT1.
>> >> >
>> >> > Note that we don't signal support for XVentanaCondOps at this point,
>> >> > as the XVentanaCondOps support is in-flight separately.  Changing the
>> >> > defaults for VT1 can happen late in the cycle, so no need to link the
>> >> > two different changesets.
>> >> >
>> >> > Changes in v2:
>> >> > - Rebased and changed over to .rst-based documentation
>> >> > - Updated to catch more fusion cases
>> >> > - Signals support for Zifencei
>> >> >
>> >> > Philipp Tomsich (2):
>> >> >   RISC-V: Add basic support for the Ventana-VT1 core
>> >> >   RISC-V: Add instruction fusion (for ventana-vt1)
>> >> >
>> >> >  gcc/config/riscv/riscv-cores.def              |   3 +
>> >> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
>> >> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>> >> >  .../risc-v-options.rst                        |   5 +-
>> >> >  4 files changed, 240 insertions(+), 3 deletions(-)
>> >>
>> >> I guess we never really properly talked about this on the GCC mailing
>> >> lists, but IMO it's fine to start taking code for designs that have been
>> >> announced under the assumption that if the hardware doesn't actually
>> >> show up according to those timelines that it will be assumed to have
>> >> never existed and thus be removed more quickly than usual.
>> >>
>> >> That said, I can't find anything describing that the VT-1 exists aside
>> >> from these patches.  Is there anything that describes this design and
>> >> when it's expected to be available?
>> >
>> > I have to defer to Jeff on this one.
>>
>> Looks like you already committed it, though:
>>
>> 991cfe5b30c ("RISC-V: Add instruction fusion (for ventana-vt1)")
>> b4fca4fc70d ("RISC-V: Add basic support for the Ventana-VT1 core")
>>
>> We talked about this multiple times and I thought you were on board with
>> the proposed "hardware needs to be announced" changes, did I
>> misunderstand that?
>
> Sorry — I had assumed that the "basic support" changes were agreed
> upon between you and Jeff, given that Jeff had given the OK.

If anything was agreed on we would have talked about it on publicly on
the mailing list, these are community-oriented decisions and need to be
made as such.  It's true that sometimes folks talk outside the mailing 
lists about these things, but we're pretty careful to reflect everything 
back so everyone has a chance to be part of these discussions.

> My position is still the same as discussed at LPC that "hardware needs
> to be announced".

Even that hasn't been talked about on the mailing lists -- or really
even in any GNU toolchain related forum, we talked some about it some at 
Plumbers for Linux and in private about GCC, but the takeaway there for 
GCC was that you wanted to go talk to the Ventana folks to see if it was 
OK with them.

Sounds like that just added to the confusion, though, so maybe we should 
just have these discussion on the lists from now on?

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 23:47, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> [Trying to join the threads here.]
>
> On Mon, 14 Nov 2022 13:28:23 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > Jeff,
> >
> > On Mon, 14 Nov 2022 at 22:23, Jeff Law <jeffreyalaw@gmail.com> wrote:
> >>
> >>
> >> On 11/14/22 13:00, Palmer Dabbelt wrote:
> >> > On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> >> >>
> >> >> This series provides support for the Ventana VT1 (a 4-way superscalar
> >> >> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> >> >> for the supported instruction fusion patterns.
> >> >>
> >> >> This includes the addition of the fusion-aware scheduling
> >> >> infrastructure for RISC-V and implements idiom recognition for the
> >> >> fusion patterns supported by VT1.
> >> >>
> >> >> Note that we don't signal support for XVentanaCondOps at this point,
> >> >> as the XVentanaCondOps support is in-flight separately. Changing the
> >> >> defaults for VT1 can happen late in the cycle, so no need to link the
> >> >> two different changesets.
> >> >>
> >> >> Changes in v2:
> >> >> - Rebased and changed over to .rst-based documentation
> >> >> - Updated to catch more fusion cases
> >> >> - Signals support for Zifencei
> >> >>
> >> >> Philipp Tomsich (2):
> >> >>   RISC-V: Add basic support for the Ventana-VT1 core
> >> >>   RISC-V: Add instruction fusion (for ventana-vt1)
> >> >>
> >> >>  gcc/config/riscv/riscv-cores.def              |   3 +
> >> >>  gcc/config/riscv/riscv-opts.h                 |   2 +-
> >> >>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> >> >>  .../risc-v-options.rst                        |   5 +-
> >> >>  4 files changed, 240 insertions(+), 3 deletions(-)
> >> >
> >> > I guess we never really properly talked about this on the GCC mailing
> >> > lists, but IMO it's fine to start taking code for designs that have
> >> > been announced under the assumption that if the hardware doesn't
> >> > actually show up according to those timelines that it will be assumed
> >> > to have never existed and thus be removed more quickly than usual.
> >> Absolutely.   I have zero interest in carrying around code for
> >> nonexistent or dead variants.
> >> >
> >> > That said, I can't find anything describing that the VT-1 exists aside
> >> > from these patches.  Is there anything that describes this design and
> >> > when it's expected to be available?
> >>
> >> What do you need?  I can give some broad overview information on the
> >> design, but it would likely just mirror what's already been mentioned in
> >> these patches.
> >>
> >>
> >> As far as schedules.  I'm not sure what I can say.  I'll check on that.
>
> I'm less worried about the "does this pipeline model match the HW" bits,
> at least until the HW is publicly available then all we can do is rely
> on the vendor (and even after the HW is public the vendor might be the
> only one who cares enough to figure things out, nothing we can really do
> upstream there).  We've had some issues with nobody caring enough about
> the C906 pipeline model to sort out whether some patches are a net win,
> but if nobody (including the vendor) cares about the HW enough to
> benchmark things then there's not much we can do.
>
> My bigger worry is getting roped in to supporting a bunch of hardware
> that doesn't actually exist yet and may never make it outside some
> vendor's lab.  That can generally be a ton of work and filters
> throughout GCC, even outside of the RISC-V backend.  We've already got
> enough chaos just trying to follow the ISA, chasing down issues related
> to hardware that may not ever manifest is just going to lead to
> craziness.
>
> So on my end the point of the schedule is to have something we can look
> at and determine that the hardware is somehow defunct.  The fairest way
> we could come up with was to tie it to some sort of company announcement
> of the hardware: obviously everyone knows their internal timelines, but
> that's not fair to companies that don't employ someone with commit
> access.  Requirement some sort of public announcement means everyone has
> the same rules to play by, IMO that's really important in RISC-V land as
> there's so many vendors.
>
> >> It was never my intention to bypass any process/procedures here. So if I
> >> did, my apologies.
> >
> > The controversial part is XVentanaCondOps (as it is a vendor-defined
> > extension), so I'll certainly hold off on that until both you and
> > Palmer are in agreement on how to proceed there.
>
> The pipeline models are essentially in the same spot.  We've got a bit
> of a precedent there for taking them just based on an announcement, but
> there isn't one here.
>
> [and the other side of the thread]
>
> On Mon, 14 Nov 2022 13:14:35 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > On Mon, 14 Nov 2022 at 21:58, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> >>
> >> On Mon, 14 Nov 2022 12:03:38 PST (-0800), philipp.tomsich@vrull.eu wrote:
> >> > On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> >> >>
> >> >> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> >> >> >
> >> >> > This series provides support for the Ventana VT1 (a 4-way superscalar
> >> >> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> >> >> > for the supported instruction fusion patterns.
> >> >> >
> >> >> > This includes the addition of the fusion-aware scheduling
> >> >> > infrastructure for RISC-V and implements idiom recognition for the
> >> >> > fusion patterns supported by VT1.
> >> >> >
> >> >> > Note that we don't signal support for XVentanaCondOps at this point,
> >> >> > as the XVentanaCondOps support is in-flight separately.  Changing the
> >> >> > defaults for VT1 can happen late in the cycle, so no need to link the
> >> >> > two different changesets.
> >> >> >
> >> >> > Changes in v2:
> >> >> > - Rebased and changed over to .rst-based documentation
> >> >> > - Updated to catch more fusion cases
> >> >> > - Signals support for Zifencei
> >> >> >
> >> >> > Philipp Tomsich (2):
> >> >> >   RISC-V: Add basic support for the Ventana-VT1 core
> >> >> >   RISC-V: Add instruction fusion (for ventana-vt1)
> >> >> >
> >> >> >  gcc/config/riscv/riscv-cores.def              |   3 +
> >> >> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
> >> >> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> >> >> >  .../risc-v-options.rst                        |   5 +-
> >> >> >  4 files changed, 240 insertions(+), 3 deletions(-)
> >> >>
> >> >> I guess we never really properly talked about this on the GCC mailing
> >> >> lists, but IMO it's fine to start taking code for designs that have been
> >> >> announced under the assumption that if the hardware doesn't actually
> >> >> show up according to those timelines that it will be assumed to have
> >> >> never existed and thus be removed more quickly than usual.
> >> >>
> >> >> That said, I can't find anything describing that the VT-1 exists aside
> >> >> from these patches.  Is there anything that describes this design and
> >> >> when it's expected to be available?
> >> >
> >> > I have to defer to Jeff on this one.
> >>
> >> Looks like you already committed it, though:
> >>
> >> 991cfe5b30c ("RISC-V: Add instruction fusion (for ventana-vt1)")
> >> b4fca4fc70d ("RISC-V: Add basic support for the Ventana-VT1 core")
> >>
> >> We talked about this multiple times and I thought you were on board with
> >> the proposed "hardware needs to be announced" changes, did I
> >> misunderstand that?
> >
> > Sorry — I had assumed that the "basic support" changes were agreed
> > upon between you and Jeff, given that Jeff had given the OK.
>
> If anything was agreed on we would have talked about it on publicly on
> the mailing list, these are community-oriented decisions and need to be
> made as such.  It's true that sometimes folks talk outside the mailing
> lists about these things, but we're pretty careful to reflect everything
> back so everyone has a chance to be part of these discussions.
>
> > My position is still the same as discussed at LPC that "hardware needs
> > to be announced".
>
> Even that hasn't been talked about on the mailing lists -- or really
> even in any GNU toolchain related forum, we talked some about it some at
> Plumbers for Linux and in private about GCC, but the takeaway there for
> GCC was that you wanted to go talk to the Ventana folks to see if it was
> OK with them.

The mistake of making an assumption was mine, as I had again raised
the issue with the Ventana folks (and particularly with Jeff) as
recently as two weeks ago and read more into his "OK" than it was
meant to mean.

> Sounds like that just added to the confusion, though, so maybe we should
> just have these discussion on the lists from now on?

I'll stay with my assertion that some things are easier discussed
off-list.  However, taking them back to the appropriate mailing lists
and larger groups (as we had done at LPC in the RISC-V MC) is the best
way to summarize and ensure consensus.

We need to go through the process of how this "announcement" happens
once, so we have a blueprint for the future and no one resorts to
assumptions next time.
Let's get this done for the XVentanaCondOps merge between Jeff and the
rest of us, so we have something that we can refer back to.

Apologies again for the confusion,
Philipp.

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Richard Biener]

On Tue, Nov 15, 2022 at 12:01 AM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> On Mon, 14 Nov 2022 at 23:47, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> >
> > [Trying to join the threads here.]
> >
> > On Mon, 14 Nov 2022 13:28:23 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > > Jeff,
> > >
> > > On Mon, 14 Nov 2022 at 22:23, Jeff Law <jeffreyalaw@gmail.com> wrote:
> > >>
> > >>
> > >> On 11/14/22 13:00, Palmer Dabbelt wrote:
> > >> > On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > >> >>
> > >> >> This series provides support for the Ventana VT1 (a 4-way superscalar
> > >> >> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> > >> >> for the supported instruction fusion patterns.
> > >> >>
> > >> >> This includes the addition of the fusion-aware scheduling
> > >> >> infrastructure for RISC-V and implements idiom recognition for the
> > >> >> fusion patterns supported by VT1.
> > >> >>
> > >> >> Note that we don't signal support for XVentanaCondOps at this point,
> > >> >> as the XVentanaCondOps support is in-flight separately. Changing the
> > >> >> defaults for VT1 can happen late in the cycle, so no need to link the
> > >> >> two different changesets.
> > >> >>
> > >> >> Changes in v2:
> > >> >> - Rebased and changed over to .rst-based documentation
> > >> >> - Updated to catch more fusion cases
> > >> >> - Signals support for Zifencei
> > >> >>
> > >> >> Philipp Tomsich (2):
> > >> >>   RISC-V: Add basic support for the Ventana-VT1 core
> > >> >>   RISC-V: Add instruction fusion (for ventana-vt1)
> > >> >>
> > >> >>  gcc/config/riscv/riscv-cores.def              |   3 +
> > >> >>  gcc/config/riscv/riscv-opts.h                 |   2 +-
> > >> >>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> > >> >>  .../risc-v-options.rst                        |   5 +-
> > >> >>  4 files changed, 240 insertions(+), 3 deletions(-)
> > >> >
> > >> > I guess we never really properly talked about this on the GCC mailing
> > >> > lists, but IMO it's fine to start taking code for designs that have
> > >> > been announced under the assumption that if the hardware doesn't
> > >> > actually show up according to those timelines that it will be assumed
> > >> > to have never existed and thus be removed more quickly than usual.
> > >> Absolutely.   I have zero interest in carrying around code for
> > >> nonexistent or dead variants.
> > >> >
> > >> > That said, I can't find anything describing that the VT-1 exists aside
> > >> > from these patches.  Is there anything that describes this design and
> > >> > when it's expected to be available?
> > >>
> > >> What do you need?  I can give some broad overview information on the
> > >> design, but it would likely just mirror what's already been mentioned in
> > >> these patches.
> > >>
> > >>
> > >> As far as schedules.  I'm not sure what I can say.  I'll check on that.
> >
> > I'm less worried about the "does this pipeline model match the HW" bits,
> > at least until the HW is publicly available then all we can do is rely
> > on the vendor (and even after the HW is public the vendor might be the
> > only one who cares enough to figure things out, nothing we can really do
> > upstream there).  We've had some issues with nobody caring enough about
> > the C906 pipeline model to sort out whether some patches are a net win,
> > but if nobody (including the vendor) cares about the HW enough to
> > benchmark things then there's not much we can do.
> >
> > My bigger worry is getting roped in to supporting a bunch of hardware
> > that doesn't actually exist yet and may never make it outside some
> > vendor's lab.  That can generally be a ton of work and filters
> > throughout GCC, even outside of the RISC-V backend.  We've already got
> > enough chaos just trying to follow the ISA, chasing down issues related
> > to hardware that may not ever manifest is just going to lead to
> > craziness.
> >
> > So on my end the point of the schedule is to have something we can look
> > at and determine that the hardware is somehow defunct.  The fairest way
> > we could come up with was to tie it to some sort of company announcement
> > of the hardware: obviously everyone knows their internal timelines, but
> > that's not fair to companies that don't employ someone with commit
> > access.  Requirement some sort of public announcement means everyone has
> > the same rules to play by, IMO that's really important in RISC-V land as
> > there's so many vendors.
> >
> > >> It was never my intention to bypass any process/procedures here. So if I
> > >> did, my apologies.
> > >
> > > The controversial part is XVentanaCondOps (as it is a vendor-defined
> > > extension), so I'll certainly hold off on that until both you and
> > > Palmer are in agreement on how to proceed there.
> >
> > The pipeline models are essentially in the same spot.  We've got a bit
> > of a precedent there for taking them just based on an announcement, but
> > there isn't one here.
> >
> > [and the other side of the thread]
> >
> > On Mon, 14 Nov 2022 13:14:35 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > > On Mon, 14 Nov 2022 at 21:58, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> > >>
> > >> On Mon, 14 Nov 2022 12:03:38 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > >> > On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> > >> >>
> > >> >> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> > >> >> >
> > >> >> > This series provides support for the Ventana VT1 (a 4-way superscalar
> > >> >> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> > >> >> > for the supported instruction fusion patterns.
> > >> >> >
> > >> >> > This includes the addition of the fusion-aware scheduling
> > >> >> > infrastructure for RISC-V and implements idiom recognition for the
> > >> >> > fusion patterns supported by VT1.
> > >> >> >
> > >> >> > Note that we don't signal support for XVentanaCondOps at this point,
> > >> >> > as the XVentanaCondOps support is in-flight separately.  Changing the
> > >> >> > defaults for VT1 can happen late in the cycle, so no need to link the
> > >> >> > two different changesets.
> > >> >> >
> > >> >> > Changes in v2:
> > >> >> > - Rebased and changed over to .rst-based documentation
> > >> >> > - Updated to catch more fusion cases
> > >> >> > - Signals support for Zifencei
> > >> >> >
> > >> >> > Philipp Tomsich (2):
> > >> >> >   RISC-V: Add basic support for the Ventana-VT1 core
> > >> >> >   RISC-V: Add instruction fusion (for ventana-vt1)
> > >> >> >
> > >> >> >  gcc/config/riscv/riscv-cores.def              |   3 +
> > >> >> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
> > >> >> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> > >> >> >  .../risc-v-options.rst                        |   5 +-
> > >> >> >  4 files changed, 240 insertions(+), 3 deletions(-)
> > >> >>
> > >> >> I guess we never really properly talked about this on the GCC mailing
> > >> >> lists, but IMO it's fine to start taking code for designs that have been
> > >> >> announced under the assumption that if the hardware doesn't actually
> > >> >> show up according to those timelines that it will be assumed to have
> > >> >> never existed and thus be removed more quickly than usual.
> > >> >>
> > >> >> That said, I can't find anything describing that the VT-1 exists aside
> > >> >> from these patches.  Is there anything that describes this design and
> > >> >> when it's expected to be available?
> > >> >
> > >> > I have to defer to Jeff on this one.
> > >>
> > >> Looks like you already committed it, though:
> > >>
> > >> 991cfe5b30c ("RISC-V: Add instruction fusion (for ventana-vt1)")
> > >> b4fca4fc70d ("RISC-V: Add basic support for the Ventana-VT1 core")
> > >>
> > >> We talked about this multiple times and I thought you were on board with
> > >> the proposed "hardware needs to be announced" changes, did I
> > >> misunderstand that?
> > >
> > > Sorry — I had assumed that the "basic support" changes were agreed
> > > upon between you and Jeff, given that Jeff had given the OK.
> >
> > If anything was agreed on we would have talked about it on publicly on
> > the mailing list, these are community-oriented decisions and need to be
> > made as such.  It's true that sometimes folks talk outside the mailing
> > lists about these things, but we're pretty careful to reflect everything
> > back so everyone has a chance to be part of these discussions.
> >
> > > My position is still the same as discussed at LPC that "hardware needs
> > > to be announced".
> >
> > Even that hasn't been talked about on the mailing lists -- or really
> > even in any GNU toolchain related forum, we talked some about it some at
> > Plumbers for Linux and in private about GCC, but the takeaway there for
> > GCC was that you wanted to go talk to the Ventana folks to see if it was
> > OK with them.
>
> The mistake of making an assumption was mine, as I had again raised
> the issue with the Ventana folks (and particularly with Jeff) as
> recently as two weeks ago and read more into his "OK" than it was
> meant to mean.
>
> > Sounds like that just added to the confusion, though, so maybe we should
> > just have these discussion on the lists from now on?
>
> I'll stay with my assertion that some things are easier discussed
> off-list.  However, taking them back to the appropriate mailing lists
> and larger groups (as we had done at LPC in the RISC-V MC) is the best
> way to summarize and ensure consensus.
>
> We need to go through the process of how this "announcement" happens
> once, so we have a blueprint for the future and no one resorts to
> assumptions next time.
> Let's get this done for the XVentanaCondOps merge between Jeff and the
> rest of us, so we have something that we can refer back to.

Just to add from the release manager perspective: RISC-V is neither a
primary nor a secondary target so whether it builds, works or delivers
what it promises is of no concern for the release criteria.  That also
effectively
means you are not bound by the stage1 / stage3 / stage4 rules - you get
to define the rules yourself.  Obviously only if changes affect the port only.
And obviously the release will happen without consulting you, which means
you _should_ set your own rules just in case you don't want to follow
stage3 / stage4 rules strictly.

Just wanted to re-iterate this since the patches seem to be posted to
timely make the stage1 deadline which doesn't really exist for you.

Richard.

> Apologies again for the confusion,
> Philipp.

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Palmer Dabbelt]

On Mon, 14 Nov 2022 23:25:54 PST (-0800), richard.guenther@gmail.com wrote:
> On Tue, Nov 15, 2022 at 12:01 AM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
>>
>> On Mon, 14 Nov 2022 at 23:47, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>> >
>> > [Trying to join the threads here.]
>> >
>> > On Mon, 14 Nov 2022 13:28:23 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> > > Jeff,
>> > >
>> > > On Mon, 14 Nov 2022 at 22:23, Jeff Law <jeffreyalaw@gmail.com> wrote:
>> > >>
>> > >>
>> > >> On 11/14/22 13:00, Palmer Dabbelt wrote:
>> > >> > On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> > >> >>
>> > >> >> This series provides support for the Ventana VT1 (a 4-way superscalar
>> > >> >> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
>> > >> >> for the supported instruction fusion patterns.
>> > >> >>
>> > >> >> This includes the addition of the fusion-aware scheduling
>> > >> >> infrastructure for RISC-V and implements idiom recognition for the
>> > >> >> fusion patterns supported by VT1.
>> > >> >>
>> > >> >> Note that we don't signal support for XVentanaCondOps at this point,
>> > >> >> as the XVentanaCondOps support is in-flight separately. Changing the
>> > >> >> defaults for VT1 can happen late in the cycle, so no need to link the
>> > >> >> two different changesets.
>> > >> >>
>> > >> >> Changes in v2:
>> > >> >> - Rebased and changed over to .rst-based documentation
>> > >> >> - Updated to catch more fusion cases
>> > >> >> - Signals support for Zifencei
>> > >> >>
>> > >> >> Philipp Tomsich (2):
>> > >> >>   RISC-V: Add basic support for the Ventana-VT1 core
>> > >> >>   RISC-V: Add instruction fusion (for ventana-vt1)
>> > >> >>
>> > >> >>  gcc/config/riscv/riscv-cores.def              |   3 +
>> > >> >>  gcc/config/riscv/riscv-opts.h                 |   2 +-
>> > >> >>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>> > >> >>  .../risc-v-options.rst                        |   5 +-
>> > >> >>  4 files changed, 240 insertions(+), 3 deletions(-)
>> > >> >
>> > >> > I guess we never really properly talked about this on the GCC mailing
>> > >> > lists, but IMO it's fine to start taking code for designs that have
>> > >> > been announced under the assumption that if the hardware doesn't
>> > >> > actually show up according to those timelines that it will be assumed
>> > >> > to have never existed and thus be removed more quickly than usual.
>> > >> Absolutely.   I have zero interest in carrying around code for
>> > >> nonexistent or dead variants.
>> > >> >
>> > >> > That said, I can't find anything describing that the VT-1 exists aside
>> > >> > from these patches.  Is there anything that describes this design and
>> > >> > when it's expected to be available?
>> > >>
>> > >> What do you need?  I can give some broad overview information on the
>> > >> design, but it would likely just mirror what's already been mentioned in
>> > >> these patches.
>> > >>
>> > >>
>> > >> As far as schedules.  I'm not sure what I can say.  I'll check on that.
>> >
>> > I'm less worried about the "does this pipeline model match the HW" bits,
>> > at least until the HW is publicly available then all we can do is rely
>> > on the vendor (and even after the HW is public the vendor might be the
>> > only one who cares enough to figure things out, nothing we can really do
>> > upstream there).  We've had some issues with nobody caring enough about
>> > the C906 pipeline model to sort out whether some patches are a net win,
>> > but if nobody (including the vendor) cares about the HW enough to
>> > benchmark things then there's not much we can do.
>> >
>> > My bigger worry is getting roped in to supporting a bunch of hardware
>> > that doesn't actually exist yet and may never make it outside some
>> > vendor's lab.  That can generally be a ton of work and filters
>> > throughout GCC, even outside of the RISC-V backend.  We've already got
>> > enough chaos just trying to follow the ISA, chasing down issues related
>> > to hardware that may not ever manifest is just going to lead to
>> > craziness.
>> >
>> > So on my end the point of the schedule is to have something we can look
>> > at and determine that the hardware is somehow defunct.  The fairest way
>> > we could come up with was to tie it to some sort of company announcement
>> > of the hardware: obviously everyone knows their internal timelines, but
>> > that's not fair to companies that don't employ someone with commit
>> > access.  Requirement some sort of public announcement means everyone has
>> > the same rules to play by, IMO that's really important in RISC-V land as
>> > there's so many vendors.
>> >
>> > >> It was never my intention to bypass any process/procedures here. So if I
>> > >> did, my apologies.
>> > >
>> > > The controversial part is XVentanaCondOps (as it is a vendor-defined
>> > > extension), so I'll certainly hold off on that until both you and
>> > > Palmer are in agreement on how to proceed there.
>> >
>> > The pipeline models are essentially in the same spot.  We've got a bit
>> > of a precedent there for taking them just based on an announcement, but
>> > there isn't one here.
>> >
>> > [and the other side of the thread]
>> >
>> > On Mon, 14 Nov 2022 13:14:35 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> > > On Mon, 14 Nov 2022 at 21:58, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>> > >>
>> > >> On Mon, 14 Nov 2022 12:03:38 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> > >> > On Mon, 14 Nov 2022 at 21:00, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>> > >> >>
>> > >> >> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>> > >> >> >
>> > >> >> > This series provides support for the Ventana VT1 (a 4-way superscalar
>> > >> >> > rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
>> > >> >> > for the supported instruction fusion patterns.
>> > >> >> >
>> > >> >> > This includes the addition of the fusion-aware scheduling
>> > >> >> > infrastructure for RISC-V and implements idiom recognition for the
>> > >> >> > fusion patterns supported by VT1.
>> > >> >> >
>> > >> >> > Note that we don't signal support for XVentanaCondOps at this point,
>> > >> >> > as the XVentanaCondOps support is in-flight separately.  Changing the
>> > >> >> > defaults for VT1 can happen late in the cycle, so no need to link the
>> > >> >> > two different changesets.
>> > >> >> >
>> > >> >> > Changes in v2:
>> > >> >> > - Rebased and changed over to .rst-based documentation
>> > >> >> > - Updated to catch more fusion cases
>> > >> >> > - Signals support for Zifencei
>> > >> >> >
>> > >> >> > Philipp Tomsich (2):
>> > >> >> >   RISC-V: Add basic support for the Ventana-VT1 core
>> > >> >> >   RISC-V: Add instruction fusion (for ventana-vt1)
>> > >> >> >
>> > >> >> >  gcc/config/riscv/riscv-cores.def              |   3 +
>> > >> >> >  gcc/config/riscv/riscv-opts.h                 |   2 +-
>> > >> >> >  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>> > >> >> >  .../risc-v-options.rst                        |   5 +-
>> > >> >> >  4 files changed, 240 insertions(+), 3 deletions(-)
>> > >> >>
>> > >> >> I guess we never really properly talked about this on the GCC mailing
>> > >> >> lists, but IMO it's fine to start taking code for designs that have been
>> > >> >> announced under the assumption that if the hardware doesn't actually
>> > >> >> show up according to those timelines that it will be assumed to have
>> > >> >> never existed and thus be removed more quickly than usual.
>> > >> >>
>> > >> >> That said, I can't find anything describing that the VT-1 exists aside
>> > >> >> from these patches.  Is there anything that describes this design and
>> > >> >> when it's expected to be available?
>> > >> >
>> > >> > I have to defer to Jeff on this one.
>> > >>
>> > >> Looks like you already committed it, though:
>> > >>
>> > >> 991cfe5b30c ("RISC-V: Add instruction fusion (for ventana-vt1)")
>> > >> b4fca4fc70d ("RISC-V: Add basic support for the Ventana-VT1 core")
>> > >>
>> > >> We talked about this multiple times and I thought you were on board with
>> > >> the proposed "hardware needs to be announced" changes, did I
>> > >> misunderstand that?
>> > >
>> > > Sorry — I had assumed that the "basic support" changes were agreed
>> > > upon between you and Jeff, given that Jeff had given the OK.
>> >
>> > If anything was agreed on we would have talked about it on publicly on
>> > the mailing list, these are community-oriented decisions and need to be
>> > made as such.  It's true that sometimes folks talk outside the mailing
>> > lists about these things, but we're pretty careful to reflect everything
>> > back so everyone has a chance to be part of these discussions.
>> >
>> > > My position is still the same as discussed at LPC that "hardware needs
>> > > to be announced".
>> >
>> > Even that hasn't been talked about on the mailing lists -- or really
>> > even in any GNU toolchain related forum, we talked some about it some at
>> > Plumbers for Linux and in private about GCC, but the takeaway there for
>> > GCC was that you wanted to go talk to the Ventana folks to see if it was
>> > OK with them.
>>
>> The mistake of making an assumption was mine, as I had again raised
>> the issue with the Ventana folks (and particularly with Jeff) as
>> recently as two weeks ago and read more into his "OK" than it was
>> meant to mean.
>>
>> > Sounds like that just added to the confusion, though, so maybe we should
>> > just have these discussion on the lists from now on?
>>
>> I'll stay with my assertion that some things are easier discussed
>> off-list.  However, taking them back to the appropriate mailing lists
>> and larger groups (as we had done at LPC in the RISC-V MC) is the best
>> way to summarize and ensure consensus.
>>
>> We need to go through the process of how this "announcement" happens
>> once, so we have a blueprint for the future and no one resorts to
>> assumptions next time.
>> Let's get this done for the XVentanaCondOps merge between Jeff and the
>> rest of us, so we have something that we can refer back to.
>
> Just to add from the release manager perspective: RISC-V is neither a
> primary nor a secondary target so whether it builds, works or delivers
> what it promises is of no concern for the release criteria.  That also
> effectively
> means you are not bound by the stage1 / stage3 / stage4 rules - you get
> to define the rules yourself.  Obviously only if changes affect the port only.
> And obviously the release will happen without consulting you, which means
> you _should_ set your own rules just in case you don't want to follow
> stage3 / stage4 rules strictly.

Oh ya, we take advantage of that all the time.  Probably way too 
much...

> Just wanted to re-iterate this since the patches seem to be posted to
> timely make the stage1 deadline which doesn't really exist for you.

... though just for our own personal sanity we've been trying to get our 
stuff merged earlier in the cycle.  I think we've always taken the "we 
can just do whatever" thing a bit too liberally and ended up with a lot 
of stress in the last 2-4 weeks before release when everything's broken 
and there's still features going in.

That said, given the magnitude/timing of this patch dump it looks like 
we're going to be in a similar spot for this release.  No big deal, the 
whole sanity thing is sort of a aspirational goal ;)

>
> Richard.
>
>> Apologies again for the confusion,
>> Philipp.

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Jeff Law]

On 11/14/22 13:00, Palmer Dabbelt wrote:
> On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
>>
>> This series provides support for the Ventana VT1 (a 4-way superscalar
>> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
>> for the supported instruction fusion patterns.
>>
>> This includes the addition of the fusion-aware scheduling
>> infrastructure for RISC-V and implements idiom recognition for the
>> fusion patterns supported by VT1.
>>
>> Note that we don't signal support for XVentanaCondOps at this point,
>> as the XVentanaCondOps support is in-flight separately. Changing the
>> defaults for VT1 can happen late in the cycle, so no need to link the
>> two different changesets.
>>
>> Changes in v2:
>> - Rebased and changed over to .rst-based documentation
>> - Updated to catch more fusion cases
>> - Signals support for Zifencei
>>
>> Philipp Tomsich (2):
>>   RISC-V: Add basic support for the Ventana-VT1 core
>>   RISC-V: Add instruction fusion (for ventana-vt1)
>>
>>  gcc/config/riscv/riscv-cores.def              |   3 +
>>  gcc/config/riscv/riscv-opts.h                 |   2 +-
>>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
>>  .../risc-v-options.rst                        |   5 +-
>>  4 files changed, 240 insertions(+), 3 deletions(-)
>
> I guess we never really properly talked about this on the GCC mailing 
> lists, but IMO it's fine to start taking code for designs that have 
> been announced under the assumption that if the hardware doesn't 
> actually show up according to those timelines that it will be assumed 
> to have never existed and thus be removed more quickly than usual.
Absolutely.   I have zero interest in carrying around code for 
nonexistent or dead variants.
>
> That said, I can't find anything describing that the VT-1 exists aside 
> from these patches.  Is there anything that describes this design and 
> when it's expected to be available?

What do you need?  I can give some broad overview information on the 
design, but it would likely just mirror what's already been mentioned in 
these patches.


As far as schedules.  I'm not sure what I can say.  I'll check on that.


It was never my intention to bypass any process/procedures here. So if I 
did, my apologies.


jeff

Re: [PATCH v2 0/2] Basic support for the Ventana VT1 w/ instruction fusion

[Philipp Tomsich]

Jeff,

On Mon, 14 Nov 2022 at 22:23, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/14/22 13:00, Palmer Dabbelt wrote:
> > On Sun, 13 Nov 2022 12:48:22 PST (-0800), philipp.tomsich@vrull.eu wrote:
> >>
> >> This series provides support for the Ventana VT1 (a 4-way superscalar
> >> rv64gc_zba_zbb_zbc_zbs_zifenci_xventanacondops core) including support
> >> for the supported instruction fusion patterns.
> >>
> >> This includes the addition of the fusion-aware scheduling
> >> infrastructure for RISC-V and implements idiom recognition for the
> >> fusion patterns supported by VT1.
> >>
> >> Note that we don't signal support for XVentanaCondOps at this point,
> >> as the XVentanaCondOps support is in-flight separately. Changing the
> >> defaults for VT1 can happen late in the cycle, so no need to link the
> >> two different changesets.
> >>
> >> Changes in v2:
> >> - Rebased and changed over to .rst-based documentation
> >> - Updated to catch more fusion cases
> >> - Signals support for Zifencei
> >>
> >> Philipp Tomsich (2):
> >>   RISC-V: Add basic support for the Ventana-VT1 core
> >>   RISC-V: Add instruction fusion (for ventana-vt1)
> >>
> >>  gcc/config/riscv/riscv-cores.def              |   3 +
> >>  gcc/config/riscv/riscv-opts.h                 |   2 +-
> >>  gcc/config/riscv/riscv.cc                     | 233 ++++++++++++++++++
> >>  .../risc-v-options.rst                        |   5 +-
> >>  4 files changed, 240 insertions(+), 3 deletions(-)
> >
> > I guess we never really properly talked about this on the GCC mailing
> > lists, but IMO it's fine to start taking code for designs that have
> > been announced under the assumption that if the hardware doesn't
> > actually show up according to those timelines that it will be assumed
> > to have never existed and thus be removed more quickly than usual.
> Absolutely.   I have zero interest in carrying around code for
> nonexistent or dead variants.
> >
> > That said, I can't find anything describing that the VT-1 exists aside
> > from these patches.  Is there anything that describes this design and
> > when it's expected to be available?
>
> What do you need?  I can give some broad overview information on the
> design, but it would likely just mirror what's already been mentioned in
> these patches.
>
>
> As far as schedules.  I'm not sure what I can say.  I'll check on that.
>
>
> It was never my intention to bypass any process/procedures here. So if I
> did, my apologies.

The controversial part is XVentanaCondOps (as it is a vendor-defined
extension), so I'll certainly hold off on that until both you and
Palmer are in agreement on how to proceed there.

Thanks,
Philipp.

> jeff
>