On Thu, 22 Oct 2020, Tamar Christina wrote:
> Hi Richi,
>
> > -----Original Message-----
> > From: rguent...@c653.arch.suse.de <rguent...@c653.arch.suse.de> On
> > Behalf Of Richard Biener
> > Sent: Thursday, October 22, 2020 9:44 AM
> > To: Tamar Christina <tamar.christ...@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; o...@ucw.cz
> > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late
> >
> > On Wed, 21 Oct 2020, Tamar Christina wrote:
> >
> > > Hi All,
> > >
> > > This moves the code that checks for load/store lanes further in the
> > > pipeline and places it after slp_optimize. This would allow us to
> > > perform optimizations on the SLP tree and only bail out if we really have
> > > a
> > permute.
> > >
> > > With this change it allows us to handle permutes such as {1,1,1,1}
> > > which should be handled by a load and replicate.
> > >
> > > This change however makes it all or nothing. Either all instances can
> > > be handled or none at all. This is why some of the test cases have been
> > adjusted.
> >
> > So this possibly leaves a loop unvectorized in case there's a ldN/stN
> > opportunity but another SLP instance with a permutation not handled by
> > interleaving is present. What I was originally suggesting is to only
> > cancel the
> > SLP build if _all_ instances can be handled with ldN/stN.
>
> Ah I had somehow misunderstood this. I'll make that change.
>
> >
> > Of course I'm also happy with completely removing this heuristics.
> >
> > Note some of the comments look off now, also the assignment to ok before
> > the goto is pointless and you should probably turn this into a dump print
> > instead.
>
> Is it pointless? The first thing again does is assert:
>
> again:
> /* Ensure that "ok" is false (with an opt_problem if dumping is enabled).
> */
> gcc_assert (!ok);
>
> and up to that point ok would still be it's default value of
> opt_result::success ();
>
> So since I have to change it anyway I was using it to report the reason.
Ah, OK. Though the opt_problem isn't ever reported when originating from
this piece.
> I can make it into a dump, but would still have to assign to `ok` unless I'm
> missing
> something.
True. I guess there should be some kind of hint why we decided to disable
SLP, so dumping sth would be appreciated.
Richard.
>
> Regards,
> Tamar
>
> >
> > Thanks,
> > Richard.
> >
> > > Bootstrapped Regtested on aarch64-none-linux-gnu, -x86_64-pc-linux-gnu
> > > and no issues.
> > >
> > > Ok for master?
> >
> >
> >
> > > Thanks,
> > > Tamar
> > >
> > > gcc/ChangeLog:
> > >
> > > * tree-vect-slp.c (vect_analyze_slp_instance): Moved load/store
> > lanes
> > > check to ...
> > > * tree-vect-loop.c (vect_analyze_loop_2): ..Here
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * gcc.dg/vect/slp-11b.c: Update output scan.
> > > * gcc.dg/vect/slp-perm-6.c: Likewise.
> > >
> > >
> >
> > --
> > Richard Biener <rguent...@suse.de>
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > Nuernberg, Germany; GF: Felix Imend
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
