> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: Friday, August 1, 2025 6:06 AM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH 2/2] Put SLP_TREE_SIMD_CLONE_INFO into type specifc data
>
>
>
> > Am 31.07.2025 um 17:43 schrieb Tamar Christina <tamar.christ...@arm.com>:
> >
> >
> >>
> >> -----Original Message-----
> >> From: Richard Biener <rguent...@suse.de>
> >> Sent: Thursday, July 31, 2025 3:09 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.
> >>
> >> Any further considerations?
> >>
> >
> > Looks much better now!
> >
> >> 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 | 7 ++++---
> >> gcc/tree-vectorizer.h | 19 +++++++++++++------
> >> 3 files changed, 17 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 3fa4585160c..24a3030bd9f 100644
> >> --- a/gcc/tree-vect-stmts.cc
> >> +++ b/gcc/tree-vect-stmts.cc
> >> @@ -3892,9 +3892,9 @@ 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;
> >> + vect_simd_clone_data &data = slp_node->get_data (_data);
> >> + vec<tree>& simd_clone_info = data.simd_clone_info;
> >
> > Yeah it's indeed a shame we can't avoid the local, we can always hide the
> sequence
> > Behind a macro, something like
> >
> > EXPAND_SLP_DATA (data, slp_node) which expands to
> >
> > vect_simd_clone_data _data;
> > vect_simd_clone_data &data = slp_node->get_data (_data);
> >
> > but I honestly don't mind seeing the expansion explicitly.
>
> Yeah. In case we split the function into a analysis and transform one this
> tiny
> ugliness would go away.
>
> >> arginfo.reserve (nargs, true);
> >> auto_vec<slp_tree> slp_op;
> >> slp_op.safe_grow_cleared (nargs);
> >> @@ -4291,6 +4291,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 (std::move (_data));
> >
> > Do we still need the std::move here? doesn't the move constructor already
> > take care of it?
>
> Without it it complains I call the deleted non-move CTOR, so apparently it is
> necessary.
>
Ah because it's an lvalue and move constructors take only rvalues. Fair enough.
Looks good to me FWIW!
Thanks,
Tamar
> Richard
>
> > Otherwise looks good to me.
> >
> > Thanks for the cleanup!
> >
> > Tamar
> >
> >> 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 63ba7c1e8f5..b8312b8d6bd 100644
> >> --- a/gcc/tree-vectorizer.h
> >> +++ b/gcc/tree-vectorizer.h
> >> @@ -243,6 +243,19 @@ 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) = default;
> >> +
> >> + /* 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 +284,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;
> >> @@ -395,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
