[PATCH 1/2][vect]PR 88915: Vectorize epilogues when versioning loops

[Richard Biener]

On Fri, 25 Oct 2019, Andre Vieira (lists) wrote:

> 
> 
> On 22/10/2019 14:56, Richard Biener wrote:
> > On Tue, 22 Oct 2019, Andre Vieira (lists) wrote:
> > 
> >> Hi Richi,
> >>
> >> See inline responses to your comments.
> >>
> >> On 11/10/2019 13:57, Richard Biener wrote:
> >>> On Thu, 10 Oct 2019, Andre Vieira (lists) wrote:
> >>>
> >>>> Hi,
> >>>>
> >>
> >>>
> >>> +
> >>> +  /* Keep track of vector sizes we know we can vectorize the epilogue
> >>> with.  */
> >>> +  vector_sizes epilogue_vsizes;
> >>>    };
> >>>
> >>> please don't enlarge struct loop, instead track this somewhere
> >>> in the vectorizer (in loop_vinfo?  I see you already have
> >>> epilogue_vinfos there - so the loop_vinfo simply lacks
> >>> convenient access to the vector_size?)  I don't see any
> >>> use that could be trivially adjusted to look at a loop_vinfo
> >>> member instead.
> >>
> >> Done.
> >>>
> >>> For the vect_update_inits_of_drs this means that we'd possibly
> >>> do less CSE.  Not sure if really an issue.
> >>
> >> CSE of what exactly? You are afraid we are repeating a calculation here we
> >> have done elsewhere before?
> > 
> > All uses of those inits now possibly get the expression instead of
> > just the SSA name we inserted code for once.  But as said, we'll see.
> > 
> 
> This code changed after some comments from Richard Sandiford.
> 
> > +  /* We are done vectorizing the main loop, so now we update the
> > epilogues
> > +     stmt_vec_info's.  At the same time we set the gimple UID of each
> > +     statement in the epilogue, as these are used to look them up in the
> > +     epilogues loop_vec_info later.  We also keep track of what
> > +     stmt_vec_info's have PATTERN_DEF_SEQ's and RELATED_STMT's that might
> > +     need updating and we construct a mapping between variables defined
> > in
> > +     the main loop and their corresponding names in epilogue.  */
> > +  for (unsigned i = 0; i < epilogue->num_nodes; ++i)
> > 
> > so for the following code I wonder if you can make use of the
> > fact that loop copying also copies UIDs, so you should be able
> > to match stmts via their UIDs and get at the other loop infos
> > stmt_info by the copy loop stmt UID.
> > 
> > I wonder why you need no modification for the SLP tree?
> > 
> I checked with Tamar and the SLP tree works with the position of operands and
> not SSA_NAMES.  So we should be fine.

There's now SLP_TREE_SCALAR_OPS but only for invariants so I guess
we should indeed be fine here.  Everything else is already
stmt_infos which you patch with the new underlying stmts.

Richard.