> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: Thursday, July 31, 2025 3:09 PM
> To: [email protected]
> Cc: Tamar Christina <[email protected]>
> 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.
> 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?
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
