* 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).