Fix ldist memset discovery with -0.0 (PR tree-optimization/72824)

Hi!

Only +0.0 stores can be optimized into memset, -0.0 can't, so if we are
honoring signed zeros, we should make sure the constant is positive.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-08-08  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/72824
	* tree-loop-distribution.c (const_with_all_bytes_same): If honoring
	signed zeros, verify real_zerop is not negative.

	* gcc.c-torture/execute/ieee/pr72824.c: New test.


	Jakub

On Mon, 8 Aug 2016, Jakub Jelinek wrote:

> Hi!
> 
> Only +0.0 stores can be optimized into memset, -0.0 can't, so if we are
> honoring signed zeros, we should make sure the constant is positive.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Hmm, I don't think we should see -0.0 as +0.0 with -fno-signed-zeros.
As far as I can see this is a memory load/store op and we may not
transform, say,

  double x = a[i];
  b[i] = x;

into a copy that changes -0.0 to +0.0.  loop distribution looks for
a value with all-zero bytes.

Richard.

> 2016-08-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/72824
> 	* tree-loop-distribution.c (const_with_all_bytes_same): If honoring
> 	signed zeros, verify real_zerop is not negative.
> 
> 	* gcc.c-torture/execute/ieee/pr72824.c: New test.
> 
> --- gcc/tree-loop-distribution.c.jj	2016-07-16 10:41:04.000000000 +0200
> +++ gcc/tree-loop-distribution.c	2016-08-07 13:55:19.704681784 +0200
> @@ -750,12 +750,40 @@ const_with_all_bytes_same (tree val)
>    int i, len;
>  
>    if (integer_zerop (val)
> -      || real_zerop (val)
>        || (TREE_CODE (val) == CONSTRUCTOR
>            && !TREE_CLOBBER_P (val)
>            && CONSTRUCTOR_NELTS (val) == 0))
>      return 0;
>  
> +  if (real_zerop (val))
> +    {
> +      if (!HONOR_SIGNED_ZEROS (val))
> +	return 0;
> +      /* If honoring signed zeros, only return 0 for +0.0, not for -0.0.  */
> +      switch (TREE_CODE (val))
> +	{
> +	case REAL_CST:
> +	  if (!real_isneg (TREE_REAL_CST_PTR (val)))
> +	    return 0;
> +	  break;
> +	case COMPLEX_CST:
> +	  if (!const_with_all_bytes_same (TREE_REALPART (val))
> +	      && !const_with_all_bytes_same (TREE_IMAGPART (val)))
> +	    return 0;
> +	  break;
> +	case VECTOR_CST:
> +	  unsigned int j;
> +	  for (j = 0; j < VECTOR_CST_NELTS (val); ++j)
> +	    if (const_with_all_bytes_same (val))
> +	      break;
> +	  if (j == VECTOR_CST_NELTS (val))
> +	    return 0;
> +	  break;
> +	default:
> +	  break;
> +	}
> +    }
> +
>    if (CHAR_BIT != 8 || BITS_PER_UNIT != 8)
>      return -1;
>  
> --- gcc/testsuite/gcc.c-torture/execute/ieee/pr72824.c.jj	2016-08-07 13:19:42.443863775 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/ieee/pr72824.c	2016-08-07 13:19:35.034958218 +0200
> @@ -0,0 +1,19 @@
> +/* PR tree-optimization/72824 */
> +
> +static inline void
> +foo (float *x, float value)
> +{
> +  int i;
> +  for (i = 0; i < 32; ++i)
> +    x[i] = value;
> +}
> +
> +int
> +main ()
> +{
> +  float x[32];
> +  foo (x, -0.f);
> +  if (__builtin_copysignf (1.0, x[3]) != -1.0f)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>

On Tue, Aug 09, 2016 at 08:53:54AM +0200, Richard Biener wrote:
> Hmm, I don't think we should see -0.0 as +0.0 with -fno-signed-zeros.
> As far as I can see this is a memory load/store op and we may not
> transform, say,
> 
>   double x = a[i];
>   b[i] = x;
> 
> into a copy that changes -0.0 to +0.0.  loop distribution looks for
> a value with all-zero bytes.

So shall I just drop the if (!HONOR_SIGNED_ZEROS (val)) return 0; from the
patch?  With that removed, it will not transform b[i] = -0.0; into
memset (b, 0, ...); even with -fno-signed-zeros.

> > 2016-08-08  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR tree-optimization/72824
> > 	* tree-loop-distribution.c (const_with_all_bytes_same): If honoring
> > 	signed zeros, verify real_zerop is not negative.
> > 
> > 	* gcc.c-torture/execute/ieee/pr72824.c: New test.
> > 
> > --- gcc/tree-loop-distribution.c.jj	2016-07-16 10:41:04.000000000 +0200
> > +++ gcc/tree-loop-distribution.c	2016-08-07 13:55:19.704681784 +0200
> > @@ -750,12 +750,40 @@ const_with_all_bytes_same (tree val)
> >    int i, len;
> >  
> >    if (integer_zerop (val)
> > -      || real_zerop (val)
> >        || (TREE_CODE (val) == CONSTRUCTOR
> >            && !TREE_CLOBBER_P (val)
> >            && CONSTRUCTOR_NELTS (val) == 0))
> >      return 0;
> >  
> > +  if (real_zerop (val))
> > +    {
> > +      if (!HONOR_SIGNED_ZEROS (val))
> > +	return 0;
> > +      /* If honoring signed zeros, only return 0 for +0.0, not for -0.0.  */
> > +      switch (TREE_CODE (val))
> > +	{
> > +	case REAL_CST:
> > +	  if (!real_isneg (TREE_REAL_CST_PTR (val)))
> > +	    return 0;
> > +	  break;
> > +	case COMPLEX_CST:
> > +	  if (!const_with_all_bytes_same (TREE_REALPART (val))
> > +	      && !const_with_all_bytes_same (TREE_IMAGPART (val)))
> > +	    return 0;
> > +	  break;
> > +	case VECTOR_CST:
> > +	  unsigned int j;
> > +	  for (j = 0; j < VECTOR_CST_NELTS (val); ++j)
> > +	    if (const_with_all_bytes_same (val))
> > +	      break;
> > +	  if (j == VECTOR_CST_NELTS (val))
> > +	    return 0;
> > +	  break;
> > +	default:
> > +	  break;
> > +	}
> > +    }
> > +
> >    if (CHAR_BIT != 8 || BITS_PER_UNIT != 8)
> >      return -1;
> >  

	Jakub

On Tue, 9 Aug 2016, Jakub Jelinek wrote:

> On Tue, Aug 09, 2016 at 08:53:54AM +0200, Richard Biener wrote:
> > Hmm, I don't think we should see -0.0 as +0.0 with -fno-signed-zeros.
> > As far as I can see this is a memory load/store op and we may not
> > transform, say,
> > 
> >   double x = a[i];
> >   b[i] = x;
> > 
> > into a copy that changes -0.0 to +0.0.  loop distribution looks for
> > a value with all-zero bytes.
> 
> So shall I just drop the if (!HONOR_SIGNED_ZEROS (val)) return 0; from the
> patch?  With that removed, it will not transform b[i] = -0.0; into
> memset (b, 0, ...); even with -fno-signed-zeros.

Yes, ok with that change (maybe add a comment that we do this on purpose).

Thanks,
Richard.

> > > 2016-08-08  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > > 	PR tree-optimization/72824
> > > 	* tree-loop-distribution.c (const_with_all_bytes_same): If honoring
> > > 	signed zeros, verify real_zerop is not negative.
> > > 
> > > 	* gcc.c-torture/execute/ieee/pr72824.c: New test.
> > > 
> > > --- gcc/tree-loop-distribution.c.jj	2016-07-16 10:41:04.000000000 +0200
> > > +++ gcc/tree-loop-distribution.c	2016-08-07 13:55:19.704681784 +0200
> > > @@ -750,12 +750,40 @@ const_with_all_bytes_same (tree val)
> > >    int i, len;
> > >  
> > >    if (integer_zerop (val)
> > > -      || real_zerop (val)
> > >        || (TREE_CODE (val) == CONSTRUCTOR
> > >            && !TREE_CLOBBER_P (val)
> > >            && CONSTRUCTOR_NELTS (val) == 0))
> > >      return 0;
> > >  
> > > +  if (real_zerop (val))
> > > +    {
> > > +      if (!HONOR_SIGNED_ZEROS (val))
> > > +	return 0;
> > > +      /* If honoring signed zeros, only return 0 for +0.0, not for -0.0.  */
> > > +      switch (TREE_CODE (val))
> > > +	{
> > > +	case REAL_CST:
> > > +	  if (!real_isneg (TREE_REAL_CST_PTR (val)))
> > > +	    return 0;
> > > +	  break;
> > > +	case COMPLEX_CST:
> > > +	  if (!const_with_all_bytes_same (TREE_REALPART (val))
> > > +	      && !const_with_all_bytes_same (TREE_IMAGPART (val)))
> > > +	    return 0;
> > > +	  break;
> > > +	case VECTOR_CST:
> > > +	  unsigned int j;
> > > +	  for (j = 0; j < VECTOR_CST_NELTS (val); ++j)
> > > +	    if (const_with_all_bytes_same (val))
> > > +	      break;
> > > +	  if (j == VECTOR_CST_NELTS (val))
> > > +	    return 0;
> > > +	  break;
> > > +	default:
> > > +	  break;
> > > +	}
> > > +    }
> > > +
> > >    if (CHAR_BIT != 8 || BITS_PER_UNIT != 8)
> > >      return -1;
> > >  
> 
> 	Jakub
> 
>