Fix PR55152

On Thu, 29 Sep 2016, Richard Biener wrote:

> On Wed, 28 Sep 2016, Joseph Myers wrote:
> 
> > On Wed, 28 Sep 2016, Richard Biener wrote:
> > 
> > > Index: gcc/testsuite/gcc.dg/pr55152.c
> > > ===================================================================
> > > --- gcc/testsuite/gcc.dg/pr55152.c	(revision 0)
> > > +++ gcc/testsuite/gcc.dg/pr55152.c	(working copy)
> > > @@ -0,0 +1,13 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O -ffinite-math-only -fstrict-overflow -fdump-tree-optimized" } */
> > > +
> > > +double g (double a)
> > > +{
> > > +  return (a>=-a)?a:-a;
> > 
> > You should need -fno-signed-zeros for this (that is, for the 
> > transformation to MAX_EXPR), not -ffinite-math-only.  For a == -0, that 
> > function should return -0, but abs would produce +0.
> 
> This means that tree-ssa-phiopt.c has a bug:
> 
> static bool
> minmax_replacement (basic_block cond_bb, basic_block middle_bb,
>                     edge e0, edge e1, gimple *phi,
>                     tree arg0, tree arg1)
> {
> ...
>   /* The optimization may be unsafe due to NaNs.  */
>   if (HONOR_NANS (type))
>     return false;
> 
> and it should check HONOR_SIGNED_ZEROS as well.
> 
> I'll fix that as a followup.

Committed as follows.

Bootstrapped / tested on x86_64-unknown-linux-gnu.

Richard.

2016-09-29  Richard Biener  <rguenther@suse.de>

	PR middle-end/55152
	* match.pd: Add max(a,-a) -> abs(a) pattern.
	* tree-ssa-phiopt.c (minmax_replacement): Disable for
	HONOR_SIGNED_ZEROS types.

	* gcc.dg/pr55152.c: New testcase.
	* gcc.dg/tree-ssa/phi-opt-5.c: Adjust.

On Thu, Sep 29, 2016 at 8:23 PM, Richard Biener <rguenther@suse.de> wrote:
> On Thu, 29 Sep 2016, Richard Biener wrote:
>
>> On Wed, 28 Sep 2016, Joseph Myers wrote:
>>
>> > On Wed, 28 Sep 2016, Richard Biener wrote:
>> >
>> > > Index: gcc/testsuite/gcc.dg/pr55152.c
>> > > ===================================================================
>> > > --- gcc/testsuite/gcc.dg/pr55152.c        (revision 0)
>> > > +++ gcc/testsuite/gcc.dg/pr55152.c        (working copy)
>> > > @@ -0,0 +1,13 @@
>> > > +/* { dg-do compile } */
>> > > +/* { dg-options "-O -ffinite-math-only -fstrict-overflow -fdump-tree-optimized" } */
>> > > +
>> > > +double g (double a)
>> > > +{
>> > > +  return (a>=-a)?a:-a;
>> >
>> > You should need -fno-signed-zeros for this (that is, for the
>> > transformation to MAX_EXPR), not -ffinite-math-only.  For a == -0, that
>> > function should return -0, but abs would produce +0.
>>
>> This means that tree-ssa-phiopt.c has a bug:
>>
>> static bool
>> minmax_replacement (basic_block cond_bb, basic_block middle_bb,
>>                     edge e0, edge e1, gimple *phi,
>>                     tree arg0, tree arg1)
>> {
>> ...
>>   /* The optimization may be unsafe due to NaNs.  */
>>   if (HONOR_NANS (type))
>>     return false;
>>
>> and it should check HONOR_SIGNED_ZEROS as well.
>>
>> I'll fix that as a followup.
>
> Committed as follows.
>
> Bootstrapped / tested on x86_64-unknown-linux-gnu.
>
> Richard.
>
> 2016-09-29  Richard Biener  <rguenther@suse.de>
>
>         PR middle-end/55152
>         * match.pd: Add max(a,-a) -> abs(a) pattern.


Hmm, shouldn't we also do "min(a, -a) -> - abs(a)" ?

Thanks,
Andrew Pinski

>         * tree-ssa-phiopt.c (minmax_replacement): Disable for
>         HONOR_SIGNED_ZEROS types.
>
>         * gcc.dg/pr55152.c: New testcase.
>         * gcc.dg/tree-ssa/phi-opt-5.c: Adjust.
>
> Index: gcc/match.pd
> ===================================================================
> --- gcc/match.pd        (revision 240565)
> +++ gcc/match.pd        (working copy)
> @@ -1271,6 +1284,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  (simplify
>   (max:c (min:c @0 @1) @1)
>   @1)
> +/* max(a,-a) -> abs(a).  */
> +(simplify
> + (max:c @0 (negate @0))
> + (if (TREE_CODE (type) != COMPLEX_TYPE
> +      && (! ANY_INTEGRAL_TYPE_P (type)
> +         || TYPE_OVERFLOW_UNDEFINED (type)))
> +  (abs @0)))
>  (simplify
>   (min @0 @1)
>   (switch
> Index: gcc/tree-ssa-phiopt.c
> ===================================================================
> --- gcc/tree-ssa-phiopt.c       (revision 240565)
> +++ gcc/tree-ssa-phiopt.c       (working copy)
> @@ -1075,7 +1075,7 @@ minmax_replacement (basic_block cond_bb,
>    type = TREE_TYPE (PHI_RESULT (phi));
>
>    /* The optimization may be unsafe due to NaNs.  */
> -  if (HONOR_NANS (type))
> +  if (HONOR_NANS (type) || HONOR_SIGNED_ZEROS (type))
>      return false;
>
>    cond = as_a <gcond *> (last_stmt (cond_bb));
> Index: gcc/testsuite/gcc.dg/pr55152.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr55152.c      (revision 0)
> +++ gcc/testsuite/gcc.dg/pr55152.c      (working copy)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -ffinite-math-only -fno-signed-zeros -fstrict-overflow -fdump-tree-optimized" } */
> +
> +double g (double a)
> +{
> +  return (a>=-a)?a:-a;
> +}
> +int f(int a)
> +{
> +  return (a>=-a)?a:-a;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "ABS_EXPR" 2 "optimized" } } */
> Index: gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c   (revision 240612)
> +++ gcc/testsuite/gcc.dg/tree-ssa/phi-opt-5.c   (working copy)
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O1 -ffinite-math-only -fdump-tree-phiopt1" } */
> +/* { dg-options "-O1 -ffinite-math-only -fno-signed-zeros -fdump-tree-phiopt1" } */
>
>  float repl1 (float varx)
>  {