| Message ID | alpine.DEB.2.02.1208021438370.7013@stedding.saclay.inria.fr |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 2, 2012 at 2:48 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Thu, 2 Aug 2012, Richard Guenther wrote: > >> On Wed, Aug 1, 2012 at 9:21 PM, Marc Glisse <marc.glisse@inria.fr> wrote: >>> >>> Hello, >>> >>> an opinion on this? >>> >>> (I just noticed: I'll update the list in the comment visible at the top >>> of >>> the patch if this gets in). >> >> >> It looks ok to me but I am no floating-point expert. Can you add a >> testcase? >> >> Ok with that change. > > > Here again with a testcase. The -O is not necessary for the optimization to > happen, but it seemed wrong to me not to include it. I wondered about adding > an explicit -ftrapping-math, for documentation purposes. > > I am redoing the bootstrap+regtest, then I'll commit if I don't hear > protests about the testcase. Yes, an explicit -ftrapping-math would be good. Thanks, Richard. > gcc/ChangeLog > > 2012-06-15 Marc Glisse <marc.glisse@inria.fr> > > PR tree-optimization/53805 > * fold-const.c (invert_tree_comparison): Do invert ORDERED_EXPR and > UNORDERED_EXPR for floating point. > > gcc/testsuite/ChangeLog > > 2012-06-15 Marc Glisse <marc.glisse@inria.fr> > > PR tree-optimization/53805 > * gcc.dg/fold-notunord.c: New testcase. > > -- > Marc Glisse > Index: gcc/testsuite/gcc.dg/fold-notunord.c > =================================================================== > --- gcc/testsuite/gcc.dg/fold-notunord.c (revision 0) > +++ gcc/testsuite/gcc.dg/fold-notunord.c (revision 0) > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O -fdump-tree-optimized" } */ > + > +int f (double d) > +{ > + return !__builtin_isnan (d); > +} > + > +/* { dg-final { scan-tree-dump " ord " "optimized" } } */ > +/* { dg-final { cleanup-tree-dump "optimized" } } */ > > Property changes on: gcc/testsuite/gcc.dg/fold-notunord.c > ___________________________________________________________________ > Added: svn:eol-style > + native > Added: svn:keywords > + Author Date Id Revision URL > > Index: gcc/fold-const.c > =================================================================== > --- gcc/fold-const.c (revision 190071) > +++ gcc/fold-const.c (working copy) > @@ -2087,26 +2087,28 @@ static tree > pedantic_non_lvalue_loc (location_t loc, tree x) > { > if (pedantic_lvalues) > return non_lvalue_loc (loc, x); > > return protected_set_expr_location_unshare (x, loc); > } > > /* Given a tree comparison code, return the code that is the logical > inverse. > It is generally not safe to do this for floating-point comparisons, > except > - for EQ_EXPR and NE_EXPR, so we return ERROR_MARK in this case. */ > + for EQ_EXPR, NE_EXPR, ORDERED_EXPR and UNORDERED_EXPR, so we return > + ERROR_MARK in this case. */ > > enum tree_code > invert_tree_comparison (enum tree_code code, bool honor_nans) > { > - if (honor_nans && flag_trapping_math && code != EQ_EXPR && code != > NE_EXPR) > + if (honor_nans && flag_trapping_math && code != EQ_EXPR && code != > NE_EXPR > + && code != ORDERED_EXPR && code != UNORDERED_EXPR) > return ERROR_MARK; > > switch (code) > { > case EQ_EXPR: > return NE_EXPR; > case NE_EXPR: > return EQ_EXPR; > case GT_EXPR: > return honor_nans ? UNLE_EXPR : LE_EXPR; >
On Thu, Aug 02, 2012 at 02:48:08PM +0200, Marc Glisse wrote: > I am redoing the bootstrap+regtest, then I'll commit if I don't hear > protests about the testcase. > > gcc/ChangeLog > 2012-06-15 Marc Glisse <marc.glisse@inria.fr> > > PR tree-optimization/53805 > * fold-const.c (invert_tree_comparison): Do invert ORDERED_EXPR and > UNORDERED_EXPR for floating point. Minor protest about the ChangeLog: I think you mean "Do _not_ invert..." -Nathan
On Thu, 2 Aug 2012, Nathan Froyd wrote: > On Thu, Aug 02, 2012 at 02:48:08PM +0200, Marc Glisse wrote: >> I am redoing the bootstrap+regtest, then I'll commit if I don't hear >> protests about the testcase. >> >> gcc/ChangeLog >> 2012-06-15 Marc Glisse <marc.glisse@inria.fr> >> >> PR tree-optimization/53805 >> * fold-const.c (invert_tree_comparison): Do invert ORDERED_EXPR and >> UNORDERED_EXPR for floating point. > > Minor protest about the ChangeLog: I think you mean "Do _not_ invert..." No, I do mean do invert. The point of the patch is that even for floating point and even with trapping-math, it is still safe to invert them. Maybe I can reformulate as: "Invert ORDERED_EXPR and UNORDERED_EXPR even for trapping floating point." ?
On Thu, Aug 02, 2012 at 05:20:24PM +0200, Marc Glisse wrote: > On Thu, 2 Aug 2012, Nathan Froyd wrote: > >> PR tree-optimization/53805 > >> * fold-const.c (invert_tree_comparison): Do invert ORDERED_EXPR and > >> UNORDERED_EXPR for floating point. > > > >Minor protest about the ChangeLog: I think you mean "Do _not_ invert..." > > No, I do mean do invert. The point of the patch is that even for > floating point and even with trapping-math, it is still safe to > invert them. Ahhh, yes. I misread the patch. > Maybe I can reformulate as: "Invert ORDERED_EXPR and UNORDERED_EXPR > even for trapping floating point." ? That works for me. -Nathan
Index: gcc/testsuite/gcc.dg/fold-notunord.c =================================================================== --- gcc/testsuite/gcc.dg/fold-notunord.c (revision 0) +++ gcc/testsuite/gcc.dg/fold-notunord.c (revision 0) @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O -fdump-tree-optimized" } */ + +int f (double d) +{ + return !__builtin_isnan (d); +} + +/* { dg-final { scan-tree-dump " ord " "optimized" } } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ Property changes on: gcc/testsuite/gcc.dg/fold-notunord.c ___________________________________________________________________ Added: svn:eol-style + native Added: svn:keywords + Author Date Id Revision URL Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c (revision 190071) +++ gcc/fold-const.c (working copy) @@ -2087,26 +2087,28 @@ static tree pedantic_non_lvalue_loc (location_t loc, tree x) { if (pedantic_lvalues) return non_lvalue_loc (loc, x); return protected_set_expr_location_unshare (x, loc); } /* Given a tree comparison code, return the code that is the logical inverse. It is generally not safe to do this for floating-point comparisons, except - for EQ_EXPR and NE_EXPR, so we return ERROR_MARK in this case. */ + for EQ_EXPR, NE_EXPR, ORDERED_EXPR and UNORDERED_EXPR, so we return + ERROR_MARK in this case. */ enum tree_code invert_tree_comparison (enum tree_code code, bool honor_nans) { - if (honor_nans && flag_trapping_math && code != EQ_EXPR && code != NE_EXPR) + if (honor_nans && flag_trapping_math && code != EQ_EXPR && code != NE_EXPR + && code != ORDERED_EXPR && code != UNORDERED_EXPR) return ERROR_MARK; switch (code) { case EQ_EXPR: return NE_EXPR; case NE_EXPR: return EQ_EXPR; case GT_EXPR: return honor_nans ? UNLE_EXPR : LE_EXPR;