### Re: [PATCH] Fix PR55152

On Tue, Oct 4, 2016 at 2:50 AM, Richard Bienerwrote: > On Tue, 4 Oct 2016, Marc Glisse wrote: > >> On Tue, 4 Oct 2016, Richard Biener wrote: >> >> > Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). >> >> I thought this might fix PR 62055, but at least on x86_64, we generate much >> worse code for copysign(,-1) than for -abs :-( > > I would have expected copysign(,-1) to be a simple IOR ... > > double foo (double x) > { > return __builtin_copysign (x, -1.); > } > > foo: > .LFB0: > .cfi_startproc > movsd .LC0(%rip), %xmm1 > movapd %xmm1, %xmm2 > andpd .LC2(%rip), %xmm2 > andpd .LC1(%rip), %xmm0 > orpd%xmm2, %xmm0 > ret > > ICK. -fabs (x) yields > > foo: > .LFB0: > .cfi_startproc > andpd .LC0(%rip), %xmm0 > xorpd .LC1(%rip), %xmm0 > ret > > I expected a simple orpd .LC0(%rip), %xmm0 ... That is recorded already as PR62055 :). Thanks, Andrew > > Richard.

### Re: [PATCH] Fix PR55152

On Wed, 5 Oct 2016, Richard Biener wrote: > On Tue, 4 Oct 2016, Joseph Myers wrote: > > > On Tue, 4 Oct 2016, Richard Biener wrote: > > > > > On Tue, 4 Oct 2016, Joseph Myers wrote: > > > > > > > On Tue, 4 Oct 2016, Richard Biener wrote: > > > > > > > > > Possibly. Though then for FP we also want - abs (a) -> copysign (a, > > > > > -1). > > > > > > > > For architectures such as powerpc that have a negated-abs instruction, > > > > does it get properly generated from the copysign code? (The relevant > > > > pattern in rs6000.md uses (neg (abs)) - is that the correct canonical > > > > RTL > > > > for this?) > > > > > > I have no idea - given that there is no copysign RTL code I suppose > > > it is the only machine independent RTL that can do this? > > > > > > The question would be whether the copysign optab of said targets would > > > special-case the -1 case appropriately. > > > > Why -1? Any constant in copysign should be handled appropriately. I'd > > say that (neg (abs)) is probably better as a canonical representation, so > > map copysign from a constant to either (abs) or (neg (abs)) appropriately. > > > > Then in the case where abs is expanded with bit manipulation, (neg (abs)) > > should be expanded to a single OR. I don't know whether the RTL > > optimizers will map the AND / OR combination to a single OR, but if they > > do then there should be no need to special-case (neg (abs)) in expansion, > > and when (abs) RTL is generated a machine description's (neg (abs)) > > pattern should match automatically. > > Ok, that's a good point -- I'll do copysign (X, const) canonicalization > to [-]abs() then. Hopefully that's a valid transform even if NaNs and > signed zeros are involved. AFAICS it is. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2016-10-05 Richard Biener* match.pd (copysign(x, CST) -> [-]abs (x)): New pattern. * gcc.dg/fold-copysign-1.c: New testcase. Index: gcc/match.pd === --- gcc/match.pd(revision 240770) +++ gcc/match.pd(working copy) @@ -452,6 +452,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (hypots @0 (copysigns @1 @2)) (hypots @0 @1))) +/* copysign(x, CST) -> [-]abs (x). */ +(for copysigns (COPYSIGN) + (simplify + (copysigns @0 REAL_CST@1) + (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1))) + (negate (abs @0)) + (abs @0 + /* copysign(copysign(x, y), z) -> copysign(x, z). */ (for copysigns (COPYSIGN) (simplify Index: gcc/testsuite/gcc.dg/fold-copysign-1.c === --- gcc/testsuite/gcc.dg/fold-copysign-1.c (revision 0) +++ gcc/testsuite/gcc.dg/fold-copysign-1.c (working copy) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-cddce1" } */ + +double foo (double x) +{ + double one = 1.; + return __builtin_copysign (x, one); +} +double bar (double x) +{ + double minuszero = -0.; + return __builtin_copysign (x, minuszero); +} + +/* { dg-final { scan-tree-dump-times "= -" 1 "cddce1" } } */ +/* { dg-final { scan-tree-dump-times "= ABS_EXPR" 2 "cddce1" } } */

### Re: [PATCH] Fix PR55152

On Tue, 4 Oct 2016, Joseph Myers wrote: > On Tue, 4 Oct 2016, Richard Biener wrote: > > > On Tue, 4 Oct 2016, Joseph Myers wrote: > > > > > On Tue, 4 Oct 2016, Richard Biener wrote: > > > > > > > Possibly. Though then for FP we also want - abs (a) -> copysign (a, > > > > -1). > > > > > > For architectures such as powerpc that have a negated-abs instruction, > > > does it get properly generated from the copysign code? (The relevant > > > pattern in rs6000.md uses (neg (abs)) - is that the correct canonical RTL > > > for this?) > > > > I have no idea - given that there is no copysign RTL code I suppose > > it is the only machine independent RTL that can do this? > > > > The question would be whether the copysign optab of said targets would > > special-case the -1 case appropriately. > > Why -1? Any constant in copysign should be handled appropriately. I'd > say that (neg (abs)) is probably better as a canonical representation, so > map copysign from a constant to either (abs) or (neg (abs)) appropriately. > > Then in the case where abs is expanded with bit manipulation, (neg (abs)) > should be expanded to a single OR. I don't know whether the RTL > optimizers will map the AND / OR combination to a single OR, but if they > do then there should be no need to special-case (neg (abs)) in expansion, > and when (abs) RTL is generated a machine description's (neg (abs)) > pattern should match automatically. Ok, that's a good point -- I'll do copysign (X, const) canonicalization to [-]abs() then. Hopefully that's a valid transform even if NaNs and signed zeros are involved. For now I've installed the requested min(a,-a)->-abs(a) pattern. Bootstrapped / tested on x86_64-unknown-linux-gnu. Richard. 2016-10-05 Richard BienerPR middle-end/55152 * match.pd (min(a,-a) -> -abs(a)): New pattern. * gcc.dg/pr55152-2.c: New testcase. Index: gcc/match.pd === --- gcc/match.pd(revision 240738) +++ gcc/match.pd(working copy) @@ -1291,6 +1302,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && (! ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_UNDEFINED (type))) (abs @0))) +/* min(a,-a) -> -abs(a). */ +(simplify + (min:c @0 (negate @0)) + (if (TREE_CODE (type) != COMPLEX_TYPE + && (! ANY_INTEGRAL_TYPE_P (type) + || TYPE_OVERFLOW_UNDEFINED (type))) + (negate (abs @0 (simplify (min @0 @1) (switch Index: gcc/testsuite/gcc.dg/pr55152-2.c === --- gcc/testsuite/gcc.dg/pr55152-2.c(revision 0) +++ gcc/testsuite/gcc.dg/pr55152-2.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" } } */

### Re: [PATCH] Fix PR55152

On Tue, 4 Oct 2016, Richard Biener wrote: > On Tue, 4 Oct 2016, Joseph Myers wrote: > > > On Tue, 4 Oct 2016, Richard Biener wrote: > > > > > Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). > > > > For architectures such as powerpc that have a negated-abs instruction, > > does it get properly generated from the copysign code? (The relevant > > pattern in rs6000.md uses (neg (abs)) - is that the correct canonical RTL > > for this?) > > I have no idea - given that there is no copysign RTL code I suppose > it is the only machine independent RTL that can do this? > > The question would be whether the copysign optab of said targets would > special-case the -1 case appropriately. Why -1? Any constant in copysign should be handled appropriately. I'd say that (neg (abs)) is probably better as a canonical representation, so map copysign from a constant to either (abs) or (neg (abs)) appropriately. Then in the case where abs is expanded with bit manipulation, (neg (abs)) should be expanded to a single OR. I don't know whether the RTL optimizers will map the AND / OR combination to a single OR, but if they do then there should be no need to special-case (neg (abs)) in expansion, and when (abs) RTL is generated a machine description's (neg (abs)) pattern should match automatically. -- Joseph S. Myers jos...@codesourcery.com

### Re: [PATCH] Fix PR55152

On Tue, 4 Oct 2016, Joseph Myers wrote: > On Tue, 4 Oct 2016, Richard Biener wrote: > > > Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). > > For architectures such as powerpc that have a negated-abs instruction, > does it get properly generated from the copysign code? (The relevant > pattern in rs6000.md uses (neg (abs)) - is that the correct canonical RTL > for this?) I have no idea - given that there is no copysign RTL code I suppose it is the only machine independent RTL that can do this? The question would be whether the copysign optab of said targets would special-case the -1 case appropriately. Given your comment I'm putting the copysign part of the patch on the side for now. And I have to re-do the other part then. Richard.

### Re: [PATCH] Fix PR55152

On Tue, 4 Oct 2016, Richard Biener wrote: > Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). For architectures such as powerpc that have a negated-abs instruction, does it get properly generated from the copysign code? (The relevant pattern in rs6000.md uses (neg (abs)) - is that the correct canonical RTL for this?) -- Joseph S. Myers jos...@codesourcery.com

### Re: [PATCH] Fix PR55152

On Tue, 4 Oct 2016, Marc Glisse wrote: > On Tue, 4 Oct 2016, Richard Biener wrote: > > > Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). > > I thought this might fix PR 62055, but at least on x86_64, we generate much > worse code for copysign(,-1) than for -abs :-( I would have expected copysign(,-1) to be a simple IOR ... double foo (double x) { return __builtin_copysign (x, -1.); } foo: .LFB0: .cfi_startproc movsd .LC0(%rip), %xmm1 movapd %xmm1, %xmm2 andpd .LC2(%rip), %xmm2 andpd .LC1(%rip), %xmm0 orpd%xmm2, %xmm0 ret ICK. -fabs (x) yields foo: .LFB0: .cfi_startproc andpd .LC0(%rip), %xmm0 xorpd .LC1(%rip), %xmm0 ret I expected a simple orpd .LC0(%rip), %xmm0 ... Richard.

### Re: [PATCH] Fix PR55152

On Tue, 4 Oct 2016, Richard Biener wrote: Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). I thought this might fix PR 62055, but at least on x86_64, we generate much worse code for copysign(,-1) than for -abs :-( -- Marc Glisse

### Re: [PATCH] Fix PR55152

On Fri, 30 Sep 2016, Andrew Pinski wrote: > On Thu, Sep 29, 2016 at 8:23 PM, Richard Bienerwrote: > > 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 > > > > PR middle-end/55152 > > * match.pd: Add max(a,-a) -> abs(a) pattern. > > > Hmm, shouldn't we also do "min(a, -a) -> - abs(a)" ? Possibly. Though then for FP we also want - abs (a) -> copysign (a, -1). Testing the following. Richard. 2016-10-04 Richard Biener PR middle-end/55152 * match.pd (- abs (x) -> copysign (x, -1)): New pattern. (min(a,-a) -> -abs(a)): Likewise. * gcc.dg/pr55152-2.c: New testcase. Index: gcc/match.pd === --- gcc/match.pd(revision 240738) +++ gcc/match.pd(working copy) @@ -782,6 +782,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (simplify (abs tree_expr_nonnegative_p@0) @0) +/* - abs (x) -> copysign (x, -1). */ +(simplify + (negate (abs @0)) + (if (SCALAR_FLOAT_TYPE_P (type)) + (switch + (if (types_match (type, float_type_node)) +(BUILT_IN_COPYSIGNF @0 { build_minus_one_cst (type); })) + (if (types_match (type, double_type_node)) +(BUILT_IN_COPYSIGN @0 { build_minus_one_cst (type); })) + (if (types_match (type, long_double_type_node)) +(BUILT_IN_COPYSIGNL @0 { build_minus_one_cst (type); }) /* A few cases of fold-const.c negate_expr_p predicate. */ (match negate_expr_p @@ -1291,6 +1302,13 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && (! ANY_INTEGRAL_TYPE_P (type) || TYPE_OVERFLOW_UNDEFINED (type))) (abs @0))) +/* min(a,-a) -> -abs(a). */ +(simplify + (min:c @0 (negate @0)) + (if (TREE_CODE (type) != COMPLEX_TYPE + && (! ANY_INTEGRAL_TYPE_P (type) + || TYPE_OVERFLOW_UNDEFINED (type))) + (negate (abs @0 (simplify (min @0 @1) (switch Index: gcc/testsuite/gcc.dg/pr55152-2.c === --- gcc/testsuite/gcc.dg/pr55152-2.c(revision 0) +++ gcc/testsuite/gcc.dg/pr55152-2.c(working copy) @@ -0,0 +1,14 @@ +/* { 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" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "copysign" 1 "optimized" } } */

### Re: [PATCH] Fix PR55152

On Thu, Sep 29, 2016 at 8:23 PM, Richard Bienerwrote: > 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 > > 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 (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) > {

### Re: [PATCH] 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 BienerPR 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. 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 (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) {

### Re: [PATCH] Fix PR55152

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. Thanks, Richard.

### Re: [PATCH] Fix PR55152

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. -- Joseph S. Myers jos...@codesourcery.com