diff mbox series

[2/9] vect: Adjust vectorizable_load costing on VMAT_GATHER_SCATTER && gs_info.decl

Message ID 9bad792a4bcef35fbd9906245bf3493672b340fe.1686573640.git.linkw@linux.ibm.com
State Accepted
Headers
Series vect: Move costing next to the transform for vect load |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Kewen.Lin June 13, 2023, 2:03 a.m. UTC 
  This patch adds one extra argument cost_vec to function
vect_build_gather_load_calls, so that we can do costing
next to the tranform in vect_build_gather_load_calls.
For now, the implementation just follows the handlings in
vect_model_load_cost, it isn't so good, so placing one
FIXME for any further improvement.  This patch should not
cause any functional changes.

gcc/ChangeLog:

	* tree-vect-stmts.cc (vect_build_gather_load_calls): Add the handlings
	on costing with one extra argument cost_vec.
	(vectorizable_load): Adjust the call to vect_build_gather_load_calls.
	(vect_model_load_cost): Assert it won't get VMAT_GATHER_SCATTER with
	gs_info.decl set any more.
---
 gcc/tree-vect-stmts.cc | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

Comments

Richard Biener June 30, 2023, 11:11 a.m. UTC | #1 
On Tue, Jun 13, 2023 at 4:03 AM Kewen Lin <linkw@linux.ibm.com> wrote:
>
> This patch adds one extra argument cost_vec to function
> vect_build_gather_load_calls, so that we can do costing
> next to the tranform in vect_build_gather_load_calls.
> For now, the implementation just follows the handlings in
> vect_model_load_cost, it isn't so good, so placing one
> FIXME for any further improvement.  This patch should not
> cause any functional changes.
>
> gcc/ChangeLog:
>
>         * tree-vect-stmts.cc (vect_build_gather_load_calls): Add the handlings
>         on costing with one extra argument cost_vec.
>         (vectorizable_load): Adjust the call to vect_build_gather_load_calls.
>         (vect_model_load_cost): Assert it won't get VMAT_GATHER_SCATTER with
>         gs_info.decl set any more.
> ---
>  gcc/tree-vect-stmts.cc | 31 +++++++++++++++++++++++--------
>  1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 44514658be3..744cdf40e26 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -1135,6 +1135,8 @@ vect_model_load_cost (vec_info *vinfo,
>                       slp_tree slp_node,
>                       stmt_vector_for_cost *cost_vec)
>  {
> +  gcc_assert (memory_access_type != VMAT_GATHER_SCATTER || !gs_info->decl);
> +
>    unsigned int inside_cost = 0, prologue_cost = 0;
>    bool grouped_access_p = STMT_VINFO_GROUPED_ACCESS (stmt_info);
>
> @@ -2819,7 +2821,8 @@ vect_build_gather_load_calls (vec_info *vinfo, stmt_vec_info stmt_info,
>                               gimple_stmt_iterator *gsi,
>                               gimple **vec_stmt,
>                               gather_scatter_info *gs_info,
> -                             tree mask)
> +                             tree mask,
> +                             stmt_vector_for_cost *cost_vec)
>  {
>    loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo);
>    class loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
> @@ -2831,6 +2834,23 @@ vect_build_gather_load_calls (vec_info *vinfo, stmt_vec_info stmt_info,
>    poly_uint64 gather_off_nunits
>      = TYPE_VECTOR_SUBPARTS (gs_info->offset_vectype);
>
> +  /* FIXME: Keep the previous costing way in vect_model_load_cost by costing
> +     N scalar loads, but it should be tweaked to use target specific costs
> +     on related gather load calls.  */
> +  if (!vec_stmt)

going over the series now, I'm collecting comments but wanted to get
this one out here:
I'd rather see if (cost_vec) here, that 'vec_stmt' argument is quite
legacy (I think it can
be completely purged everywhere)

> +    {
> +      unsigned int assumed_nunits = vect_nunits_for_cost (vectype);
> +      unsigned int inside_cost;
> +      inside_cost = record_stmt_cost (cost_vec, ncopies * assumed_nunits,
> +                                     scalar_load, stmt_info, 0, vect_body);
> +      if (dump_enabled_p ())
> +       dump_printf_loc (MSG_NOTE, vect_location,
> +                        "vect_model_load_cost: inside_cost = %d, "
> +                        "prologue_cost = 0 .\n",
> +                        inside_cost);
> +      return;
> +    }
> +
>    tree arglist = TYPE_ARG_TYPES (TREE_TYPE (gs_info->decl));
>    tree rettype = TREE_TYPE (TREE_TYPE (gs_info->decl));
>    tree srctype = TREE_VALUE (arglist); arglist = TREE_CHAIN (arglist);
> @@ -9483,13 +9503,8 @@ vectorizable_load (vec_info *vinfo,
>
>    if (memory_access_type == VMAT_GATHER_SCATTER && gs_info.decl)
>      {
> -      if (costing_p)
> -       vect_model_load_cost (vinfo, stmt_info, ncopies, vf, memory_access_type,
> -                             alignment_support_scheme, misalignment, &gs_info,
> -                             slp_node, cost_vec);
> -      else
> -       vect_build_gather_load_calls (vinfo, stmt_info, gsi, vec_stmt, &gs_info,
> -                                     mask);
> +      vect_build_gather_load_calls (vinfo, stmt_info, gsi, vec_stmt, &gs_info,
> +                                   mask, cost_vec);
>        return true;
>      }
>
> --
> 2.31.1
>
diff mbox series

Patch

diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 44514658be3..744cdf40e26 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -1135,6 +1135,8 @@  vect_model_load_cost (vec_info *vinfo,
 		      slp_tree slp_node,
 		      stmt_vector_for_cost *cost_vec)
 {
+  gcc_assert (memory_access_type != VMAT_GATHER_SCATTER || !gs_info->decl);
+
   unsigned int inside_cost = 0, prologue_cost = 0;
   bool grouped_access_p = STMT_VINFO_GROUPED_ACCESS (stmt_info);
 
@@ -2819,7 +2821,8 @@  vect_build_gather_load_calls (vec_info *vinfo, stmt_vec_info stmt_info,
 			      gimple_stmt_iterator *gsi,
 			      gimple **vec_stmt,
 			      gather_scatter_info *gs_info,
-			      tree mask)
+			      tree mask,
+			      stmt_vector_for_cost *cost_vec)
 {
   loop_vec_info loop_vinfo = dyn_cast <loop_vec_info> (vinfo);
   class loop *loop = LOOP_VINFO_LOOP (loop_vinfo);
@@ -2831,6 +2834,23 @@  vect_build_gather_load_calls (vec_info *vinfo, stmt_vec_info stmt_info,
   poly_uint64 gather_off_nunits
     = TYPE_VECTOR_SUBPARTS (gs_info->offset_vectype);
 
+  /* FIXME: Keep the previous costing way in vect_model_load_cost by costing
+     N scalar loads, but it should be tweaked to use target specific costs
+     on related gather load calls.  */
+  if (!vec_stmt)
+    {
+      unsigned int assumed_nunits = vect_nunits_for_cost (vectype);
+      unsigned int inside_cost;
+      inside_cost = record_stmt_cost (cost_vec, ncopies * assumed_nunits,
+				      scalar_load, stmt_info, 0, vect_body);
+      if (dump_enabled_p ())
+	dump_printf_loc (MSG_NOTE, vect_location,
+			 "vect_model_load_cost: inside_cost = %d, "
+			 "prologue_cost = 0 .\n",
+			 inside_cost);
+      return;
+    }
+
   tree arglist = TYPE_ARG_TYPES (TREE_TYPE (gs_info->decl));
   tree rettype = TREE_TYPE (TREE_TYPE (gs_info->decl));
   tree srctype = TREE_VALUE (arglist); arglist = TREE_CHAIN (arglist);
@@ -9483,13 +9503,8 @@  vectorizable_load (vec_info *vinfo,
 
   if (memory_access_type == VMAT_GATHER_SCATTER && gs_info.decl)
     {
-      if (costing_p)
-	vect_model_load_cost (vinfo, stmt_info, ncopies, vf, memory_access_type,
-			      alignment_support_scheme, misalignment, &gs_info,
-			      slp_node, cost_vec);
-      else
-	vect_build_gather_load_calls (vinfo, stmt_info, gsi, vec_stmt, &gs_info,
-				      mask);
+      vect_build_gather_load_calls (vinfo, stmt_info, gsi, vec_stmt, &gs_info,
+				    mask, cost_vec);
       return true;
     }