> 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.
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