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