[PATCH, vec-tails] Support loop epilogue vectorization

[Richard Biener]

On Tue, 8 Nov 2016, Yuri Rumyantsev wrote:

> Richard,
> 
> Here is updated 3 patch.
> 
> I checked that all new tests related to epilogue vectorization passed with it.
> 
> Your comments will be appreciated.

A lot better now.  Instead of the ->aux dance I now prefer to
pass the original loops loop_vinfo to vect_analyze_loop as
optional argument (if non-NULL we analyze the epilogue of that 
loop_vinfo).  OTOH I remember we mainly use it to get at the
original vectorization factor?  So we can pass down an (optional)
forced vectorization factor as well?

Richard.

> 2016-11-08 15:38 GMT+03:00 Richard Biener <rguenther@suse.de>:
> > On Thu, 3 Nov 2016, Yuri Rumyantsev wrote:
> >
> >> Hi Richard,
> >>
> >> I did not understand your last remark:
> >>
> >> > That is, here (and avoid the FOR_EACH_LOOP change):
> >> >
> >> > @@ -580,12 +586,21 @@ vectorize_loops (void)
> >> >           && dump_enabled_p ())
> >> >           dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
> >> >                            "loop vectorized\n");
> >> > -       vect_transform_loop (loop_vinfo);
> >> > +       new_loop = vect_transform_loop (loop_vinfo);
> >> >         num_vectorized_loops++;
> >> >        /* Now that the loop has been vectorized, allow it to be unrolled
> >> >           etc.  */
> >> >      loop->force_vectorize = false;
> >> >
> >> > +       /* Add new loop to a processing queue.  To make it easier
> >> > +          to match loop and its epilogue vectorization in dumps
> >> > +          put new loop as the next loop to process.  */
> >> > +       if (new_loop)
> >> > +         {
> >> > +           loops.safe_insert (i + 1, new_loop->num);
> >> > +           vect_loops_num = number_of_loops (cfun);
> >> > +         }
> >> >
> >> > simply dispatch to a vectorize_epilogue (loop_vinfo, new_loop)
> >> f> unction which will set up stuff properly (and also perform
> >> > the if-conversion of the epilogue there).
> >> >
> >> > That said, if we can get in non-masked epilogue vectorization
> >> > separately that would be great.
> >>
> >> Could you please clarify your proposal.
> >
> > When a loop was vectorized set things up to immediately vectorize
> > its epilogue, avoiding changing the loop iteration and avoiding
> > the re-use of ->aux.
> >
> > Richard.
> >
> >> Thanks.
> >> Yuri.
> >>
> >> 2016-11-02 15:27 GMT+03:00 Richard Biener <rguenther@suse.de>:
> >> > On Tue, 1 Nov 2016, Yuri Rumyantsev wrote:
> >> >
> >> >> Hi All,
> >> >>
> >> >> I re-send all patches sent by Ilya earlier for review which support
> >> >> vectorization of loop epilogues and loops with low trip count. We
> >> >> assume that the only patch - vec-tails-07-combine-tail.patch - was not
> >> >> approved by Jeff.
> >> >>
> >> >> I did re-base of all patches and performed bootstrapping and
> >> >> regression testing that did not show any new failures. Also all
> >> >> changes related to new vect_do_peeling algorithm have been changed
> >> >> accordingly.
> >> >>
> >> >> Is it OK for trunk?
> >> >
> >> > I would have prefered that the series up to -03-nomask-tails would
> >> > _only_ contain epilogue loop vectorization changes but unfortunately
> >> > the patchset is oddly separated.
> >> >
> >> > I have a comment on that part nevertheless:
> >> >
> >> > @@ -1608,7 +1614,10 @@ vect_enhance_data_refs_alignment (loop_vec_info
> >> > loop_vinfo)
> >> >    /* Check if we can possibly peel the loop.  */
> >> >    if (!vect_can_advance_ivs_p (loop_vinfo)
> >> >        || !slpeel_can_duplicate_loop_p (loop, single_exit (loop))
> >> > -      || loop->inner)
> >> > +      || loop->inner
> >> > +      /* Required peeling was performed in prologue and
> >> > +        is not required for epilogue.  */
> >> > +      || LOOP_VINFO_EPILOGUE_P (loop_vinfo))
> >> >      do_peeling = false;
> >> >
> >> >    if (do_peeling
> >> > @@ -1888,7 +1897,10 @@ vect_enhance_data_refs_alignment (loop_vec_info
> >> > loop_vinfo)
> >> >
> >> >    do_versioning =
> >> >         optimize_loop_nest_for_speed_p (loop)
> >> > -       && (!loop->inner); /* FORNOW */
> >> > +       && (!loop->inner) /* FORNOW */
> >> > +        /* Required versioning was performed for the
> >> > +          original loop and is not required for epilogue.  */
> >> > +       && !LOOP_VINFO_EPILOGUE_P (loop_vinfo);
> >> >
> >> >    if (do_versioning)
> >> >      {
> >> >
> >> > please do that check in the single caller of this function.
> >> >
> >> > Otherwise I still dislike the new ->aux use and I believe that simply
> >> > passing down info from the processed parent would be _much_ cleaner.
> >> > That is, here (and avoid the FOR_EACH_LOOP change):
> >> >
> >> > @@ -580,12 +586,21 @@ vectorize_loops (void)
> >> >             && dump_enabled_p ())
> >> >            dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, vect_location,
> >> >                             "loop vectorized\n");
> >> > -       vect_transform_loop (loop_vinfo);
> >> > +       new_loop = vect_transform_loop (loop_vinfo);
> >> >         num_vectorized_loops++;
> >> >         /* Now that the loop has been vectorized, allow it to be unrolled
> >> >            etc.  */
> >> >         loop->force_vectorize = false;
> >> >
> >> > +       /* Add new loop to a processing queue.  To make it easier
> >> > +          to match loop and its epilogue vectorization in dumps
> >> > +          put new loop as the next loop to process.  */
> >> > +       if (new_loop)
> >> > +         {
> >> > +           loops.safe_insert (i + 1, new_loop->num);
> >> > +           vect_loops_num = number_of_loops (cfun);
> >> > +         }
> >> >
> >> > simply dispatch to a vectorize_epilogue (loop_vinfo, new_loop)
> >> > function which will set up stuff properly (and also perform
> >> > the if-conversion of the epilogue there).
> >> >
> >> > That said, if we can get in non-masked epilogue vectorization
> >> > separately that would be great.
> >> >
> >> > I'm still torn about all the rest of the stuff and question its
> >> > usability (esp. merging the epilogue with the main vector loop).
> >> > But it has already been approved ... oh well.
> >> >
> >> > Thanks,
> >> > Richard.
> >>
> >>
> >
> > --
> > Richard Biener <rguenther@suse.de>
> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)