[PATCH] SLP: Move load/store-lanes check till late

[Richard Biener]

On Tue, 3 Nov 2020, Tamar Christina wrote:

> Hi Richi,
> 
> We decided to take the regression in any code-gen this could
> give and fix it properly next stage-1.  As such here's a new
> patch based on your previous feedback.
> 
> Ok for master?

Looks good sofar but be aware that you elide the

-             && vect_store_lanes_supported
-                  (STMT_VINFO_VECTYPE (scalar_stmts[0]), group_size,
false))

part of the check - that is, you don't verify the store part
of the instance can use store-lanes.  Btw, this means the original
code cancelled an instance only when the SLP graph entry is a
store-lane capable store but your variant would also cancel in
case there's a load-lane capable reduction.

I think that you eventually want to re-instantiate the
store-lane check but treat it the same as any of the load checks
(thus not require all instances to be stores for the cancellation).
But at least when a store cannot use store-lanes we probably shouldn't
cancel the SLP.

Anyway, the patch is OK for master.  The store-lane check part can
be re-added as followup.

Thanks,
Richard.

> 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.
> 
> > -----Original Message-----
> > From: rguenther@c653.arch.suse.de <rguenther@c653.arch.suse.de> On
> > Behalf Of Richard Biener
> > Sent: Thursday, October 22, 2020 9:44 AM
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; ook@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.
> > 
> > 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.
> > 
> > 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 <rguenther@suse.de>
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > Nuernberg, Germany; GF: Felix Imend
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend