On 30/10/2019 13:49, Richard Biener wrote:
 >>
 >>          * expr.c (store_constructor): Add case for constructor of 
vectors.
 > 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]};
 > }
There is a special case for single element vectors, where the vector 
mode and
the element mode are the same.
I've changed this slightly, as your solution of using VECTOR_MODE_P didn't
work for my case where:
   emode = E_DImode
   eltmode = E_DImode
On aarch64. As E_DImode is not a vector mode.
Based on some checks I found in verify_gimple, I set the type of the 
replacement
constructor to the same as the original constructor, as verify_gimple 
seems to
expect flattened types. i.e. a vector of vectors of ints should have 
type vector
of ints.


 > What could be done is "compact" the
 > operands to vectorize to only contain SSA names, try to SLP
 > match and vectorize them and then combine the vectorized result
 > with the constant elements.
 >
 > Not worth doing I guess unless it's sth regular like
 >
 >  { a, b, c, d, 0, 0, 0, 0 }
 >
 > or so.  But this can be handled as followup if necessary.
 >
I agree, it should be possible to support this in future patches.


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


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


 >
 > +bool
 > +vectorize_slp_instance_root_stmt (slp_tree node, slp_instance instance)
 > +{
 > +
 >
 > extra newline
Fixed.


 > +  /* 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
Done.


 > +  if (SLP_TREE_NUMBER_OF_VEC_STMTS (node) == 1)
 >
 > the stmt replacing can be commonized between == 1 and > 1 cases.
Done.


 > +
 > +      /* 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 ().
Done.


 > @@ -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.
 >
It's necessary to avoid:

         removing stmts twice for constructors of the form {_1,_1,_1,_1}
         removing stmts that define ssa names used elsewhere (which 
previously wasn't a consideration because the scalar_stmts were stores 
to memory, not assignments to ssa names)

Updated changelog (and patch)

2019-10-31  Joel Hutton  <Joel.Hutton@arm.com>

         * expr.c (store_constructor): Modify to handle single element 
vectors.
         * tree-vect-slp.c (vect_analyze_slp_instance): Add case for 
vector constructors.
         (vect_slp_check_for_constructors): New function.
         (vect_slp_analyze_bb_1): Call new function to check for vector 
constructors.
         (vectorize_slp_instance_root_stmt): New function.
         (vect_schedule_slp): Call new function to vectorize root stmt 
of vector constructors.
         * tree-vectorizer.h (SLP_INSTANCE_ROOT_STMT): New field.

gcc/testsuite/ChangeLog:

2019-10-31  Joel Hutton  <Joel.Hutton@arm.com>

         * gcc.dg/vect/bb-slp-40.c: New test.
         * gcc.dg/vect/bb-slp-41.c: New test.