RE: [PATCH] Optimize VEC_PERM_EXPR with same permutation index and operation [PR98167]

[Tamar Christina <Tamar.Christina@arm.com>]

> -----Original Message-----
> From: Jakub Jelinek <jakub@redhat.com>
> Sent: Wednesday, November 16, 2022 3:38 PM
> To: Richard Biener <richard.guenther@gmail.com>
> Cc: Tamar Christina <Tamar.Christina@arm.com>; Hongyu Wang
> <wwwhhhyyy333@gmail.com>; Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org>; Richard Sandiford
> <Richard.Sandiford@arm.com>; Hongyu Wang <hongyu.wang@intel.com>;
> hongtao.liu@intel.com; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Optimize VEC_PERM_EXPR with same permutation
> index and operation [PR98167]
> 
> On Wed, Nov 16, 2022 at 04:30:06PM +0100, Richard Biener via Gcc-patches
> wrote:
> > On Wed, Nov 16, 2022 at 4:29 PM Richard Biener
> > <richard.guenther@gmail.com> wrote:
> > >
> > > On Wed, Nov 16, 2022 at 4:25 PM Tamar Christina
> <Tamar.Christina@arm.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > This patch is causing several ICEs because it changes the permutes
> > > > from a single register permute to a multi register due to the lowering of
> the expressions to different SSA names.
> > > >
> > > > See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107717
> > > >
> > > > I have a prototype fix which adds a new rule to CSE this back to a
> > > > single register permute, but would this be the right solution? It
> > > > seems hard to later on during expand realize that the two operands are
> the same.
> > > >
> > > > It's probably also ok to just block this from happening after
> > > > vec_lower, however I'm worried that If it wasn't CSE'd before
> vec_lower it'll lower it so something much less efficient.
> > >
> > > You can use
> > >
> > >  (vec_perm (op@7 @0 @1) @3)
> >
> > Err, (vec_perm (op@7 @0 @1) @7) obviously.
> 
> Even:
> 
> --- gcc/match.pd.jj	2022-11-15 07:56:05.240348804 +0100
> +++ gcc/match.pd	2022-11-16 16:35:34.854080956 +0100
> @@ -8259,7 +8259,7 @@ and,
>   (simplify
>    (op (vec_perm @0 @0 @2) (vec_perm @1 @1 @2))
>     (if (VECTOR_INTEGER_TYPE_P (type))
> -    (vec_perm (op @0 @1) (op @0 @1) @2))))
> +    (vec_perm (op@3 @0 @1) @3 @2))))
> 
>  /* Similar for float arithmetic when permutation constant covers
>     all vector elements.  */
> @@ -8298,4 +8298,4 @@ and,
>  	 }
>        }
>        (if (full_perm_p)
> -	(vec_perm (op @0 @1) (op @0 @1) @2))))))
> +	(vec_perm (op@3 @0 @1) @3 @2))))))
> 
> From quick look at the dumps, it seems to work fine.
> 
> 	Jakub

Yes that's the patch I'm currently reg-testing 😊

Cheers,
Tamar