On Wed, 30 Oct 2019, Joel Hutton wrote:
> 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]};
> }
Huh. On what architecture? Is that for a V2DI constructor of
V1DI vectors maybe? On x86 the code doesn't trigger.
The condition was indeed to check for vector mode elements so
maybe it should simply read
if (VECTOR_MODE_P (emode))
? eltmode is always scalar.
>
> >> * 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?
The type of the constructor itself.
> > 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.
> >>
> >>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
