RE: [PATCH]middle-end: don't lower past veclower [PR106063]

[Richard Biener <rguenther@suse.de>]

On Fri, 8 Jul 2022, Tamar Christina wrote:

> > -----Original Message-----
> > From: Richard Biener <rguenther@suse.de>
> > Sent: Thursday, July 7, 2022 8:47 AM
> > To: Tamar Christina <Tamar.Christina@arm.com>
> > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> > Subject: RE: [PATCH]middle-end: don't lower past veclower [PR106063]
> > 
> > On Thu, 7 Jul 2022, Tamar Christina wrote:
> > 
> > > > -----Original Message-----
> > > > From: Richard Biener <rguenther@suse.de>
> > > > Sent: Thursday, July 7, 2022 8:19 AM
> > > > To: Tamar Christina <Tamar.Christina@arm.com>
> > > > Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>
> > > > Subject: Re: [PATCH]middle-end: don't lower past veclower [PR106063]
> > > >
> > > > On Tue, 5 Jul 2022, Tamar Christina wrote:
> > > >
> > > > > Hi All,
> > > > >
> > > > > My previous patch can cause a problem if the pattern matches after
> > > > > veclower as it may replace the construct with a vector sequence
> > > > > which the target may not directly support.
> > > > >
> > > > > As such don't perform the rewriting if after veclower.
> > > >
> > > > Note that when doing the rewriting before veclower to a variant not
> > > > supported by the target can cause veclower to generate absymal code.
> > > > In some cases we are very careful and try to at least preserve code
> > > > supported by the target over transforming that into a variant not
> > supported.
> > > >
> > > > That said, a better fix would be to check whether the target can
> > > > perform the new comparison.  Before veclower it would be OK to do
> > > > the transform nevertheless in case it cannot do the original transform.
> > >
> > > This last statement is somewhat confusing. Did you want me to change
> > > it such that before veclower the rewrite is always done and after
> > > veclowering only if the target supports it?
> > >
> > > Or did you want me to never do the rewrite if the target doesn't support it?
> > 
> > I meant before veclower you can do the rewrite if either the rewriting result
> > is supported by the target OR if the original expression is _not_ supported by
> > the target.  The latter case might be not too important to worry doing (it
> > would still canonicalize for those targets then).  After veclower you can only
> > rewrite under the former condition.
> > 
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> and no issues.
> 
> Ok for master? and backport to GCC 12?

OK for master, backport to GCC 12 after a few days of soaking.

Thanks,
Richard.

> Thanks,
> Tamar
> 
> 
> gcc/ChangeLog:
> 
>     PR tree-optimization/106063
> 	* match.pd: Only rewrite if target support it.
> 
> gcc/testsuite/ChangeLog:
> 
>     PR tree-optimization/106063
> 	* gcc.dg/pr106063.c: New test.
> 
> --- inline copy of patch ---
> 
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 40c09bedadb89dabb6622559a8f69df5384e61fd..5800a105c3cdada9d5e1d8019176ebbe5969ccb0 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -6041,10 +6041,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>     (cmp (bit_and:c@2 @0 cst@1) integer_zerop)
>      (with { tree csts = bitmask_inv_cst_vector_p (@1); }
>       (if (csts && (VECTOR_TYPE_P (TREE_TYPE (@1)) || single_use (@2)))
> -      (if (TYPE_UNSIGNED (TREE_TYPE (@1)))
> -       (icmp @0 { csts; })
> -       (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
> -	 (icmp (view_convert:utype @0) { csts; }))))))))
> +      (with { auto optab = VECTOR_TYPE_P (TREE_TYPE (@1))
> +			 ? optab_vector : optab_default;
> +	      tree utype = unsigned_type_for (TREE_TYPE (@1)); }
> +       (if (target_supports_op_p (utype, icmp, optab)
> +	    || (optimize_vectors_before_lowering_p ()
> +		&& (!target_supports_op_p (type, cmp, optab)
> +		    || !target_supports_op_p (type, BIT_AND_EXPR, optab))))
> +	(if (TYPE_UNSIGNED (TREE_TYPE (@1)))
> +	 (icmp @0 { csts; })
> +	 (icmp (view_convert:utype @0) { csts; })))))))))
>  
>  /* When one argument is a constant, overflow detection can be simplified.
>     Currently restricted to single use so as not to interfere too much with
> diff --git a/gcc/testsuite/gcc.dg/pr106063.c b/gcc/testsuite/gcc.dg/pr106063.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b23596724f6bb98c53af2dce77d31509bab10378
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr106063.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-forwprop --disable-tree-evrp" } */
> +typedef __int128 __attribute__((__vector_size__ (16))) V;
> +
> +V
> +foo (V v)
> +{
> +  return (v & (V){15}) == v;
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstra