> -----Original Message-----
> From: Tamar Christina <tamar.christ...@arm.com>
> Sent: Thursday, July 31, 2025 7:41 AM
> To: Richard Biener <rguent...@suse.de>; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH 2/2] Put SLP_TREE_SIMD_CLONE_INFO into type specifc data
>
> > -----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)
Should be !costing_p but you get the idea 😊
Tamar
> {
> 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
