Richard,
I checked one of the tests designed for epilogue vectorization using
patches 1 - 3 and found out that build compiler performs vectorization
of epilogues with --param vect-epilogues-nomask=1 passed:
$ gcc -Ofast -mavx2 t1.c -S --param vect-epilogues-nomask=1 -o
t1.new-nomask.s -fdump-tree-vect-details
$ grep VECTORIZED -c t1.c.156t.vect
4
Without param only 2 loops are vectorized.
Should I simply add a part of tests related to this feature or I must
delete all not necessary changes also?
Thanks.
Yuri.
2016-11-14 16:40 GMT+03:00 Richard Biener <rguent...@suse.de>:
> On Mon, 14 Nov 2016, Yuri Rumyantsev wrote:
>
>> Richard,
>>
>> In my previous patch I forgot to remove couple lines related to aux field.
>> Here is the correct updated patch.
>
> Yeah, I noticed. This patch would be ok for trunk (together with
> necessary parts from 1 and 2) if all not required parts are removed
> (and you'd add the testcases covering non-masked tail vect).
>
> Thus, can you please produce a single complete patch containing only
> non-masked epilogue vectoriziation?
>
> Thanks,
> Richard.
>
>> Thanks.
>> Yuri.
>>
>> 2016-11-14 15:51 GMT+03:00 Richard Biener <rguent...@suse.de>:
>> > On Fri, 11 Nov 2016, Yuri Rumyantsev wrote:
>> >
>> >> Richard,
>> >>
>> >> I prepare updated 3 patch with passing additional argument to
>> >> vect_analyze_loop as you proposed (untested).
>> >>
>> >> You wrote:
>> >> tw, I wonder if you can produce a single patch containing just
>> >> epilogue vectorization, that is combine patches 1-3 but rip out
>> >> changes only needed by later patches?
>> >>
>> >> Did you mean that I exclude all support for vectorization epilogues,
>> >> i.e. exclude from 2-nd patch all non-related changes
>> >> like
>> >>
>> >> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
>> >> index 11863af..32011c1 100644
>> >> --- a/gcc/tree-vect-loop.c
>> >> +++ b/gcc/tree-vect-loop.c
>> >> @@ -1120,6 +1120,12 @@ new_loop_vec_info (struct loop *loop)
>> >> LOOP_VINFO_PEELING_FOR_GAPS (res) = false;
>> >> LOOP_VINFO_PEELING_FOR_NITER (res) = false;
>> >> LOOP_VINFO_OPERANDS_SWAPPED (res) = false;
>> >> + LOOP_VINFO_CAN_BE_MASKED (res) = false;
>> >> + LOOP_VINFO_REQUIRED_MASKS (res) = 0;
>> >> + LOOP_VINFO_COMBINE_EPILOGUE (res) = false;
>> >> + LOOP_VINFO_MASK_EPILOGUE (res) = false;
>> >> + LOOP_VINFO_NEED_MASKING (res) = false;
>> >> + LOOP_VINFO_ORIG_LOOP_INFO (res) = NULL;
>> >
>> > Yes.
>> >
>> >> Did you mean also that new combined patch must be working patch, i.e.
>> >> can be integrated without other patches?
>> >
>> > Yes.
>> >
>> >> Could you please look at updated patch?
>> >
>> > Will do.
>> >
>> > Thanks,
>> > Richard.
>> >
>> >> Thanks.
>> >> Yuri.
>> >>
>> >> 2016-11-10 15:36 GMT+03:00 Richard Biener <rguent...@suse.de>:
>> >> > On Thu, 10 Nov 2016, Richard Biener wrote:
>> >> >
>> >> >> 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?
>> >> >
>> >> > Btw, I wonder if you can produce a single patch containing just
>> >> > epilogue vectorization, that is combine patches 1-3 but rip out
>> >> > changes only needed by later patches?
>> >> >
>> >> > Thanks,
>> >> > Richard.
>> >> >
>> >> >> Richard.
>> >> >>
>> >> >> > 2016-11-08 15:38 GMT+03:00 Richard Biener <rguent...@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 <rguent...@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 <rguent...@suse.de>
>> >> >> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham
>> >> >> > > Norton, HRB 21284 (AG Nuernberg)
>> >> >> >
>> >> >>
>> >> >>
>> >> >
>> >> > --
>> >> > Richard Biener <rguent...@suse.de>
>> >> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton,
>> >> > HRB 21284 (AG Nuernberg)
>> >>
>> >
>> > --
>> > Richard Biener <rguent...@suse.de>
>> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
>> > 21284 (AG Nuernberg)
>>
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
> 21284 (AG Nuernberg)
