Re: [PATCH V2] RISC-V: Switch RVV cost model.

2024-01-10 Thread Robin Dapp
LGTM.

Regards
 Robin



[PATCH V2] RISC-V: Switch RVV cost model.

2024-01-10 Thread Juzhe-Zhong
This patch is preparing patch for the following cost model tweak.

Since we don't have vector cost model in default tune info (rocket),
we set the cost model default as generic cost model by default.

The reason we want to switch to generic vector cost model is the default
cost model generates inferior codegen for various benchmarks.

For example, PR113247, we have performance bug that we end up having over 70%
performance drop of SHA256.  Currently, no matter how we adapt cost model,
we are not able to fix the performance bug since we always use default cost 
model by default.

Also, tweak the generic cost model back to default cost model since we have 
some FAILs in
current tests.

After this patch, we (me an Robin) can work on cost model tunning together to 
improve performane
in various benchmarks.

Tested on both RV32 and RV64, ok for trunk ?

gcc/ChangeLog:

* config/riscv/riscv.cc (get_common_costs): Switch RVV cost model.
(get_vector_costs): Ditto.
(riscv_builtin_vectorization_cost): Ditto.

---
 gcc/config/riscv/riscv.cc | 144 --
 1 file changed, 75 insertions(+), 69 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 32183d63180..cca01fd54d9 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -352,48 +352,49 @@ const enum reg_class 
riscv_regno_to_class[FIRST_PSEUDO_REGISTER] = {
   VD_REGS, VD_REGS,VD_REGS,VD_REGS,
 };
 
-/* Generic costs for VLS vector operations.   */
-static const common_vector_cost generic_vls_vector_cost = {
+/* RVV costs for VLS vector operations.   */
+static const common_vector_cost rvv_vls_vector_cost = {
   1, /* int_stmt_cost  */
   1, /* fp_stmt_cost  */
   1, /* gather_load_cost  */
   1, /* scatter_store_cost  */
-  2, /* vec_to_scalar_cost  */
+  1, /* vec_to_scalar_cost  */
   1, /* scalar_to_vec_cost  */
-  2, /* permute_cost  */
+  1, /* permute_cost  */
   1, /* align_load_cost  */
   1, /* align_store_cost  */
-  1, /* unalign_load_cost  */
-  1, /* unalign_store_cost  */
+  2, /* unalign_load_cost  */
+  2, /* unalign_store_cost  */
 };
 
-/* Generic costs for VLA vector operations.  */
-static const scalable_vector_cost generic_vla_vector_cost = {
+/* RVV costs for VLA vector operations.  */
+static const scalable_vector_cost rvv_vla_vector_cost = {
   {
 1, /* int_stmt_cost  */
 1, /* fp_stmt_cost  */
 1, /* gather_load_cost  */
 1, /* scatter_store_cost  */
-2, /* vec_to_scalar_cost  */
+1, /* vec_to_scalar_cost  */
 1, /* scalar_to_vec_cost  */
-2, /* permute_cost  */
+1, /* permute_cost  */
 1, /* align_load_cost  */
 1, /* align_store_cost  */
-1, /* unalign_load_cost  */
-1, /* unalign_store_cost  */
+2, /* unalign_load_cost  */
+2, /* unalign_store_cost  */
   },
 };
 
-/* Generic costs for vector insn classes.  */
+/* Generic costs for vector insn classes.  It is supposed to be the vector cost
+   models used by default if no other cost model was specified.  */
 static const struct cpu_vector_cost generic_vector_cost = {
-  1,   /* scalar_int_stmt_cost  */
-  1,   /* scalar_fp_stmt_cost  */
-  1,   /* scalar_load_cost  */
-  1,   /* scalar_store_cost  */
-  3,   /* cond_taken_branch_cost  */
-  1,   /* cond_not_taken_branch_cost  */
-  _vls_vector_cost, /* vls  */
-  _vla_vector_cost, /* vla */
+  1,   /* scalar_int_stmt_cost  */
+  1,   /* scalar_fp_stmt_cost  */
+  1,   /* scalar_load_cost  */
+  1,   /* scalar_store_cost  */
+  3,   /* cond_taken_branch_cost  */
+  1,   /* cond_not_taken_branch_cost  */
+  _vls_vector_cost, /* vls  */
+  _vla_vector_cost, /* vla */
 };
 
 /* Costs to use when optimizing for rocket.  */
@@ -10372,11 +10373,10 @@ riscv_frame_pointer_required (void)
   return riscv_save_frame_pointer && !crtl->is_leaf;
 }
 
-/* Return the appropriate common costs for vectors of type VECTYPE.  */
+/* Return the appropriate common costs according to VECTYPE from COSTS.  */
 static const common_vector_cost *
-get_common_costs (tree vectype)
+get_common_costs (const cpu_vector_cost *costs, tree vectype)
 {
-  const cpu_vector_cost *costs = tune_param->vec_costs;
   gcc_assert (costs);
 
   if (vectype && riscv_v_ext_vls_mode_p (TYPE_MODE (vectype)))
@@ -10384,78 +10384,84 @@ get_common_costs (tree vectype)
   return costs->vla;
 }
 
+/* Return the CPU vector costs according to -mtune if tune info has non-NULL
+   vector cost.  Otherwide, return the default generic vector costs.  */
+static const cpu_vector_cost *
+get_vector_costs ()
+{
+  const cpu_vector_cost *costs = tune_param->vec_costs;
+  if (!costs)
+return _vector_cost;
+  return costs;
+}
+
 /* Implement targetm.vectorize.builtin_vectorization_cost.  */
 
 static int