> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Wednesday, July 30, 2025 12:17 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Tamar Christina <tamar.christ...@arm.com>
> Subject: [PATCH 2/2] Put SLP_TREE_SIMD_CLONE_INFO into type specifc data
> 
> The following adds vect_simd_clone_data as a container for vect
> type specific data for vectorizable_simd_clone_call and moves
> SLP_TREE_SIMD_CLONE_INFO there.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> This time for a simpler candidate (and more immediately useful
> since it shrinks the SLP node size)
> 
> I suppose this is somewhat better - do you have a better suggestion
> for the dances around the data object creation upon analysis success?

I've only checked that this compiles, but how about something like

> 
> Thanks,
> Richard.
> 
>       * tree-vectorizer.h (vect_simd_clone_data): New.
>       (_slp_tree::simd_clone_info): Remove.
>       (SLP_TREE_SIMD_CLONE_INFO): Likewise.
>       * tree-vect-slp.cc (_slp_tree::_slp_tree): Adjust.
>       (_slp_tree::~_slp_tree): Likewise.
>       * tree-vect-stmts.cc (vectorizable_simd_clone_call): Use
>       tyupe specific data to store SLP_TREE_SIMD_CLONE_INFO.
> ---
>  gcc/tree-vect-slp.cc   |  2 --
>  gcc/tree-vect-stmts.cc |  8 +++++---
>  gcc/tree-vectorizer.h  | 22 ++++++++++++++++------
>  3 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
> index 233543214fa..4038fe84e87 100644
> --- a/gcc/tree-vect-slp.cc
> +++ b/gcc/tree-vect-slp.cc
> @@ -118,7 +118,6 @@ _slp_tree::_slp_tree ()
>    SLP_TREE_CHILDREN (this) = vNULL;
>    SLP_TREE_LOAD_PERMUTATION (this) = vNULL;
>    SLP_TREE_LANE_PERMUTATION (this) = vNULL;
> -  SLP_TREE_SIMD_CLONE_INFO (this) = vNULL;
>    SLP_TREE_DEF_TYPE (this) = vect_uninitialized_def;
>    SLP_TREE_CODE (this) = ERROR_MARK;
>    this->ldst_lanes = false;
> @@ -150,7 +149,6 @@ _slp_tree::~_slp_tree ()
>    SLP_TREE_VEC_DEFS (this).release ();
>    SLP_TREE_LOAD_PERMUTATION (this).release ();
>    SLP_TREE_LANE_PERMUTATION (this).release ();
> -  SLP_TREE_SIMD_CLONE_INFO (this).release ();
>    if (this->failed)
>      free (failed);
>    if (this->data)
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 5d34fcd193b..d9ff7fe7960 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -3873,9 +3873,10 @@ vectorizable_simd_clone_call (vec_info *vinfo,
> stmt_vec_info stmt_info,
>    if (nargs == 0)
>      return false;
> 
> -  vec<tree>& simd_clone_info = SLP_TREE_SIMD_CLONE_INFO (slp_node);
> -  if (cost_vec)
> -    simd_clone_info.truncate (0);
> +  vect_simd_clone_data _data;
> +  vec<tree>& simd_clone_info
> +    = cost_vec ? _data.simd_clone_info
> +      : static_cast <vect_simd_clone_data *> 
> (slp_node->data)->simd_clone_info;

What if we put the conditional extraction on the creation of _data.  So 
something like:

  vect_simd_clone_data *_data;
  slp_get_vect_data (cost_vec, slp_node, _data);
  vec<tree>& simd_clone_info = _data->simd_clone_info;

PS. Could we do the extraction based on whether slp_node->data is null or not?
       then we don't need the cost_vec parameter?

The helper in tree-vectorizer.h is then

template <typename T>
inline void slp_get_vect_data (bool costing_p, slp_tree slp_node, T& refvalue)
{
  if (costing_p && slp_node->data)
    {
      refvalue = static_cast<T>(slp_node->data);
      return;
    }

  refvalue = T{};
}
So we move the instantiation into the helper instead.

>    arginfo.reserve (nargs, true);
>    auto_vec<slp_tree> slp_op;
>    slp_op.safe_grow_cleared (nargs);
> @@ -4272,6 +4273,7 @@ vectorizable_simd_clone_call (vec_info *vinfo,
> stmt_vec_info stmt_info,
>       }
> 
>        SLP_TREE_TYPE (slp_node) = call_simd_clone_vec_info_type;
> +      slp_node->data = new vect_simd_clone_data (_data);

And then this becomes 

slp_node->data = _data;

but we have to delete _data on failure.  That or have  the default case assign
it to slp_tree and have its destructor take care of it?

so something like

template <typename T>
inline void slp_get_vect_data (bool costing_p, slp_tree slp_node, T& refvalue)
{
  if (costing_p && slp_node->data)
    {
      refvalue = static_cast<T>(slp_node->data);
      return;
    }

  refvalue = T{};
  slp_node->data = refvalue;
}
So we don't have to worry about the delete?

>        DUMP_VECT_SCOPE ("vectorizable_simd_clone_call");
>  /*      vect_model_simple_cost (vinfo, 1, slp_node, cost_vec); */
>        return true;
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index ad27577cc55..56ba705fa39 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -243,6 +243,22 @@ struct vect_data {
>    virtual ~vect_data () = default;
>  };
> 
> +/* Analysis data from vectorizable_simd_clone_call for
> +   call_simd_clone_vec_info_type.  */
> +struct vect_simd_clone_data : vect_data {
> +  virtual ~vect_simd_clone_data () = default;
> +  vect_simd_clone_data () = default;
> +  vect_simd_clone_data (vect_simd_clone_data &other)
> +  {
> +    simd_clone_info = std::move (other.simd_clone_info);
> +  }
> +

I think we can have the compiler generate this right? Using

vect_simd_clone_data(vect_simd_clone_data&& other) = default;

so we don't have to move the members individually.

Would something like this work?

Thanks,
Tamar

> +  /* Selected SIMD clone's function info.  First vector element
> +     is SIMD clone's function decl, followed by a pair of trees (base + step)
> +     for linear arguments (pair of NULLs for other arguments).  */
> +  auto_vec<tree> simd_clone_info;
> +};
> +
>  /* A computation tree of an SLP instance.  Each node corresponds to a group 
> of
>     stmts to be packed in a SIMD stmt.  */
>  struct _slp_tree {
> @@ -271,11 +287,6 @@ struct _slp_tree {
>       denotes the number of output lanes.  */
>    lane_permutation_t lane_permutation;
> 
> -  /* Selected SIMD clone's function info.  First vector element
> -     is SIMD clone's function decl, followed by a pair of trees (base + step)
> -     for linear arguments (pair of NULLs for other arguments).  */
> -  vec<tree> simd_clone_info;
> -
>    tree vectype;
>    /* Vectorized defs.  */
>    vec<tree> vec_defs;
> @@ -392,7 +403,6 @@ public:
>  #define SLP_TREE_NUMBER_OF_VEC_STMTS(S)          (S)->vec_stmts_size
>  #define SLP_TREE_LOAD_PERMUTATION(S)             (S)->load_permutation
>  #define SLP_TREE_LANE_PERMUTATION(S)             (S)->lane_permutation
> -#define SLP_TREE_SIMD_CLONE_INFO(S)              (S)->simd_clone_info
>  #define SLP_TREE_DEF_TYPE(S)                  (S)->def_type
>  #define SLP_TREE_VECTYPE(S)                   (S)->vectype
>  #define SLP_TREE_REPRESENTATIVE(S)            (S)->representative
> --
> 2.43.0

Reply via email to