Re: [PATCH] SLP: Move load/store-lanes check till late
On Fri, 6 Nov 2020 at 12:33, Tamar Christina wrote: > > > > > -Original Message- > > From: Christophe Lyon > > Sent: Friday, November 6, 2020 10:27 AM > > To: Tamar Christina > > Cc: Richard Biener ; nd ; gcc- > > patc...@gcc.gnu.org; o...@ucw.cz > > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late > > > > On Fri, 6 Nov 2020 at 11:18, Tamar Christina > > wrote: > > > > > > > > > > > > > -Original Message- > > > > From: Christophe Lyon > > > > Sent: Friday, November 6, 2020 8:42 AM > > > > To: Tamar Christina > > > > Cc: Richard Biener ; nd ; gcc- > > > > patc...@gcc.gnu.org; o...@ucw.cz > > > > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late > > > > > > > > On Thu, 5 Nov 2020 at 11:21, Tamar Christina via Gcc-patches > > > patc...@gcc.gnu.org> wrote: > > > > > > > > > > > -Original Message- > > > > > > From: rguent...@c653.arch.suse.de > > > > > > On Behalf Of Richard Biener > > > > > > Sent: Thursday, November 5, 2020 10:17 AM > > > > > > To: Tamar Christina > > > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late > > > > > > > > > > > > On Wed, 4 Nov 2020, Tamar Christina wrote: > > > > > > > > > > > > > Hi Richi, > > > > > > > > > > > > > > > -Original Message- > > > > > > > > From: rguent...@c653.arch.suse.de > > > > > > > > On Behalf Of Richard Biener > > > > > > > > Sent: Wednesday, November 4, 2020 8:07 AM > > > > > > > > To: Tamar Christina > > > > > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till > > > > > > > > late > > > > > > > > > > > > > > > > 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 do still have it, > > > > > > > > > > > > > > if (loads_permuted > > > > > > > && vect_store_lanes_supported (vectype, group_size, > > > > > > > false)) > > > > > > > > > > > > > > I just grab the type from the SLP_TREE_VECTYPE (slp_root); > > > > > > > which should be the store if one exists. > > > > > > > > > > > > > > > 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
RE: [PATCH] SLP: Move load/store-lanes check till late
> -Original Message- > From: Christophe Lyon > Sent: Friday, November 6, 2020 10:27 AM > To: Tamar Christina > Cc: Richard Biener ; nd ; gcc- > patc...@gcc.gnu.org; o...@ucw.cz > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late > > On Fri, 6 Nov 2020 at 11:18, Tamar Christina > wrote: > > > > > > > > > -Original Message- > > > From: Christophe Lyon > > > Sent: Friday, November 6, 2020 8:42 AM > > > To: Tamar Christina > > > Cc: Richard Biener ; nd ; gcc- > > > patc...@gcc.gnu.org; o...@ucw.cz > > > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late > > > > > > On Thu, 5 Nov 2020 at 11:21, Tamar Christina via Gcc-patches > > patc...@gcc.gnu.org> wrote: > > > > > > > > > -Original Message- > > > > > From: rguent...@c653.arch.suse.de > > > > > On Behalf Of Richard Biener > > > > > Sent: Thursday, November 5, 2020 10:17 AM > > > > > To: Tamar Christina > > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late > > > > > > > > > > On Wed, 4 Nov 2020, Tamar Christina wrote: > > > > > > > > > > > Hi Richi, > > > > > > > > > > > > > -----Original Message- > > > > > > > From: rguent...@c653.arch.suse.de > > > > > > > On Behalf Of Richard Biener > > > > > > > Sent: Wednesday, November 4, 2020 8:07 AM > > > > > > > To: Tamar Christina > > > > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till > > > > > > > late > > > > > > > > > > > > > > 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 do still have it, > > > > > > > > > > > > if (loads_permuted > > > > > > && vect_store_lanes_supported (vectype, group_size, > > > > > > false)) > > > > > > > > > > > > I just grab the type from the SLP_TREE_VECTYPE (slp_root); > > > > > > which should be the store if one exists. > > > > > > > > > > > > > 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. > > > > > > > > > > > > I did however elide the kind check, that was added as part of > > > > > > the rebase, it looked like kind wasn't Being stored inside the > > > > > > SLP instance and > > > > > I'd have to redo the analysis to find it. > > > > > >
Re: [PATCH] SLP: Move load/store-lanes check till late
On Fri, 6 Nov 2020 at 11:18, Tamar Christina wrote: > > > > > -Original Message- > > From: Christophe Lyon > > Sent: Friday, November 6, 2020 8:42 AM > > To: Tamar Christina > > Cc: Richard Biener ; nd ; gcc- > > patc...@gcc.gnu.org; o...@ucw.cz > > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late > > > > On Thu, 5 Nov 2020 at 11:21, Tamar Christina via Gcc-patches > patc...@gcc.gnu.org> wrote: > > > > > > > -Original Message- > > > > From: rguent...@c653.arch.suse.de On > > > > Behalf Of Richard Biener > > > > Sent: Thursday, November 5, 2020 10:17 AM > > > > To: Tamar Christina > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late > > > > > > > > On Wed, 4 Nov 2020, Tamar Christina wrote: > > > > > > > > > Hi Richi, > > > > > > > > > > > -Original Message- > > > > > > From: rguent...@c653.arch.suse.de > > > > > > On Behalf Of Richard Biener > > > > > > Sent: Wednesday, November 4, 2020 8:07 AM > > > > > > To: Tamar Christina > > > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late > > > > > > > > > > > > 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 do still have it, > > > > > > > > > > if (loads_permuted > > > > > && vect_store_lanes_supported (vectype, group_size, > > > > > false)) > > > > > > > > > > I just grab the type from the SLP_TREE_VECTYPE (slp_root); which > > > > > should be the store if one exists. > > > > > > > > > > > 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. > > > > > > > > > > I did however elide the kind check, that was added as part of the > > > > > rebase, it looked like kind wasn't Being stored inside the SLP > > > > > instance and > > > > I'd have to redo the analysis to find it. > > > > > > > > > > Does it does reasonable to include kind as a field in the SLP > > > > > instance? > > > > > > > > > > > > > > > > > Anyway, the patch is OK for master. The store-lane check part > > > > > > can be re- added as followup. > > > > > > > > > > > > > > > > Thanks! Will do. > > > > > > > > Btw, the patch regressed > > > > > > > > FAIL: gcc.dg/vect/slp-11b.c -flto -ffat-lto-objects > > > > scan-tree-dump-times vect "vectorizing stmts using SLP" 1 > > > > FAIL: gcc.dg/vect/slp-11b.c scan-tree-dump-tim
RE: [PATCH] SLP: Move load/store-lanes check till late
> -Original Message- > From: Christophe Lyon > Sent: Friday, November 6, 2020 8:42 AM > To: Tamar Christina > Cc: Richard Biener ; nd ; gcc- > patc...@gcc.gnu.org; o...@ucw.cz > Subject: Re: [PATCH] SLP: Move load/store-lanes check till late > > On Thu, 5 Nov 2020 at 11:21, Tamar Christina via Gcc-patches patc...@gcc.gnu.org> wrote: > > > > > -Original Message- > > > From: rguent...@c653.arch.suse.de On > > > Behalf Of Richard Biener > > > Sent: Thursday, November 5, 2020 10:17 AM > > > To: Tamar Christina > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late > > > > > > On Wed, 4 Nov 2020, Tamar Christina wrote: > > > > > > > Hi Richi, > > > > > > > > > -Original Message- > > > > > From: rguent...@c653.arch.suse.de > > > > > On Behalf Of Richard Biener > > > > > Sent: Wednesday, November 4, 2020 8:07 AM > > > > > To: Tamar Christina > > > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz > > > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late > > > > > > > > > > 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 do still have it, > > > > > > > > if (loads_permuted > > > > && vect_store_lanes_supported (vectype, group_size, > > > > false)) > > > > > > > > I just grab the type from the SLP_TREE_VECTYPE (slp_root); which > > > > should be the store if one exists. > > > > > > > > > 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. > > > > > > > > I did however elide the kind check, that was added as part of the > > > > rebase, it looked like kind wasn't Being stored inside the SLP > > > > instance and > > > I'd have to redo the analysis to find it. > > > > > > > > Does it does reasonable to include kind as a field in the SLP instance? > > > > > > > > > > > > > > Anyway, the patch is OK for master. The store-lane check part > > > > > can be re- added as followup. > > > > > > > > > > > > > Thanks! Will do. > > > > > > Btw, the patch regressed > > > > > > FAIL: gcc.dg/vect/slp-11b.c -flto -ffat-lto-objects > > > scan-tree-dump-times vect "vectorizing stmts using SLP" 1 > > > FAIL: gcc.dg/vect/slp-11b.c scan-tree-dump-times vect "vectorizing > > > stmts using SLP" 1 > > > FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects > > > scan-tree-dump vect "Built SLP cancelled: can use load/store-lanes" > > > FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects > > > scan-tree-dump vect "LOAD_LANES" > > > FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects > > > scan-tree-dump vect "STORE_LANES" > > > FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "Built SLP
Re: [PATCH] SLP: Move load/store-lanes check till late
On Thu, 5 Nov 2020 at 11:21, Tamar Christina via Gcc-patches
wrote:
>
> > -Original Message-
> > From: rguent...@c653.arch.suse.de On
> > Behalf Of Richard Biener
> > Sent: Thursday, November 5, 2020 10:17 AM
> > To: Tamar Christina
> > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
> >
> > On Wed, 4 Nov 2020, Tamar Christina wrote:
> >
> > > Hi Richi,
> > >
> > > > -Original Message-
> > > > From: rguent...@c653.arch.suse.de On
> > > > Behalf Of Richard Biener
> > > > Sent: Wednesday, November 4, 2020 8:07 AM
> > > > To: Tamar Christina
> > > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> > > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
> > > >
> > > > 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 do still have it,
> > >
> > > if (loads_permuted
> > > && vect_store_lanes_supported (vectype, group_size, false))
> > >
> > > I just grab the type from the SLP_TREE_VECTYPE (slp_root); which
> > > should be the store if one exists.
> > >
> > > > 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.
> > >
> > > I did however elide the kind check, that was added as part of the
> > > rebase, it looked like kind wasn't Being stored inside the SLP instance
> > > and
> > I'd have to redo the analysis to find it.
> > >
> > > Does it does reasonable to include kind as a field in the SLP instance?
> > >
> > > >
> > > > Anyway, the patch is OK for master. The store-lane check part can
> > > > be re- added as followup.
> > > >
> > >
> > > Thanks! Will do.
> >
> > Btw, the patch regressed
> >
> > FAIL: gcc.dg/vect/slp-11b.c -flto -ffat-lto-objects scan-tree-dump-times
> > vect
> > "vectorizing stmts using SLP" 1
> > FAIL: gcc.dg/vect/slp-11b.c scan-tree-dump-times vect "vectorizing stmts
> > using SLP" 1
> > FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump vect
> > "Built SLP cancelled: can use load/store-lanes"
> > FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump vect
> > "LOAD_LANES"
> > FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump vect
> > "STORE_LANES"
> > FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "Built SLP cancelled:
> > can use load/store-lanes"
> > FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "LOAD_LANES"
> > FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "STORE_LANES"
> >
> > on x86_64. The slp-11b.c testcase is interesting since there extract_muldiv
> > folding makes the group of four stores not matching so we split into a size
> > of
> > 3 and one remaining store.
> > This causes us to arrive at
> >
> > note: node 0x441a940 (max_nunits=4, refcnt=2)
> > note: stmt 0 _2 = in[_1];
> > note: stmt 1 _6 = in[_5];
> > note: stmt 2 _10 = in[_8];
> > note: load permutation { 0 2 1 }
> >
> > which on
RE: [PATCH] SLP: Move load/store-lanes check till late
> -Original Message-
> From: rguent...@c653.arch.suse.de On
> Behalf Of Richard Biener
> Sent: Thursday, November 5, 2020 10:17 AM
> To: Tamar Christina
> Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
>
> On Wed, 4 Nov 2020, Tamar Christina wrote:
>
> > Hi Richi,
> >
> > > -Original Message-
> > > From: rguent...@c653.arch.suse.de On
> > > Behalf Of Richard Biener
> > > Sent: Wednesday, November 4, 2020 8:07 AM
> > > To: Tamar Christina
> > > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> > > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
> > >
> > > 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 do still have it,
> >
> > if (loads_permuted
> > && vect_store_lanes_supported (vectype, group_size, false))
> >
> > I just grab the type from the SLP_TREE_VECTYPE (slp_root); which
> > should be the store if one exists.
> >
> > > 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.
> >
> > I did however elide the kind check, that was added as part of the
> > rebase, it looked like kind wasn't Being stored inside the SLP instance and
> I'd have to redo the analysis to find it.
> >
> > Does it does reasonable to include kind as a field in the SLP instance?
> >
> > >
> > > Anyway, the patch is OK for master. The store-lane check part can
> > > be re- added as followup.
> > >
> >
> > Thanks! Will do.
>
> Btw, the patch regressed
>
> FAIL: gcc.dg/vect/slp-11b.c -flto -ffat-lto-objects scan-tree-dump-times vect
> "vectorizing stmts using SLP" 1
> FAIL: gcc.dg/vect/slp-11b.c scan-tree-dump-times vect "vectorizing stmts
> using SLP" 1
> FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump vect
> "Built SLP cancelled: can use load/store-lanes"
> FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump vect
> "LOAD_LANES"
> FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump vect
> "STORE_LANES"
> FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "Built SLP cancelled:
> can use load/store-lanes"
> FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "LOAD_LANES"
> FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "STORE_LANES"
>
> on x86_64. The slp-11b.c testcase is interesting since there extract_muldiv
> folding makes the group of four stores not matching so we split into a size of
> 3 and one remaining store.
> This causes us to arrive at
>
> note: node 0x441a940 (max_nunits=4, refcnt=2)
> note: stmt 0 _2 = in[_1];
> note: stmt 1 _6 = in[_5];
> note: stmt 2 _10 = in[_8];
> note: load permutation { 0 2 1 }
>
> which on x86_64 we in the end cannot handle (without SSE4 I think) so it fails
> to SLP there. Guess arm can do the permute but not the load-lane here.
>
> For gcc.dg/vect/slp-perm-6.c the XFAILs shouldn't be done
> for !vect_load_lanes targets. Not sure if that's possible easily, like with a
> { target vect_load_lanes } { xfail vect_load_lanes } combo ...? I suggest to
> make it xfail everywhere instead and add a comment as to we're expecting
> those only for vect_load_lanes targets.
Yes just fixed these, the change in gcc.dg/vect/slp-11b.c shouldn't be there
and I update
RE: [PATCH] SLP: Move load/store-lanes check till late
On Wed, 4 Nov 2020, Tamar Christina wrote:
> Hi Richi,
>
> > -Original Message-
> > From: rguent...@c653.arch.suse.de On
> > Behalf Of Richard Biener
> > Sent: Wednesday, November 4, 2020 8:07 AM
> > To: Tamar Christina
> > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
> >
> > 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 do still have it,
>
> if (loads_permuted
> && vect_store_lanes_supported (vectype, group_size, false))
>
> I just grab the type from the SLP_TREE_VECTYPE (slp_root); which should be
> the store if
> one exists.
>
> > 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.
>
> I did however elide the kind check, that was added as part of the rebase, it
> looked like kind wasn't
> Being stored inside the SLP instance and I'd have to redo the analysis to
> find it.
>
> Does it does reasonable to include kind as a field in the SLP instance?
>
> >
> > Anyway, the patch is OK for master. The store-lane check part can be re-
> > added as followup.
> >
>
> Thanks! Will do.
Btw, the patch regressed
FAIL: gcc.dg/vect/slp-11b.c -flto -ffat-lto-objects scan-tree-dump-times
vect "vectorizing stmts using SLP" 1
FAIL: gcc.dg/vect/slp-11b.c scan-tree-dump-times vect "vectorizing stmts
using SLP" 1
FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump
vect "Built SLP cancelled: can use load/store-lanes"
FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump
vect "LOAD_LANES"
FAIL: gcc.dg/vect/slp-perm-6.c -flto -ffat-lto-objects scan-tree-dump
vect "STORE_LANES"
FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "Built SLP cancelled:
can use load/store-lanes"
FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "LOAD_LANES"
FAIL: gcc.dg/vect/slp-perm-6.c scan-tree-dump vect "STORE_LANES"
on x86_64. The slp-11b.c testcase is interesting since there
extract_muldiv folding makes the group of four stores
not matching so we split into a size of 3 and one remaining store.
This causes us to arrive at
note: node 0x441a940 (max_nunits=4, refcnt=2)
note: stmt 0 _2 = in[_1];
note: stmt 1 _6 = in[_5];
note: stmt 2 _10 = in[_8];
note: load permutation { 0 2 1 }
which on x86_64 we in the end cannot handle (without SSE4 I think)
so it fails to SLP there. Guess arm can do the permute but not
the load-lane here.
For gcc.dg/vect/slp-perm-6.c the XFAILs shouldn't be done for
!vect_load_lanes targets. Not sure if that's possible easily,
like with a { target vect_load_lanes } { xfail vect_load_lanes }
combo ...? I suggest to make it xfail everywhere instead and
add a comment as to we're expecting those only for vect_load_lanes
targets.
Richard.
> > 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: rguent...@c653.arch.suse.de On
> > > > Behalf Of Richard Biener
> > > > Sent: Thursday, October 22, 2020 9:44 AM
> > > > To: Tamar Christ
RE: [PATCH] SLP: Move load/store-lanes check till late
On Wed, 4 Nov 2020, Tamar Christina wrote:
> Hi Richi,
>
> > -Original Message-
> > From: rguent...@c653.arch.suse.de On
> > Behalf Of Richard Biener
> > Sent: Wednesday, November 4, 2020 8:07 AM
> > To: Tamar Christina
> > Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> > Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
> >
> > 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 do still have it,
>
> if (loads_permuted
> && vect_store_lanes_supported (vectype, group_size, false))
>
> I just grab the type from the SLP_TREE_VECTYPE (slp_root); which should be
> the store if
> one exists.
Ah, I see.
> > 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.
>
> I did however elide the kind check, that was added as part of the rebase, it
> looked like kind wasn't
> Being stored inside the SLP instance and I'd have to redo the analysis to
> find it.
>
> Does it does reasonable to include kind as a field in the SLP instance?
Yeah, it's on my list of things - I just didn't (yet) have a use outside
of the current narrow scope. So feel free to add a field to the SLP
instance and move the enum to tree-vectorizer.h.
> >
> > Anyway, the patch is OK for master. The store-lane check part can be re-
> > added as followup.
> >
>
> Thanks! Will do.
>
> > 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: rguent...@c653.arch.suse.de On
> > > > Behalf Of Richard Biener
> > > > Sent: Thursday, October 22, 2020 9:44 AM
> > > > To: Tamar Christina
> > > > Cc: gcc-patches@gcc.gnu.org; nd ; 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/st
RE: [PATCH] SLP: Move load/store-lanes check till late
Hi Richi,
> -Original Message-
> From: rguent...@c653.arch.suse.de On
> Behalf Of Richard Biener
> Sent: Wednesday, November 4, 2020 8:07 AM
> To: Tamar Christina
> Cc: gcc-patches@gcc.gnu.org; nd ; o...@ucw.cz
> Subject: RE: [PATCH] SLP: Move load/store-lanes check till late
>
> 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 do still have it,
if (loads_permuted
&& vect_store_lanes_supported (vectype, group_size, false))
I just grab the type from the SLP_TREE_VECTYPE (slp_root); which should be the
store if
one exists.
> 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.
I did however elide the kind check, that was added as part of the rebase, it
looked like kind wasn't
Being stored inside the SLP instance and I'd have to redo the analysis to find
it.
Does it does reasonable to include kind as a field in the SLP instance?
>
> Anyway, the patch is OK for master. The store-lane check part can be re-
> added as followup.
>
Thanks! Will do.
> 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: rguent...@c653.arch.suse.de On
> > > Behalf Of Richard Biener
> > > Sent: Thursday, October 22, 2020 9:44 AM
> > > To: Tamar Christina
> > > Cc: gcc-patches@gcc.gnu.org; nd ; 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.
> > >
> > > 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
> > > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > > Nuernberg, Germany; GF: Felix Imend
> >
>
> --
> Richard Biener
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> Nuernberg, Germany; GF: Felix Imend
RE: [PATCH] SLP: Move load/store-lanes check till late
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: rguent...@c653.arch.suse.de On
> > Behalf Of Richard Biener
> > Sent: Thursday, October 22, 2020 9:44 AM
> > To: Tamar Christina
> > Cc: gcc-patches@gcc.gnu.org; nd ; 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.
> >
> > 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
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > Nuernberg, Germany; GF: Felix Imend
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
RE: [PATCH] SLP: Move load/store-lanes check till late
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?
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: rguent...@c653.arch.suse.de On
> Behalf Of Richard Biener
> Sent: Thursday, October 22, 2020 9:44 AM
> To: Tamar Christina
> Cc: gcc-patches@gcc.gnu.org; nd ; 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.
>
> 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
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> Nuernberg, Germany; GF: Felix Imend
diff --git a/gcc/testsuite/gcc.dg/vect/slp-11b.c b/gcc/testsuite/gcc.dg/vect/slp-11b.c
index 0cc23770badf0e00ef98769a2dd14a92dca32cca..fe5bb0c3ce7682c7cef1313e342d95aba3fe11b2 100644
--- a/gcc/testsuite/gcc.dg/vect/slp-11b.c
+++ b/gcc/testsuite/gcc.dg/vect/slp-11b.c
@@ -45,4 +45,4 @@ int main (void)
/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_strided4 && vect_int_mult } } } } */
/* { dg-final { scan-tree-dump-times "vectorized 0 loops" 1 "vect" { target { ! { vect_strided4 && vect_int_mult } } } } } */
-/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" } } */
+/* { dg-final { scan-tree-dump-times "re-trying with SLP disabled" 1 "vect" } } */
diff --git a/gcc/testsuite/gcc.dg/vect/slp-perm-6.c b/gcc/testsuite/gcc.dg/vect/slp-perm-6.c
index 38489291a2659c989121d44c9e9e7bdfaa12f868..07bf8916de7ce88bbb1d65437f8bf6d8ab17efe6 100644
--- a/gcc/testsuite/gcc.dg/vect/slp-perm-6.c
+++ b/gcc/testsuite/gcc.dg/vect/slp-perm-6.c
@@ -106,7 +106,7 @@ int main (int argc, const char* argv[])
/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 2 "vect" { target { vect_perm3_int && { {! vect_load_lanes } && {! vect_partial_vectors_usage_1 } } } } } } */
/* The epilogues are vectorized using partial vectors. */
/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 4 "vect" { target { vect_perm3_int && { {! vect_load_lanes } && vect_partial_vectors_usage_1 } } } } } */
-/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target vect_load_lanes } } } */
+/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 0 "vect" { target vect_load_lanes } } } */
/* { dg-final { scan-tree-dump "Built SLP cancelled: can use load/store-lanes" "vect" { target { vect_perm3_int && vect_load_lanes } } } } */
/* { dg-final { scan-tree-dump "LOAD_LANES" "vect" { target vect_load_
RE: [PATCH] SLP: Move load/store-lanes check till late
On Thu, 22 Oct 2020, Tamar Christina wrote:
> Hi Richi,
>
> > -Original Message-
> > From: rguent...@c653.arch.suse.de On
> > Behalf Of Richard Biener
> > Sent: Thursday, October 22, 2020 9:44 AM
> > To: Tamar Christina
> > Cc: gcc-patches@gcc.gnu.org; nd ; 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
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > Nuernberg, Germany; GF: Felix Imend
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
RE: [PATCH] SLP: Move load/store-lanes check till late
Hi Richi,
> -Original Message-
> From: rguent...@c653.arch.suse.de On
> Behalf Of Richard Biener
> Sent: Thursday, October 22, 2020 9:44 AM
> To: Tamar Christina
> Cc: gcc-patches@gcc.gnu.org; nd ; 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.
I can make it into a dump, but would still have to assign to `ok` unless I'm
missing
something.
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
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> Nuernberg, Germany; GF: Felix Imend
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
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
