On 30/10/2019 13:49, Richard Biener wrote:
> Why do you need this? The vectorizer already creates such CTORs. Any
> testcase that you can show?
typedef long v2di __attribute__((vector_size(16)));
v2di v;
void
foo()
{
v = (v2di){v[1], v[0]};
}
>> * tree-vect-slp.c (vect_analyze_slp_instance): Add case for vector
>> constructors.
>> (vect_bb_slp_scalar_cost): Likewise.
>> (vect_slp_check_for_constructors): New function.
>> (vect_slp_analyze_bb_1): Add check for vector constructors.
>> (vect_schedule_slp_instance): Add case to fixup vector constructor
>> stmt.
>> (vectorize_slp_instance_root_stmt): New function
>> * tree-vectorizer.h (SLP_INSTANCE_ROOT_STMT): New field.
> + SLP_INSTANCE_ROOT_STMT (new_instance) = constructor ?
> stmt_info->stmt\
> + : NULL;
>
> SLP_INSTANCE_ROOT_STMT should be a stmt_vec_info I guess...
>
> @@ -2801,6 +2830,10 @@ vect_bb_slp_scalar_cost (basic_block bb,
> stmt_vec_info use_stmt_info = vinfo->lookup_stmt
> (use_stmt);
> if (!use_stmt_info || !PURE_SLP_STMT (use_stmt_info))
> {
> + /* Check this is not a constructor that will be
> vectorized
> + away. */
> + if (BB_VINFO_GROUPED_STORES (vinfo).contains
> (use_stmt_info))
> + continue;
> (*life)[i] = true;
>
> ... which you then simply mark as PURE_SLP_STMT where we call
> vect_mark_slp_stmts in vect_slp_analyze_bb_1.
>
> I see you have the TYPE_VECTOR_SUBPARTS check still at transform
> stage in vectorize_slp_instance_root_stmt, please simply move
> it to vect_slp_check_for_constructors or to vect_analyze_slp_instance
> where you have the other rejects (non-SSA names in the ctor).
If the check is in vect_slp_check_for_constructors, which vector is used
as the input to tYPE_VECTOR_SUBPARTS, the lhs of the constructor?
> That is, vectorize_slp_instance_root_stmt may not fail.
>
> +bool
> +vectorize_slp_instance_root_stmt (slp_tree node, slp_instance instance)
> +{
> +
>
> extra newline
>
> + if (SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1)
>
> the stmt replacing can be commonized between == 1 and > 1 cases.
>
> FOR_EACH_VEC_ELT (slp_instances, i, instance)
> {
> + slp_tree node = SLP_INSTANCE_TREE (instance);
> /* Schedule the tree of INSTANCE. */
> - vect_schedule_slp_instance (SLP_INSTANCE_TREE (instance),
> + vect_schedule_slp_instance (node,
> instance, bst_map);
> +
> + /* Vectorize the instance root. */
> + if (instance->root == node && SLP_INSTANCE_ROOT_STMT (instance)
> + && SLP_TREE_VEC_STMTS (node).exists ())
> + if (!vectorize_slp_instance_root_stmt (node, instance))
> + {
>
> instance->root == node is always true. Likewise
> SLP_TREE_VEC_STMTS (node).exists ().
>
> @@ -4061,15 +4201,42 @@ vect_schedule_slp (vec_info *vinfo)
> if (is_a <loop_vec_info> (vinfo))
>
> the ChangeLog doesn't mention this. I guess this isn't necessary
> unless you fail vectorizing late which you shouldn't.
>
> diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
> index
> 56be28b0cc5a77412f996e70636b08d5b615813e..9f8419e4208b7d438ace41892022f93ebcadd019
> 100644
> --- a/gcc/tree-vectorizer.h
> +++ b/gcc/tree-vectorizer.h
> @@ -151,6 +151,10 @@ public:
> /* The root of SLP tree. */
> slp_tree root;
>
> + /* For vector constructors, the constructor stmt that the SLP tree is
> built
> + from, NULL otherwise. */
> + gimple *root_stmt;
>
> as said, make this a stmt_vec_info
>
> Thanks,
> Richard.
>
>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-10-10 Joel Hutton <joel.hut...@arm.com>
>>
>> * gcc.dg/vect/bb-slp-40.c: New test.
>> * gcc.dg/vect/bb-slp-41.c: New test.
>>
>>
