Re: [PATCH] Fix BIT_FIELD_REF folding (PR middle-end/85195)

[Richard Biener <rguenther at suse dot de>]

On Wed, 4 Apr 2018, Jakub Jelinek wrote:

> Hi!
> 
> On the following testcase because of the SCCVN limit we end up with a weird,
> but valid, BIT_FIELD_REF - trying to extract V1TImode type out of a V1TImode
> SSA_NAME, with 128 bits width and offset 0 (just SSA_NAME move would be
> enough).  Not trying to address why we create it, rather fix how we handle
> it.
> 
> The problem is that the BIT_FIELD_REF vector extraction is guarded with:
>  (if (VECTOR_TYPE_P (TREE_TYPE (@0))
>       && (types_match (type, TREE_TYPE (TREE_TYPE (@0)))
>           || (VECTOR_TYPE_P (type)
>               && types_match (TREE_TYPE (type), TREE_TYPE (TREE_TYPE (@0))))))
> and thus we must handle vector type as result, but assume incorrectly that
> if count is 1, then we must be asking for an element and thus not vector
> type.  For the first hunk we could do what we do in the second hunk, do
> a VIEW_CONVERT_EXPR, but it seems cleaner to just fall through into the
> count > 1 code which builds a CONSTRUCTOR.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I think I prefer a V_C_E in the first case like in the second.  That
may help relaxing the type constraint above to a size one so we
can handle SImode extracts from a VnSFmode vector for example.

It also shows we are lacking a generic
(BIT_FIELD_REF @0 bitsize-of-@0 integer_zerop) to (VIEW_CONVERT @0)
pattern which would need to be ordered before the constructor one
to make sure it matches first (and it would have handled the
very specific case as well).

So - OK with doing the same in the first hunk as in the second.

Thanks,
Richard.

> 2018-04-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/85195
> 	* match.pd (BIT_FIELD_REF CONSTRUCTOR@0 @1 @2): If count == 1, only
> 	use elt value if type is not vector type, otherwise build CONSTRUCTOR.
> 	For n == const_k, use view_convert.
> 
> 	* gcc.dg/pr85195.c: New test.
> 
> --- gcc/match.pd.jj	2018-03-28 21:15:00.000000000 +0200
> +++ gcc/match.pd	2018-04-04 14:35:10.424127155 +0200
> @@ -4648,7 +4648,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>        (if (multiple_p (idx, k, &elt) && multiple_p (n, k, &count))
>         (if (CONSTRUCTOR_NELTS (ctor) == 0)
>          { build_constructor (type, NULL); }
> -	(if (count == 1)
> +	(if (count == 1 && !VECTOR_TYPE_P (type))
>  	 (if (elt < CONSTRUCTOR_NELTS (ctor))
>  	  { CONSTRUCTOR_ELT (ctor, elt)->value; }
>  	  { build_zero_cst (type); })
> @@ -4668,7 +4668,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  	(if (CONSTRUCTOR_NELTS (ctor) <= idx / const_k)
>  	 { build_zero_cst (type); })
>  	(if (n == const_k)
> -	 { CONSTRUCTOR_ELT (ctor, idx / const_k)->value; })
> +	 (view_convert { CONSTRUCTOR_ELT (ctor, idx / const_k)->value; }))
>  	(BIT_FIELD_REF { CONSTRUCTOR_ELT (ctor, idx / const_k)->value; }
>  		       @1 { bitsize_int ((idx % const_k) * width); })))))))))
>  
> --- gcc/testsuite/gcc.dg/pr85195.c.jj	2018-04-04 14:38:29.233235494 +0200
> +++ gcc/testsuite/gcc.dg/pr85195.c	2018-04-04 14:38:14.696227566 +0200
> @@ -0,0 +1,19 @@
> +/* PR middle-end/85195 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-Wno-psabi -O -fno-tree-ccp --param=sccvn-max-scc-size=10" } */
> +
> +typedef __int128 V __attribute__ ((vector_size (16)));
> +
> +extern int bar (V);
> +
> +V v;
> +int i;
> +
> +V
> +foo (void)
> +{
> +  do
> +    v *= bar (v & i);
> +  while ((V){}[0]);
> +  return v;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)