* ORDERED_EXPR in invert_tree_comparison
@ 2012-07-19 15:52 Marc Glisse
2012-08-01 19:21 ` Marc Glisse
0 siblings, 1 reply; 8+ messages in thread
From: Marc Glisse @ 2012-07-19 15:52 UTC (permalink / raw)
To: gcc-patches
Hello,
the simple patch below passes the testsuite after a c,c++ bootstrap
without new regressions. Note however that
#include <math.h>
int f(double a, double b){
return (!isunordered(a,b))&&(a<b);
}
is then optimized by ifcombine to "return (a<b);", which seems wrong in
the absence of -fno-trapping-math. I don't know if there are ways to
trigger this latent bug without the patch.
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.
--- fold-const.c (revision 189622)
+++ fold-const.c (working copy)
@@ -2096,13 +2096,14 @@ pedantic_non_lvalue_loc (location_t loc,
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. */
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;
--
Marc Glisse
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: ORDERED_EXPR in invert_tree_comparison 2012-07-19 15:52 ORDERED_EXPR in invert_tree_comparison Marc Glisse @ 2012-08-01 19:21 ` Marc Glisse 2012-08-02 8:51 ` Richard Guenther 0 siblings, 1 reply; 8+ messages in thread From: Marc Glisse @ 2012-08-01 19:21 UTC (permalink / raw) To: gcc-patches 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). On Thu, 19 Jul 2012, Marc Glisse wrote: > Hello, > > the simple patch below passes the testsuite after a c,c++ bootstrap without > new regressions. Note however that > > #include <math.h> > int f(double a, double b){ > return (!isunordered(a,b))&&(a<b); > } > > is then optimized by ifcombine to "return (a<b);", which seems wrong in the > absence of -fno-trapping-math. I don't know if there are ways to trigger this > latent bug without the patch. > > > > 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. > > --- fold-const.c (revision 189622) > +++ fold-const.c (working copy) > @@ -2096,13 +2096,14 @@ pedantic_non_lvalue_loc (location_t loc, > 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. */ > > 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; -- Marc Glisse ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ORDERED_EXPR in invert_tree_comparison 2012-08-01 19:21 ` Marc Glisse @ 2012-08-02 8:51 ` Richard Guenther 2012-08-02 12:48 ` Marc Glisse 0 siblings, 1 reply; 8+ messages in thread From: Richard Guenther @ 2012-08-02 8:51 UTC (permalink / raw) To: Marc Glisse; +Cc: gcc-patches 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. Thanks, Richard. > > On Thu, 19 Jul 2012, Marc Glisse wrote: > >> Hello, >> >> the simple patch below passes the testsuite after a c,c++ bootstrap >> without new regressions. Note however that >> >> #include <math.h> >> int f(double a, double b){ >> return (!isunordered(a,b))&&(a<b); >> } >> >> is then optimized by ifcombine to "return (a<b);", which seems wrong in >> the absence of -fno-trapping-math. I don't know if there are ways to trigger >> this latent bug without the patch. >> >> >> >> 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. >> >> --- fold-const.c (revision 189622) >> +++ fold-const.c (working copy) >> @@ -2096,13 +2096,14 @@ pedantic_non_lvalue_loc (location_t loc, >> 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. */ >> >> 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; > > > -- > Marc Glisse ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ORDERED_EXPR in invert_tree_comparison 2012-08-02 8:51 ` Richard Guenther @ 2012-08-02 12:48 ` Marc Glisse 2012-08-02 13:37 ` Richard Guenther 2012-08-02 13:56 ` Nathan Froyd 0 siblings, 2 replies; 8+ messages in thread From: Marc Glisse @ 2012-08-02 12:48 UTC (permalink / raw) To: Richard Guenther; +Cc: gcc-patches [-- Attachment #1: Type: TEXT/PLAIN, Size: 1050 bytes --] 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. 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 [-- Attachment #2: Type: TEXT/PLAIN, Size: 1972 bytes --] 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); } \f /* 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; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ORDERED_EXPR in invert_tree_comparison 2012-08-02 12:48 ` Marc Glisse @ 2012-08-02 13:37 ` Richard Guenther 2012-08-02 13:56 ` Nathan Froyd 1 sibling, 0 replies; 8+ messages in thread From: Richard Guenther @ 2012-08-02 13:37 UTC (permalink / raw) To: Marc Glisse; +Cc: gcc-patches 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; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ORDERED_EXPR in invert_tree_comparison 2012-08-02 12:48 ` Marc Glisse 2012-08-02 13:37 ` Richard Guenther @ 2012-08-02 13:56 ` Nathan Froyd 2012-08-02 15:20 ` Marc Glisse 1 sibling, 1 reply; 8+ messages in thread From: Nathan Froyd @ 2012-08-02 13:56 UTC (permalink / raw) To: Marc Glisse; +Cc: Richard Guenther, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ORDERED_EXPR in invert_tree_comparison 2012-08-02 13:56 ` Nathan Froyd @ 2012-08-02 15:20 ` Marc Glisse 2012-08-02 15:24 ` Nathan Froyd 0 siblings, 1 reply; 8+ messages in thread From: Marc Glisse @ 2012-08-02 15:20 UTC (permalink / raw) To: Nathan Froyd; +Cc: Richard Guenther, gcc-patches 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." ? -- Marc Glisse ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: ORDERED_EXPR in invert_tree_comparison 2012-08-02 15:20 ` Marc Glisse @ 2012-08-02 15:24 ` Nathan Froyd 0 siblings, 0 replies; 8+ messages in thread From: Nathan Froyd @ 2012-08-02 15:24 UTC (permalink / raw) To: Marc Glisse; +Cc: Richard Guenther, gcc-patches 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-08-02 15:24 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-07-19 15:52 ORDERED_EXPR in invert_tree_comparison Marc Glisse 2012-08-01 19:21 ` Marc Glisse 2012-08-02 8:51 ` Richard Guenther 2012-08-02 12:48 ` Marc Glisse 2012-08-02 13:37 ` Richard Guenther 2012-08-02 13:56 ` Nathan Froyd 2012-08-02 15:20 ` Marc Glisse 2012-08-02 15:24 ` Nathan Froyd
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).