From: Richard Biener <rguenther@suse.de>
To: Yuri Rumyantsev <ysrumyan@gmail.com>
Cc: Jeff Law <law@redhat.com>, gcc-patches <gcc-patches@gcc.gnu.org>,
Ilya Enkovich <enkovich.gnu@gmail.com>
Subject: Re: [PATCH, vec-tails] Support loop epilogue vectorization
Date: Thu, 10 Nov 2016 12:36:00 -0000 [thread overview]
Message-ID: <alpine.LSU.2.11.1611101335210.5294@t29.fhfr.qr> (raw)
In-Reply-To: <alpine.LSU.2.11.1611101329050.5294@t29.fhfr.qr>
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 <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)
next prev parent reply other threads:[~2016-11-10 12:36 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-01 12:38 Yuri Rumyantsev
2016-11-02 12:27 ` Richard Biener
2016-11-03 12:33 ` Yuri Rumyantsev
2016-11-08 12:39 ` Richard Biener
2016-11-08 14:17 ` Yuri Rumyantsev
2016-11-10 12:34 ` Richard Biener
2016-11-10 12:36 ` Richard Biener [this message]
2016-11-11 11:15 ` Yuri Rumyantsev
2016-11-11 14:15 ` Yuri Rumyantsev
2016-11-11 14:43 ` Yuri Rumyantsev
2016-11-14 12:56 ` Richard Biener
2016-11-14 12:51 ` Richard Biener
2016-11-14 13:30 ` Yuri Rumyantsev
2016-11-14 13:41 ` Richard Biener
2016-11-14 15:39 ` Yuri Rumyantsev
2016-11-14 17:59 ` Richard Biener
2016-11-15 14:42 ` Yuri Rumyantsev
2016-11-16 9:56 ` Richard Biener
2016-11-18 13:20 ` Christophe Lyon
2016-11-18 15:46 ` Yuri Rumyantsev
2016-11-18 15:54 ` Christophe Lyon
2016-11-24 13:42 ` Yuri Rumyantsev
2016-11-28 14:39 ` Richard Biener
2016-11-28 16:57 ` Yuri Rumyantsev
2016-12-01 11:34 ` Richard Biener
2016-12-01 14:27 ` Yuri Rumyantsev
2016-12-01 14:46 ` Richard Biener
[not found] ` <CAEoMCqSkWgz+DJLe1M1CDxbk4LBtBU4r3rcVv7OcgpsGW4eTJA@mail.gmail.com>
[not found] ` <CAEoMCqRVVYTYWfhYrpi3TOuBe6XBw4ScVNstoqd8YShBsvRwMw@mail.gmail.com>
[not found] ` <CAEoMCqTdOHO_OxJ-5gxDJRPQDS+9kYkKd+WdgGJz8WMuUzD61A@mail.gmail.com>
[not found] ` <CAEoMCqQ5ZaT6TPbDL37DOZCEF5DHKWx995yn2fQZO3kV+vQ+EA@mail.gmail.com>
[not found] ` <CAEoMCqTCaRQU-mia98uX00CtpKA9w03fhaR2hXCdywXuVAQmSw@mail.gmail.com>
[not found] ` <CAEoMCqST8pOZmndKKuYWSyD=juPdGG1UAJ6NyAV3qkuxjV+3gA@mail.gmail.com>
[not found] ` <alpine.LSU.2.11.1612131455080.5294@t29.fhfr.qr>
2016-12-21 10:14 ` Yuri Rumyantsev
2016-12-21 17:23 ` Yuri Rumyantsev
2016-11-29 16:22 ` Christophe Lyon
2016-11-05 18:35 ` Jeff Law
2016-11-06 11:16 ` Richard Biener
2016-11-09 10:37 ` Bin.Cheng
2016-11-09 11:28 ` Yuri Rumyantsev
2016-11-09 11:46 ` Bin.Cheng
2016-11-09 12:12 ` Yuri Rumyantsev
2016-11-09 12:40 ` Bin.Cheng
2016-11-09 12:52 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LSU.2.11.1611101335210.5294@t29.fhfr.qr \
--to=rguenther@suse.de \
--cc=enkovich.gnu@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=ysrumyan@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).