* Fix 61441
@ 2015-09-01 10:23 Sujoy Saraswati
2015-09-01 12:21 ` Richard Biener
0 siblings, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-09-01 10:23 UTC (permalink / raw)
To: gcc-patches
The following patch fixes 61441. It converts sNaN to qNaN on folding
when -fno-signaling-nans is used.
Bootstrap and regression tests on x86_64-linux-gnu and
aarch64-unknown-linux-gnu passed with changes done on trunk.
Is this fix fine ?
Regards,
Sujoy
2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
PR tree-optimization/61441
* tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
flag_signaling_nans is off.
(ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
PR tree-optimization/61441
* gcc.dg/pr61441.c: New testcase.
Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c (revision 226965)
+++ gcc/tree-ssa-ccp.c (working copy)
@@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
return 0;
}
+/* Convert sNaN to qNaN when flag_signaling_nans is off */
+
+static void
+convert_snan_to_qnan (tree expr)
+{
+ if (expr
+ && (TREE_CODE (expr) == REAL_CST)
+ && !flag_signaling_nans)
+ {
+ REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
+
+ if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
+ && REAL_VALUE_ISNAN (*d)
+ && d->signalling)
+ d->signalling = 0;
+ }
+}
+
/* Return the value for the address expression EXPR based on alignment
information. */
@@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
if (val.lattice_val != CONSTANT
|| val.mask != 0)
return false;
+ convert_snan_to_qnan (val.value);
if (dump_file)
{
@@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
bool res;
if (!useless_type_conversion_p (TREE_TYPE (lhs),
TREE_TYPE (new_rhs)))
+ {
new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
+ convert_snan_to_qnan (new_rhs);
+ }
res = update_call_from_tree (gsi, new_rhs);
gcc_assert (res);
return true;
@@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
tree new_rhs = fold_builtin_alloca_with_align (stmt);
if (new_rhs)
{
+ convert_snan_to_qnan (new_rhs);
bool res = update_call_from_tree (gsi, new_rhs);
tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
gcc_assert (res);
@@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
{
tree rhs = unshare_expr (val);
if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
+ {
rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
+ convert_snan_to_qnan (rhs);
+ }
gimple_assign_set_rhs_from_tree (gsi, rhs);
return true;
}
@@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
/* Evaluate the statement, which could be
either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
val = evaluate_stmt (stmt);
+ convert_snan_to_qnan (val.value);
/* If STMT is an assignment to an SSA_NAME, we only have one
value to set. */
@@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
if (val.lattice_val != CONSTANT
|| val.mask != 0)
return SSA_PROP_VARYING;
+ convert_snan_to_qnan (val.value);
/* Find which edge out of the conditional block will be taken and add it
to the worklist. If no single edge can be determined statically,
Index: gcc/testsuite/gcc.dg/pr61441.c
===================================================================
--- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
@@ -0,0 +1,17 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -lm" } */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <math.h>
+
+int main (void)
+{
+ float sNaN = __builtin_nansf ("");
+ double x = (double) sNaN;
+ if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+ return 0;
+}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-01 10:23 Fix 61441 Sujoy Saraswati
@ 2015-09-01 12:21 ` Richard Biener
2015-09-02 11:36 ` Sujoy Saraswati
0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2015-09-01 12:21 UTC (permalink / raw)
To: Sujoy Saraswati; +Cc: GCC Patches
On Tue, Sep 1, 2015 at 12:23 PM, Sujoy Saraswati <ssaraswati@gmail.com> wrote:
> The following patch fixes 61441. It converts sNaN to qNaN on folding
> when -fno-signaling-nans is used.
>
> Bootstrap and regression tests on x86_64-linux-gnu and
> aarch64-unknown-linux-gnu passed with changes done on trunk.
>
> Is this fix fine ?
Note that I'm curious what
the actual bug is - is it that (double) sNaN creates a sNaN? Then the fix
should be elsewhere, in constant folding itself
(fold_convert_const_real_from_real
or real_convert).
If that isn't the bug you have very many other passes to fix for the
same problem.
So - can you please explain?
Thanks,
Richard.
> Regards,
> Sujoy
>
> 2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
>
> PR tree-optimization/61441
> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
> flag_signaling_nans is off.
> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
> convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
>
> PR tree-optimization/61441
> * gcc.dg/pr61441.c: New testcase.
>
> Index: gcc/tree-ssa-ccp.c
> ===================================================================
> --- gcc/tree-ssa-ccp.c (revision 226965)
> +++ gcc/tree-ssa-ccp.c (working copy)
> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
> return 0;
> }
>
> +/* Convert sNaN to qNaN when flag_signaling_nans is off */
> +
> +static void
> +convert_snan_to_qnan (tree expr)
> +{
> + if (expr
> + && (TREE_CODE (expr) == REAL_CST)
> + && !flag_signaling_nans)
> + {
> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
> +
> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
> + && REAL_VALUE_ISNAN (*d)
> + && d->signalling)
> + d->signalling = 0;
> + }
> +}
> +
> /* Return the value for the address expression EXPR based on alignment
> information. */
>
> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
> if (val.lattice_val != CONSTANT
> || val.mask != 0)
> return false;
> + convert_snan_to_qnan (val.value);
>
> if (dump_file)
> {
> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
> bool res;
> if (!useless_type_conversion_p (TREE_TYPE (lhs),
> TREE_TYPE (new_rhs)))
> + {
> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
> + convert_snan_to_qnan (new_rhs);
> + }
> res = update_call_from_tree (gsi, new_rhs);
> gcc_assert (res);
> return true;
> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
> tree new_rhs = fold_builtin_alloca_with_align (stmt);
> if (new_rhs)
> {
> + convert_snan_to_qnan (new_rhs);
> bool res = update_call_from_tree (gsi, new_rhs);
> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
> gcc_assert (res);
> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
> {
> tree rhs = unshare_expr (val);
> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
> + {
> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
> + convert_snan_to_qnan (rhs);
> + }
> gimple_assign_set_rhs_from_tree (gsi, rhs);
> return true;
> }
> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
> /* Evaluate the statement, which could be
> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
> val = evaluate_stmt (stmt);
> + convert_snan_to_qnan (val.value);
>
> /* If STMT is an assignment to an SSA_NAME, we only have one
> value to set. */
> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
> if (val.lattice_val != CONSTANT
> || val.mask != 0)
> return SSA_PROP_VARYING;
> + convert_snan_to_qnan (val.value);
>
> /* Find which edge out of the conditional block will be taken and add it
> to the worklist. If no single edge can be determined statically,
>
> Index: gcc/testsuite/gcc.dg/pr61441.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
> @@ -0,0 +1,17 @@
> +/* { dg-do run } */
> +/* { dg-options "-O1 -lm" } */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <math.h>
> +
> +int main (void)
> +{
> + float sNaN = __builtin_nansf ("");
> + double x = (double) sNaN;
> + if (issignaling(x))
> + {
> + __builtin_abort();
> + }
> + return 0;
> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-01 12:21 ` Richard Biener
@ 2015-09-02 11:36 ` Sujoy Saraswati
2015-09-02 11:56 ` Richard Biener
0 siblings, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-09-02 11:36 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
Hi Richard,
> Note that I'm curious what
> the actual bug is - is it that (double) sNaN creates a sNaN? Then the fix
> should be elsewhere, in constant folding itself
> (fold_convert_const_real_from_real
> or real_convert).
>
> If that isn't the bug you have very many other passes to fix for the
> same problem.
>
> So - can you please explain?
In this test case, the floating point operation for converting the
float to double is what should convert the sNaN to qNaN. I tried to
cover more floating point operations than just the conversion
operations exposed by this test case. For example, if we consider the
following program -
#define _GNU_SOURCE
#include <stdio.h>
#include <math.h>
int main (void)
{
float x;
float sNaN = __builtin_nansf ("");
x = sNaN + .0;
return issignaling(x);
}
The operation (sNaN + .0) should also result in qNaN after folding.
Hence, I thought of doing the sNaN to qNaN conversion in various
places under tree-ssa-ccp, where the result upon a folding is
available. I do agree that this approach may mean many more such
places should also be covered in other passes, but thought of sending
the fix for the ccp pass to start with.
Let me know if you suggest alternate approach.
Regards,
Sujoy
> Thanks,
> Richard.
>
>> Regards,
>> Sujoy
>>
>> 2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
>>
>> PR tree-optimization/61441
>> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
>> flag_signaling_nans is off.
>> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
>> convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
>>
>> PR tree-optimization/61441
>> * gcc.dg/pr61441.c: New testcase.
>>
>> Index: gcc/tree-ssa-ccp.c
>> ===================================================================
>> --- gcc/tree-ssa-ccp.c (revision 226965)
>> +++ gcc/tree-ssa-ccp.c (working copy)
>> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
>> return 0;
>> }
>>
>> +/* Convert sNaN to qNaN when flag_signaling_nans is off */
>> +
>> +static void
>> +convert_snan_to_qnan (tree expr)
>> +{
>> + if (expr
>> + && (TREE_CODE (expr) == REAL_CST)
>> + && !flag_signaling_nans)
>> + {
>> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
>> +
>> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
>> + && REAL_VALUE_ISNAN (*d)
>> + && d->signalling)
>> + d->signalling = 0;
>> + }
>> +}
>> +
>> /* Return the value for the address expression EXPR based on alignment
>> information. */
>>
>> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>> if (val.lattice_val != CONSTANT
>> || val.mask != 0)
>> return false;
>> + convert_snan_to_qnan (val.value);
>>
>> if (dump_file)
>> {
>> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>> bool res;
>> if (!useless_type_conversion_p (TREE_TYPE (lhs),
>> TREE_TYPE (new_rhs)))
>> + {
>> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
>> + convert_snan_to_qnan (new_rhs);
>> + }
>> res = update_call_from_tree (gsi, new_rhs);
>> gcc_assert (res);
>> return true;
>> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>> tree new_rhs = fold_builtin_alloca_with_align (stmt);
>> if (new_rhs)
>> {
>> + convert_snan_to_qnan (new_rhs);
>> bool res = update_call_from_tree (gsi, new_rhs);
>> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
>> gcc_assert (res);
>> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>> {
>> tree rhs = unshare_expr (val);
>> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
>> + {
>> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
>> + convert_snan_to_qnan (rhs);
>> + }
>> gimple_assign_set_rhs_from_tree (gsi, rhs);
>> return true;
>> }
>> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
>> /* Evaluate the statement, which could be
>> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
>> val = evaluate_stmt (stmt);
>> + convert_snan_to_qnan (val.value);
>>
>> /* If STMT is an assignment to an SSA_NAME, we only have one
>> value to set. */
>> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
>> if (val.lattice_val != CONSTANT
>> || val.mask != 0)
>> return SSA_PROP_VARYING;
>> + convert_snan_to_qnan (val.value);
>>
>> /* Find which edge out of the conditional block will be taken and add it
>> to the worklist. If no single edge can be determined statically,
>>
>> Index: gcc/testsuite/gcc.dg/pr61441.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
>> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
>> @@ -0,0 +1,17 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O1 -lm" } */
>> +
>> +#define _GNU_SOURCE
>> +#include <stdio.h>
>> +#include <math.h>
>> +
>> +int main (void)
>> +{
>> + float sNaN = __builtin_nansf ("");
>> + double x = (double) sNaN;
>> + if (issignaling(x))
>> + {
>> + __builtin_abort();
>> + }
>> + return 0;
>> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-02 11:36 ` Sujoy Saraswati
@ 2015-09-02 11:56 ` Richard Biener
2015-09-02 12:16 ` Sujoy Saraswati
2015-09-10 7:33 ` Sujoy Saraswati
0 siblings, 2 replies; 17+ messages in thread
From: Richard Biener @ 2015-09-02 11:56 UTC (permalink / raw)
To: Sujoy Saraswati; +Cc: GCC Patches
On Wed, Sep 2, 2015 at 1:36 PM, Sujoy Saraswati <ssaraswati@gmail.com> wrote:
> Hi Richard,
>
>> Note that I'm curious what
>> the actual bug is - is it that (double) sNaN creates a sNaN? Then the fix
>> should be elsewhere, in constant folding itself
>> (fold_convert_const_real_from_real
>> or real_convert).
>>
>> If that isn't the bug you have very many other passes to fix for the
>> same problem.
>>
>> So - can you please explain?
>
> In this test case, the floating point operation for converting the
> float to double is what should convert the sNaN to qNaN. I tried to
> cover more floating point operations than just the conversion
> operations exposed by this test case. For example, if we consider the
> following program -
>
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <math.h>
>
> int main (void)
> {
> float x;
> float sNaN = __builtin_nansf ("");
> x = sNaN + .0;
> return issignaling(x);
> }
>
> The operation (sNaN + .0) should also result in qNaN after folding.
> Hence, I thought of doing the sNaN to qNaN conversion in various
> places under tree-ssa-ccp, where the result upon a folding is
> available. I do agree that this approach may mean many more such
> places should also be covered in other passes, but thought of sending
> the fix for the ccp pass to start with.
>
> Let me know if you suggest alternate approach.
CCP and other passes ultimatively end up using fold-const.c:const_{unop,binop}
for constant folding so that is where the fix should go to (or to real.c). That
will automatically handle other passes doing similar transforms.
Richard.
> Regards,
> Sujoy
>
>> Thanks,
>> Richard.
>>
>>> Regards,
>>> Sujoy
>>>
>>> 2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
>>>
>>> PR tree-optimization/61441
>>> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
>>> flag_signaling_nans is off.
>>> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
>>> convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
>>>
>>> PR tree-optimization/61441
>>> * gcc.dg/pr61441.c: New testcase.
>>>
>>> Index: gcc/tree-ssa-ccp.c
>>> ===================================================================
>>> --- gcc/tree-ssa-ccp.c (revision 226965)
>>> +++ gcc/tree-ssa-ccp.c (working copy)
>>> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
>>> return 0;
>>> }
>>>
>>> +/* Convert sNaN to qNaN when flag_signaling_nans is off */
>>> +
>>> +static void
>>> +convert_snan_to_qnan (tree expr)
>>> +{
>>> + if (expr
>>> + && (TREE_CODE (expr) == REAL_CST)
>>> + && !flag_signaling_nans)
>>> + {
>>> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
>>> +
>>> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
>>> + && REAL_VALUE_ISNAN (*d)
>>> + && d->signalling)
>>> + d->signalling = 0;
>>> + }
>>> +}
>>> +
>>> /* Return the value for the address expression EXPR based on alignment
>>> information. */
>>>
>>> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>> if (val.lattice_val != CONSTANT
>>> || val.mask != 0)
>>> return false;
>>> + convert_snan_to_qnan (val.value);
>>>
>>> if (dump_file)
>>> {
>>> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>> bool res;
>>> if (!useless_type_conversion_p (TREE_TYPE (lhs),
>>> TREE_TYPE (new_rhs)))
>>> + {
>>> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
>>> + convert_snan_to_qnan (new_rhs);
>>> + }
>>> res = update_call_from_tree (gsi, new_rhs);
>>> gcc_assert (res);
>>> return true;
>>> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>> tree new_rhs = fold_builtin_alloca_with_align (stmt);
>>> if (new_rhs)
>>> {
>>> + convert_snan_to_qnan (new_rhs);
>>> bool res = update_call_from_tree (gsi, new_rhs);
>>> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
>>> gcc_assert (res);
>>> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>> {
>>> tree rhs = unshare_expr (val);
>>> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
>>> + {
>>> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
>>> + convert_snan_to_qnan (rhs);
>>> + }
>>> gimple_assign_set_rhs_from_tree (gsi, rhs);
>>> return true;
>>> }
>>> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
>>> /* Evaluate the statement, which could be
>>> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
>>> val = evaluate_stmt (stmt);
>>> + convert_snan_to_qnan (val.value);
>>>
>>> /* If STMT is an assignment to an SSA_NAME, we only have one
>>> value to set. */
>>> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
>>> if (val.lattice_val != CONSTANT
>>> || val.mask != 0)
>>> return SSA_PROP_VARYING;
>>> + convert_snan_to_qnan (val.value);
>>>
>>> /* Find which edge out of the conditional block will be taken and add it
>>> to the worklist. If no single edge can be determined statically,
>>>
>>> Index: gcc/testsuite/gcc.dg/pr61441.c
>>> ===================================================================
>>> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
>>> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
>>> @@ -0,0 +1,17 @@
>>> +/* { dg-do run } */
>>> +/* { dg-options "-O1 -lm" } */
>>> +
>>> +#define _GNU_SOURCE
>>> +#include <stdio.h>
>>> +#include <math.h>
>>> +
>>> +int main (void)
>>> +{
>>> + float sNaN = __builtin_nansf ("");
>>> + double x = (double) sNaN;
>>> + if (issignaling(x))
>>> + {
>>> + __builtin_abort();
>>> + }
>>> + return 0;
>>> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-02 11:56 ` Richard Biener
@ 2015-09-02 12:16 ` Sujoy Saraswati
2015-09-10 7:33 ` Sujoy Saraswati
1 sibling, 0 replies; 17+ messages in thread
From: Sujoy Saraswati @ 2015-09-02 12:16 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
> CCP and other passes ultimatively end up using fold-const.c:const_{unop,binop}
> for constant folding so that is where the fix should go to (or to real.c). That
> will automatically handle other passes doing similar transforms.
Thanks for the tip. I will modify my fix and post it.
Regards,
Sujoy
>
> Richard.
>
>> Regards,
>> Sujoy
>>
>>> Thanks,
>>> Richard.
>>>
>>>> Regards,
>>>> Sujoy
>>>>
>>>> 2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
>>>>
>>>> PR tree-optimization/61441
>>>> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
>>>> flag_signaling_nans is off.
>>>> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
>>>> convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
>>>>
>>>> PR tree-optimization/61441
>>>> * gcc.dg/pr61441.c: New testcase.
>>>>
>>>> Index: gcc/tree-ssa-ccp.c
>>>> ===================================================================
>>>> --- gcc/tree-ssa-ccp.c (revision 226965)
>>>> +++ gcc/tree-ssa-ccp.c (working copy)
>>>> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
>>>> return 0;
>>>> }
>>>>
>>>> +/* Convert sNaN to qNaN when flag_signaling_nans is off */
>>>> +
>>>> +static void
>>>> +convert_snan_to_qnan (tree expr)
>>>> +{
>>>> + if (expr
>>>> + && (TREE_CODE (expr) == REAL_CST)
>>>> + && !flag_signaling_nans)
>>>> + {
>>>> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
>>>> +
>>>> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
>>>> + && REAL_VALUE_ISNAN (*d)
>>>> + && d->signalling)
>>>> + d->signalling = 0;
>>>> + }
>>>> +}
>>>> +
>>>> /* Return the value for the address expression EXPR based on alignment
>>>> information. */
>>>>
>>>> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> if (val.lattice_val != CONSTANT
>>>> || val.mask != 0)
>>>> return false;
>>>> + convert_snan_to_qnan (val.value);
>>>>
>>>> if (dump_file)
>>>> {
>>>> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> bool res;
>>>> if (!useless_type_conversion_p (TREE_TYPE (lhs),
>>>> TREE_TYPE (new_rhs)))
>>>> + {
>>>> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
>>>> + convert_snan_to_qnan (new_rhs);
>>>> + }
>>>> res = update_call_from_tree (gsi, new_rhs);
>>>> gcc_assert (res);
>>>> return true;
>>>> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> tree new_rhs = fold_builtin_alloca_with_align (stmt);
>>>> if (new_rhs)
>>>> {
>>>> + convert_snan_to_qnan (new_rhs);
>>>> bool res = update_call_from_tree (gsi, new_rhs);
>>>> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
>>>> gcc_assert (res);
>>>> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> {
>>>> tree rhs = unshare_expr (val);
>>>> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
>>>> + {
>>>> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
>>>> + convert_snan_to_qnan (rhs);
>>>> + }
>>>> gimple_assign_set_rhs_from_tree (gsi, rhs);
>>>> return true;
>>>> }
>>>> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
>>>> /* Evaluate the statement, which could be
>>>> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
>>>> val = evaluate_stmt (stmt);
>>>> + convert_snan_to_qnan (val.value);
>>>>
>>>> /* If STMT is an assignment to an SSA_NAME, we only have one
>>>> value to set. */
>>>> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
>>>> if (val.lattice_val != CONSTANT
>>>> || val.mask != 0)
>>>> return SSA_PROP_VARYING;
>>>> + convert_snan_to_qnan (val.value);
>>>>
>>>> /* Find which edge out of the conditional block will be taken and add it
>>>> to the worklist. If no single edge can be determined statically,
>>>>
>>>> Index: gcc/testsuite/gcc.dg/pr61441.c
>>>> ===================================================================
>>>> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
>>>> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
>>>> @@ -0,0 +1,17 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O1 -lm" } */
>>>> +
>>>> +#define _GNU_SOURCE
>>>> +#include <stdio.h>
>>>> +#include <math.h>
>>>> +
>>>> +int main (void)
>>>> +{
>>>> + float sNaN = __builtin_nansf ("");
>>>> + double x = (double) sNaN;
>>>> + if (issignaling(x))
>>>> + {
>>>> + __builtin_abort();
>>>> + }
>>>> + return 0;
>>>> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-02 11:56 ` Richard Biener
2015-09-02 12:16 ` Sujoy Saraswati
@ 2015-09-10 7:33 ` Sujoy Saraswati
2015-09-14 13:50 ` Richard Biener
1 sibling, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-09-10 7:33 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
Hi,
Here is a modified patch for this. The change this time is in
fold-const.c and real.c.
Bootstrap and regression tests on x86_64-linux-gnu and
aarch64-unknown-linux-gnu passed with changes done on trunk.
Is this fine ?
Regards,
Sujoy
2015-09-10 Sujoy Saraswati <ssaraswati@gmail.com>
PR tree-optimization/61441
* fold-const.c (const_binop, fold_abs_const): Convert
sNaN to qNaN when flag_signaling_nans is off.
* real.c (do_add, do_multiply, do_divide, do_fix_trunc): Same.
(real_arithmetic, real_ldexp, real_convert): Same
PR tree-optimization/61441
* gcc.dg/pr61441.c: New testcase.
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 227584)
+++ gcc/fold-const.c (working copy)
@@ -1183,9 +1183,19 @@ const_binop (enum tree_code code, tree arg1, tree
/* If either operand is a NaN, just return it. Otherwise, set up
for floating-point trap; we return an overflow. */
if (REAL_VALUE_ISNAN (d1))
+ {
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ (TREE_REAL_CST_PTR (arg1))->signalling = 0;
return arg1;
+ }
else if (REAL_VALUE_ISNAN (d2))
+ {
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ (TREE_REAL_CST_PTR (arg1))->signalling = 0;
return arg2;
+ }
inexact = real_arithmetic (&value, code, &d1, &d2);
real_convert (&result, mode, &value);
@@ -13644,6 +13654,9 @@ fold_abs_const (tree arg0, tree type)
t = build_real (type, real_value_negate (&TREE_REAL_CST (arg0)));
else
t = arg0;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ (TREE_REAL_CST_PTR (t))->signalling = 0;
break;
default:
Index: gcc/real.c
===================================================================
--- gcc/real.c (revision 227584)
+++ gcc/real.c (working copy)
@@ -545,6 +545,9 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
case CLASS2 (rvc_normal, rvc_inf):
/* R + Inf = Inf. */
*r = *b;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
r->sign = sign ^ subtract_p;
return false;
@@ -558,6 +561,9 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
case CLASS2 (rvc_inf, rvc_normal):
/* Inf + R = Inf. */
*r = *a;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
return false;
case CLASS2 (rvc_inf, rvc_inf):
@@ -680,6 +686,9 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
case CLASS2 (rvc_nan, rvc_nan):
/* ANY * NaN = NaN. */
*r = *b;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -688,6 +697,9 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
case CLASS2 (rvc_nan, rvc_inf):
/* NaN * ANY = NaN. */
*r = *a;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -830,6 +842,9 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
case CLASS2 (rvc_nan, rvc_nan):
/* ANY / NaN = NaN. */
*r = *b;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -838,6 +853,9 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
case CLASS2 (rvc_nan, rvc_inf):
/* NaN / ANY = NaN. */
*r = *a;
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -968,6 +986,9 @@ do_fix_trunc (REAL_VALUE_TYPE *r, const REAL_VALUE
case rvc_zero:
case rvc_inf:
case rvc_nan:
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
break;
case rvc_normal:
@@ -1059,6 +1080,11 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co
default:
gcc_unreachable ();
}
+
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
+
return false;
}
@@ -1150,6 +1176,9 @@ real_ldexp (REAL_VALUE_TYPE *r, const REAL_VALUE_T
case rvc_zero:
case rvc_inf:
case rvc_nan:
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (!flag_signaling_nans)
+ r->signalling = 0;
break;
case rvc_normal:
@@ -2718,6 +2747,10 @@ real_convert (REAL_VALUE_TYPE *r, machine_mode mod
round_for_format (fmt, r);
+ /* Convert sNaN to qNaN when flag_signaling_nans is off */
+ if (HONOR_NANS (mode) && !flag_signaling_nans)
+ r->signalling = 0;
+
/* round_for_format de-normalizes denormals. Undo just that part. */
if (r->cl == rvc_normal)
normalize (r);
Index: gcc/testsuite/gcc.dg/pr61441.c
===================================================================
--- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
@@ -0,0 +1,34 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -lm" } */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <math.h>
+
+void conversion()
+{
+ float sNaN = __builtin_nansf ("");
+ double x = (double) sNaN;
+ if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+}
+
+void addition()
+{
+ float x;
+ float sNaN = __builtin_nansf ("");
+ x = sNaN + .0;
+ if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+}
+
+int main (void)
+{
+ conversion();
+ addition();
+ return 0;
+}
On Wed, Sep 2, 2015 at 5:26 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Sep 2, 2015 at 1:36 PM, Sujoy Saraswati <ssaraswati@gmail.com> wrote:
>> Hi Richard,
>>
>>> Note that I'm curious what
>>> the actual bug is - is it that (double) sNaN creates a sNaN? Then the fix
>>> should be elsewhere, in constant folding itself
>>> (fold_convert_const_real_from_real
>>> or real_convert).
>>>
>>> If that isn't the bug you have very many other passes to fix for the
>>> same problem.
>>>
>>> So - can you please explain?
>>
>> In this test case, the floating point operation for converting the
>> float to double is what should convert the sNaN to qNaN. I tried to
>> cover more floating point operations than just the conversion
>> operations exposed by this test case. For example, if we consider the
>> following program -
>>
>> #define _GNU_SOURCE
>> #include <stdio.h>
>> #include <math.h>
>>
>> int main (void)
>> {
>> float x;
>> float sNaN = __builtin_nansf ("");
>> x = sNaN + .0;
>> return issignaling(x);
>> }
>>
>> The operation (sNaN + .0) should also result in qNaN after folding.
>> Hence, I thought of doing the sNaN to qNaN conversion in various
>> places under tree-ssa-ccp, where the result upon a folding is
>> available. I do agree that this approach may mean many more such
>> places should also be covered in other passes, but thought of sending
>> the fix for the ccp pass to start with.
>>
>> Let me know if you suggest alternate approach.
>
> CCP and other passes ultimatively end up using fold-const.c:const_{unop,binop}
> for constant folding so that is where the fix should go to (or to real.c). That
> will automatically handle other passes doing similar transforms.
>
> Richard.
>
>> Regards,
>> Sujoy
>>
>>> Thanks,
>>> Richard.
>>>
>>>> Regards,
>>>> Sujoy
>>>>
>>>> 2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
>>>>
>>>> PR tree-optimization/61441
>>>> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
>>>> flag_signaling_nans is off.
>>>> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
>>>> convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
>>>>
>>>> PR tree-optimization/61441
>>>> * gcc.dg/pr61441.c: New testcase.
>>>>
>>>> Index: gcc/tree-ssa-ccp.c
>>>> ===================================================================
>>>> --- gcc/tree-ssa-ccp.c (revision 226965)
>>>> +++ gcc/tree-ssa-ccp.c (working copy)
>>>> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
>>>> return 0;
>>>> }
>>>>
>>>> +/* Convert sNaN to qNaN when flag_signaling_nans is off */
>>>> +
>>>> +static void
>>>> +convert_snan_to_qnan (tree expr)
>>>> +{
>>>> + if (expr
>>>> + && (TREE_CODE (expr) == REAL_CST)
>>>> + && !flag_signaling_nans)
>>>> + {
>>>> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
>>>> +
>>>> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
>>>> + && REAL_VALUE_ISNAN (*d)
>>>> + && d->signalling)
>>>> + d->signalling = 0;
>>>> + }
>>>> +}
>>>> +
>>>> /* Return the value for the address expression EXPR based on alignment
>>>> information. */
>>>>
>>>> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> if (val.lattice_val != CONSTANT
>>>> || val.mask != 0)
>>>> return false;
>>>> + convert_snan_to_qnan (val.value);
>>>>
>>>> if (dump_file)
>>>> {
>>>> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> bool res;
>>>> if (!useless_type_conversion_p (TREE_TYPE (lhs),
>>>> TREE_TYPE (new_rhs)))
>>>> + {
>>>> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
>>>> + convert_snan_to_qnan (new_rhs);
>>>> + }
>>>> res = update_call_from_tree (gsi, new_rhs);
>>>> gcc_assert (res);
>>>> return true;
>>>> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> tree new_rhs = fold_builtin_alloca_with_align (stmt);
>>>> if (new_rhs)
>>>> {
>>>> + convert_snan_to_qnan (new_rhs);
>>>> bool res = update_call_from_tree (gsi, new_rhs);
>>>> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
>>>> gcc_assert (res);
>>>> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>> {
>>>> tree rhs = unshare_expr (val);
>>>> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
>>>> + {
>>>> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
>>>> + convert_snan_to_qnan (rhs);
>>>> + }
>>>> gimple_assign_set_rhs_from_tree (gsi, rhs);
>>>> return true;
>>>> }
>>>> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
>>>> /* Evaluate the statement, which could be
>>>> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
>>>> val = evaluate_stmt (stmt);
>>>> + convert_snan_to_qnan (val.value);
>>>>
>>>> /* If STMT is an assignment to an SSA_NAME, we only have one
>>>> value to set. */
>>>> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
>>>> if (val.lattice_val != CONSTANT
>>>> || val.mask != 0)
>>>> return SSA_PROP_VARYING;
>>>> + convert_snan_to_qnan (val.value);
>>>>
>>>> /* Find which edge out of the conditional block will be taken and add it
>>>> to the worklist. If no single edge can be determined statically,
>>>>
>>>> Index: gcc/testsuite/gcc.dg/pr61441.c
>>>> ===================================================================
>>>> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
>>>> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
>>>> @@ -0,0 +1,17 @@
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O1 -lm" } */
>>>> +
>>>> +#define _GNU_SOURCE
>>>> +#include <stdio.h>
>>>> +#include <math.h>
>>>> +
>>>> +int main (void)
>>>> +{
>>>> + float sNaN = __builtin_nansf ("");
>>>> + double x = (double) sNaN;
>>>> + if (issignaling(x))
>>>> + {
>>>> + __builtin_abort();
>>>> + }
>>>> + return 0;
>>>> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-10 7:33 ` Sujoy Saraswati
@ 2015-09-14 13:50 ` Richard Biener
2015-09-14 20:39 ` Joseph Myers
0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2015-09-14 13:50 UTC (permalink / raw)
To: Sujoy Saraswati, Joseph S. Myers; +Cc: GCC Patches
On Thu, Sep 10, 2015 at 9:30 AM, Sujoy Saraswati <ssaraswati@gmail.com> wrote:
> Hi,
> Here is a modified patch for this. The change this time is in
> fold-const.c and real.c.
>
> Bootstrap and regression tests on x86_64-linux-gnu and
> aarch64-unknown-linux-gnu passed with changes done on trunk.
>
> Is this fine ?
>
> Regards,
> Sujoy
>
> 2015-09-10 Sujoy Saraswati <ssaraswati@gmail.com>
>
> PR tree-optimization/61441
> * fold-const.c (const_binop, fold_abs_const): Convert
> sNaN to qNaN when flag_signaling_nans is off.
> * real.c (do_add, do_multiply, do_divide, do_fix_trunc): Same.
> (real_arithmetic, real_ldexp, real_convert): Same
>
> PR tree-optimization/61441
> * gcc.dg/pr61441.c: New testcase.
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 227584)
> +++ gcc/fold-const.c (working copy)
> @@ -1183,9 +1183,19 @@ const_binop (enum tree_code code, tree arg1, tree
> /* If either operand is a NaN, just return it. Otherwise, set up
> for floating-point trap; we return an overflow. */
> if (REAL_VALUE_ISNAN (d1))
> + {
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + (TREE_REAL_CST_PTR (arg1))->signalling = 0;
As REAL_CSTs can be shared you need to build a new one, you can't
modify it in place.
I'll leave the correctness part of the patch to Joseph who knows FP
arithmetic better than me,
implementation-wise this is ok if you fix the REAL_CST sharing issue.
Thanks,
Richard.
> return arg1;
> + }
> else if (REAL_VALUE_ISNAN (d2))
> + {
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + (TREE_REAL_CST_PTR (arg1))->signalling = 0;
> return arg2;
> + }
>
> inexact = real_arithmetic (&value, code, &d1, &d2);
> real_convert (&result, mode, &value);
> @@ -13644,6 +13654,9 @@ fold_abs_const (tree arg0, tree type)
> t = build_real (type, real_value_negate (&TREE_REAL_CST (arg0)));
> else
> t = arg0;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + (TREE_REAL_CST_PTR (t))->signalling = 0;
> break;
>
> default:
> Index: gcc/real.c
> ===================================================================
> --- gcc/real.c (revision 227584)
> +++ gcc/real.c (working copy)
> @@ -545,6 +545,9 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
> case CLASS2 (rvc_normal, rvc_inf):
> /* R + Inf = Inf. */
> *r = *b;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> r->sign = sign ^ subtract_p;
> return false;
>
> @@ -558,6 +561,9 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
> case CLASS2 (rvc_inf, rvc_normal):
> /* Inf + R = Inf. */
> *r = *a;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> return false;
>
> case CLASS2 (rvc_inf, rvc_inf):
> @@ -680,6 +686,9 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
> case CLASS2 (rvc_nan, rvc_nan):
> /* ANY * NaN = NaN. */
> *r = *b;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> r->sign = sign;
> return false;
>
> @@ -688,6 +697,9 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
> case CLASS2 (rvc_nan, rvc_inf):
> /* NaN * ANY = NaN. */
> *r = *a;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> r->sign = sign;
> return false;
>
> @@ -830,6 +842,9 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
> case CLASS2 (rvc_nan, rvc_nan):
> /* ANY / NaN = NaN. */
> *r = *b;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> r->sign = sign;
> return false;
>
> @@ -838,6 +853,9 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
> case CLASS2 (rvc_nan, rvc_inf):
> /* NaN / ANY = NaN. */
> *r = *a;
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> r->sign = sign;
> return false;
>
> @@ -968,6 +986,9 @@ do_fix_trunc (REAL_VALUE_TYPE *r, const REAL_VALUE
> case rvc_zero:
> case rvc_inf:
> case rvc_nan:
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> break;
>
> case rvc_normal:
> @@ -1059,6 +1080,11 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co
> default:
> gcc_unreachable ();
> }
> +
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> +
> return false;
> }
>
> @@ -1150,6 +1176,9 @@ real_ldexp (REAL_VALUE_TYPE *r, const REAL_VALUE_T
> case rvc_zero:
> case rvc_inf:
> case rvc_nan:
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (!flag_signaling_nans)
> + r->signalling = 0;
> break;
>
> case rvc_normal:
> @@ -2718,6 +2747,10 @@ real_convert (REAL_VALUE_TYPE *r, machine_mode mod
>
> round_for_format (fmt, r);
>
> + /* Convert sNaN to qNaN when flag_signaling_nans is off */
> + if (HONOR_NANS (mode) && !flag_signaling_nans)
> + r->signalling = 0;
> +
> /* round_for_format de-normalizes denormals. Undo just that part. */
> if (r->cl == rvc_normal)
> normalize (r);
> Index: gcc/testsuite/gcc.dg/pr61441.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
> @@ -0,0 +1,34 @@
> +/* { dg-do run } */
> +/* { dg-options "-O1 -lm" } */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <math.h>
> +
> +void conversion()
> +{
> + float sNaN = __builtin_nansf ("");
> + double x = (double) sNaN;
> + if (issignaling(x))
> + {
> + __builtin_abort();
> + }
> +}
> +
> +void addition()
> +{
> + float x;
> + float sNaN = __builtin_nansf ("");
> + x = sNaN + .0;
> + if (issignaling(x))
> + {
> + __builtin_abort();
> + }
> +}
> +
> +int main (void)
> +{
> + conversion();
> + addition();
> + return 0;
> +}
>
> On Wed, Sep 2, 2015 at 5:26 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Wed, Sep 2, 2015 at 1:36 PM, Sujoy Saraswati <ssaraswati@gmail.com> wrote:
>>> Hi Richard,
>>>
>>>> Note that I'm curious what
>>>> the actual bug is - is it that (double) sNaN creates a sNaN? Then the fix
>>>> should be elsewhere, in constant folding itself
>>>> (fold_convert_const_real_from_real
>>>> or real_convert).
>>>>
>>>> If that isn't the bug you have very many other passes to fix for the
>>>> same problem.
>>>>
>>>> So - can you please explain?
>>>
>>> In this test case, the floating point operation for converting the
>>> float to double is what should convert the sNaN to qNaN. I tried to
>>> cover more floating point operations than just the conversion
>>> operations exposed by this test case. For example, if we consider the
>>> following program -
>>>
>>> #define _GNU_SOURCE
>>> #include <stdio.h>
>>> #include <math.h>
>>>
>>> int main (void)
>>> {
>>> float x;
>>> float sNaN = __builtin_nansf ("");
>>> x = sNaN + .0;
>>> return issignaling(x);
>>> }
>>>
>>> The operation (sNaN + .0) should also result in qNaN after folding.
>>> Hence, I thought of doing the sNaN to qNaN conversion in various
>>> places under tree-ssa-ccp, where the result upon a folding is
>>> available. I do agree that this approach may mean many more such
>>> places should also be covered in other passes, but thought of sending
>>> the fix for the ccp pass to start with.
>>>
>>> Let me know if you suggest alternate approach.
>>
>> CCP and other passes ultimatively end up using fold-const.c:const_{unop,binop}
>> for constant folding so that is where the fix should go to (or to real.c). That
>> will automatically handle other passes doing similar transforms.
>>
>> Richard.
>>
>>> Regards,
>>> Sujoy
>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>>> Regards,
>>>>> Sujoy
>>>>>
>>>>> 2015-09-01 Sujoy Saraswati <ssaraswati@gmail.com>
>>>>>
>>>>> PR tree-optimization/61441
>>>>> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when
>>>>> flag_signaling_nans is off.
>>>>> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call
>>>>> convert_snan_to_qnan to convert sNaN to qNaN on constant folding.
>>>>>
>>>>> PR tree-optimization/61441
>>>>> * gcc.dg/pr61441.c: New testcase.
>>>>>
>>>>> Index: gcc/tree-ssa-ccp.c
>>>>> ===================================================================
>>>>> --- gcc/tree-ssa-ccp.c (revision 226965)
>>>>> +++ gcc/tree-ssa-ccp.c (working copy)
>>>>> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +/* Convert sNaN to qNaN when flag_signaling_nans is off */
>>>>> +
>>>>> +static void
>>>>> +convert_snan_to_qnan (tree expr)
>>>>> +{
>>>>> + if (expr
>>>>> + && (TREE_CODE (expr) == REAL_CST)
>>>>> + && !flag_signaling_nans)
>>>>> + {
>>>>> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr);
>>>>> +
>>>>> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr)))
>>>>> + && REAL_VALUE_ISNAN (*d)
>>>>> + && d->signalling)
>>>>> + d->signalling = 0;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> /* Return the value for the address expression EXPR based on alignment
>>>>> information. */
>>>>>
>>>>> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>>> if (val.lattice_val != CONSTANT
>>>>> || val.mask != 0)
>>>>> return false;
>>>>> + convert_snan_to_qnan (val.value);
>>>>>
>>>>> if (dump_file)
>>>>> {
>>>>> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>>> bool res;
>>>>> if (!useless_type_conversion_p (TREE_TYPE (lhs),
>>>>> TREE_TYPE (new_rhs)))
>>>>> + {
>>>>> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs);
>>>>> + convert_snan_to_qnan (new_rhs);
>>>>> + }
>>>>> res = update_call_from_tree (gsi, new_rhs);
>>>>> gcc_assert (res);
>>>>> return true;
>>>>> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>>> tree new_rhs = fold_builtin_alloca_with_align (stmt);
>>>>> if (new_rhs)
>>>>> {
>>>>> + convert_snan_to_qnan (new_rhs);
>>>>> bool res = update_call_from_tree (gsi, new_rhs);
>>>>> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0);
>>>>> gcc_assert (res);
>>>>> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi)
>>>>> {
>>>>> tree rhs = unshare_expr (val);
>>>>> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
>>>>> + {
>>>>> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
>>>>> + convert_snan_to_qnan (rhs);
>>>>> + }
>>>>> gimple_assign_set_rhs_from_tree (gsi, rhs);
>>>>> return true;
>>>>> }
>>>>> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p)
>>>>> /* Evaluate the statement, which could be
>>>>> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */
>>>>> val = evaluate_stmt (stmt);
>>>>> + convert_snan_to_qnan (val.value);
>>>>>
>>>>> /* If STMT is an assignment to an SSA_NAME, we only have one
>>>>> value to set. */
>>>>> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p)
>>>>> if (val.lattice_val != CONSTANT
>>>>> || val.mask != 0)
>>>>> return SSA_PROP_VARYING;
>>>>> + convert_snan_to_qnan (val.value);
>>>>>
>>>>> /* Find which edge out of the conditional block will be taken and add it
>>>>> to the worklist. If no single edge can be determined statically,
>>>>>
>>>>> Index: gcc/testsuite/gcc.dg/pr61441.c
>>>>> ===================================================================
>>>>> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
>>>>> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
>>>>> @@ -0,0 +1,17 @@
>>>>> +/* { dg-do run } */
>>>>> +/* { dg-options "-O1 -lm" } */
>>>>> +
>>>>> +#define _GNU_SOURCE
>>>>> +#include <stdio.h>
>>>>> +#include <math.h>
>>>>> +
>>>>> +int main (void)
>>>>> +{
>>>>> + float sNaN = __builtin_nansf ("");
>>>>> + double x = (double) sNaN;
>>>>> + if (issignaling(x))
>>>>> + {
>>>>> + __builtin_abort();
>>>>> + }
>>>>> + return 0;
>>>>> +}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-14 13:50 ` Richard Biener
@ 2015-09-14 20:39 ` Joseph Myers
2015-09-16 13:01 ` Sujoy Saraswati
0 siblings, 1 reply; 17+ messages in thread
From: Joseph Myers @ 2015-09-14 20:39 UTC (permalink / raw)
To: Richard Biener; +Cc: Sujoy Saraswati, GCC Patches
On Mon, 14 Sep 2015, Richard Biener wrote:
> I'll leave the correctness part of the patch to Joseph who knows FP
> arithmetic better than me,
> implementation-wise this is ok if you fix the REAL_CST sharing issue.
Changing fold_abs_const is wrong - abs of sNaN is sNaN, no exceptions
raised. Changing real_arithmetic is wrong for the NEGATE_EXPR and
ABS_EXPR cases, both of which should just affect the sign bit without
quieting sNaNs.
All the comments in the patch should end with ". " (full stop, two
spaces).
If -fsignaling-nans, then folding of expressions involving sNaNs should be
disabled, outside of static initializers - such expressions should not get
folded to return an sNaN (it's incorrect to fold sNaN + 1 to sNaN, for
example). I think existing code may ensure that (the HONOR_SNANS check in
const_binop, for example).
Inside static initializers, expressions involving sNaNs still need to be
folded (so sNaN + 1 becomes qNaN inside such an initializer, for example,
with the translation-time exception being discarded). Again, existing
code should handle this: START_FOLD_INIT / END_FOLD_INIT already handle
clearing and restoring flag_signaling_nans.
My understanding of the design of the existing code is that real.c will do
the arithmetic regardless of whether it might raise an exception or have
rounding-mode-dependent results, with fold-const.c being responsible for
deciding whether the result can be used to fold the expression in
question. That is, you shouldn't need any flag_signaling_nans conditions
in real.c; rather, if IEEE semantics mean an sNaN is quieted, real.c
should do so unconditionally. It should be the callers in fold-const.c
that check HONOR_SNANS and disallow folding when it would lose exceptions.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-14 20:39 ` Joseph Myers
@ 2015-09-16 13:01 ` Sujoy Saraswati
2015-09-16 17:03 ` Joseph Myers
0 siblings, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-09-16 13:01 UTC (permalink / raw)
To: Joseph Myers; +Cc: Richard Biener, GCC Patches
Hi,
>> I'll leave the correctness part of the patch to Joseph who knows FP
>> arithmetic better than me,
>> implementation-wise this is ok if you fix the REAL_CST sharing issue.
Ok, will change this.
> Changing fold_abs_const is wrong - abs of sNaN is sNaN, no exceptions
> raised. Changing real_arithmetic is wrong for the NEGATE_EXPR and
> ABS_EXPR cases, both of which should just affect the sign bit without
> quieting sNaNs.
Right, I had thought unary operators will be similar to binary
operators for this. Will fix this.
> All the comments in the patch should end with ". " (full stop, two
> spaces).
Ok.
> If -fsignaling-nans, then folding of expressions involving sNaNs should be
> disabled, outside of static initializers - such expressions should not get
> folded to return an sNaN (it's incorrect to fold sNaN + 1 to sNaN, for
> example). I think existing code may ensure that (the HONOR_SNANS check in
> const_binop, for example).
Yes, with -fsignaling-nans, the const_binop will not fold since the
HONOR_SNANS check is there. However, elsewhere, like const_unop, the
code doesn't do this check.
> Inside static initializers, expressions involving sNaNs still need to be
> folded (so sNaN + 1 becomes qNaN inside such an initializer, for example,
> with the translation-time exception being discarded). Again, existing
> code should handle this: START_FOLD_INIT / END_FOLD_INIT already handle
> clearing and restoring flag_signaling_nans.
Yes.
> My understanding of the design of the existing code is that real.c will do
> the arithmetic regardless of whether it might raise an exception or have
> rounding-mode-dependent results, with fold-const.c being responsible for
> deciding whether the result can be used to fold the expression in
> question. That is, you shouldn't need any flag_signaling_nans conditions
> in real.c; rather, if IEEE semantics mean an sNaN is quieted, real.c
> should do so unconditionally. It should be the callers in fold-const.c
> that check HONOR_SNANS and disallow folding when it would lose exceptions.
I had added the flag_signaling_nans condition within real.c so that
the callers can do the folding, with the sNaN value converted to qNaN.
However, as you mention, the real.c design should do this irrespective
of the signaling flag and leave it for the callers to disallow the
folding.
I will make the modifications and post a patch.
Regards,
Sujoy
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-16 13:01 ` Sujoy Saraswati
@ 2015-09-16 17:03 ` Joseph Myers
2015-10-13 10:46 ` Sujoy Saraswati
0 siblings, 1 reply; 17+ messages in thread
From: Joseph Myers @ 2015-09-16 17:03 UTC (permalink / raw)
To: Sujoy Saraswati; +Cc: Richard Biener, GCC Patches
On Wed, 16 Sep 2015, Sujoy Saraswati wrote:
> > If -fsignaling-nans, then folding of expressions involving sNaNs should be
> > disabled, outside of static initializers - such expressions should not get
> > folded to return an sNaN (it's incorrect to fold sNaN + 1 to sNaN, for
> > example). I think existing code may ensure that (the HONOR_SNANS check in
> > const_binop, for example).
>
> Yes, with -fsignaling-nans, the const_binop will not fold since the
> HONOR_SNANS check is there. However, elsewhere, like const_unop, the
> code doesn't do this check.
Which would be a bug in the const_unop code, or the functions it calls
(for operations for which such a check is appropriate - as noted, abs and
negation should be folded unconditionally).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-09-16 17:03 ` Joseph Myers
@ 2015-10-13 10:46 ` Sujoy Saraswati
2015-10-28 17:19 ` Joseph Myers
0 siblings, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-10-13 10:46 UTC (permalink / raw)
To: Joseph Myers; +Cc: Richard Biener, GCC Patches
Hi,
This is another modified version of the patch, incorporating the
previous comments.
Bootstrap and regression tests on x86_64-linux-gnu and
aarch64-unknown-linux-gnu passed with changes done on trunk.
Is this fine ?
Regards,
Sujoy
2015-10-13 Sujoy Saraswati <ssaraswati@gmail.com>
PR tree-optimization/61441
* builtins.c (integer_valued_real_p): Return true for
NaN values.
(fold_builtin_trunc, fold_builtin_pow): Avoid the operation
if flag_signaling_nans is on and the operand is a NaN.
(fold_builtin_powi): Same.
* fold-const.c (const_binop): Convert sNaN to qNaN when
flag_signaling_nans is off.
(const_unop): Avoid the operation, other than NEGATE and
ABS, if flag_signaling_nans is on and the operand is a NaN.
(fold_convert_const_real_from_real): Avoid the operation if
flag_signaling_nans is on and the operand is a NaN.
* real.c (do_add): Make resulting NaN value to be qNaN.
(do_multiply, do_divide, do_fix_trunc): Same.
(real_arithmetic, real_ldexp): Same
* simplify-rtx.c (simplify_const_unary_operation): Avoid the
operation if flag_signaling_nans is on and the operand is a NaN.
* tree-ssa-math-opts.c (gimple_expand_builtin_pow): Same.
PR tree-optimization/61441
* gcc.dg/pr61441.c: New testcase.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c (revision 228700)
+++ gcc/builtins.c (working copy)
@@ -7357,7 +7357,11 @@ integer_valued_real_p (tree t)
&& integer_valued_real_p (TREE_OPERAND (t, 2));
case REAL_CST:
- return real_isinteger (TREE_REAL_CST_PTR (t), TYPE_MODE (TREE_TYPE (t)));
+ /* Return true for NaN values, since real_isinteger would
+ return false if the value is sNaN. */
+ return (REAL_VALUE_ISNAN (TREE_REAL_CST (t))
+ || real_isinteger (TREE_REAL_CST_PTR (t),
+ TYPE_MODE (TREE_TYPE (t))));
CASE_CONVERT:
{
@@ -7910,8 +7914,13 @@ fold_builtin_trunc (location_t loc, tree fndecl, t
tree type = TREE_TYPE (TREE_TYPE (fndecl));
x = TREE_REAL_CST (arg);
- real_trunc (&r, TYPE_MODE (type), &x);
- return build_real (type, r);
+ /* Avoid the folding if flag_signaling_nans is on. */
+ if (!(HONOR_SNANS (TYPE_MODE (type))
+ && REAL_VALUE_ISNAN (x)))
+ {
+ real_trunc (&r, TYPE_MODE (type), &x);
+ return build_real (type, r);
+ }
}
return fold_trunc_transparent_mathfn (loc, fndecl, arg);
@@ -8297,9 +8306,15 @@ fold_builtin_pow (location_t loc, tree fndecl, tre
bool inexact;
x = TREE_REAL_CST (arg0);
+
inexact = real_powi (&x, TYPE_MODE (type), &x, n);
- if (flag_unsafe_math_optimizations || !inexact)
- return build_real (type, x);
+
+ /* Avoid the folding if flag_signaling_nans is on. */
+ if (flag_unsafe_math_optimizations
+ || (!inexact
+ && !(HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0)))
+ && REAL_VALUE_ISNAN (x))))
+ return build_real (type, x);
}
/* Strip sign ops from even integer powers. */
@@ -8388,8 +8403,14 @@ fold_builtin_powi (location_t loc, tree fndecl ATT
{
REAL_VALUE_TYPE x;
x = TREE_REAL_CST (arg0);
- real_powi (&x, TYPE_MODE (type), &x, c);
- return build_real (type, x);
+
+ /* Avoid the folding if flag_signaling_nans is on. */
+ if (!(HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0)))
+ && REAL_VALUE_ISNAN (x)))
+ {
+ real_powi (&x, TYPE_MODE (type), &x, c);
+ return build_real (type, x);
+ }
}
/* Optimize pow(x,0) = 1.0. */
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 228700)
+++ gcc/fold-const.c (working copy)
@@ -1185,9 +1185,21 @@ const_binop (enum tree_code code, tree arg1, tree
/* If either operand is a NaN, just return it. Otherwise, set up
for floating-point trap; we return an overflow. */
if (REAL_VALUE_ISNAN (d1))
- return arg1;
+ {
+ /* Make resulting NaN value to be qNaN when flag_signaling_nans
+ is off. */
+ d1.signalling = 0;
+ t = build_real (type, d1);
+ return t;
+ }
else if (REAL_VALUE_ISNAN (d2))
- return arg2;
+ {
+ /* Make resulting NaN value to be qNaN when flag_signaling_nans
+ is off. */
+ d2.signalling = 0;
+ t = build_real (type, d2);
+ return t;
+ }
inexact = real_arithmetic (&value, code, &d1, &d2);
real_convert (&result, mode, &value);
@@ -1557,6 +1569,15 @@ const_binop (enum tree_code code, tree type, tree
tree
const_unop (enum tree_code code, tree type, tree arg0)
{
+ /* Don't perform the operation, other than NEGATE and ABS, if
+ flag_signaling_nans is on and the operand is a NaN. */
+ if (TREE_CODE (arg0) == REAL_CST
+ && HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0)))
+ && REAL_VALUE_ISNAN (TREE_REAL_CST (arg0))
+ && code != NEGATE_EXPR
+ && code != ABS_EXPR)
+ return NULL_TREE;
+
switch (code)
{
CASE_CONVERT:
@@ -1964,7 +1985,17 @@ fold_convert_const_real_from_real (tree type, cons
REAL_VALUE_TYPE value;
tree t;
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
+ && REAL_VALUE_ISNAN (TREE_REAL_CST (arg1)))
+ return NULL_TREE;
+
real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
+ /* Make resulting NaN value to be qNaN when flag_signaling_nans
+ is off. */
+ if (REAL_VALUE_ISNAN (value))
+ value.signalling = 0;
t = build_real (type, value);
/* If converting an infinity or NAN to a representation that doesn't
Index: gcc/real.c
===================================================================
--- gcc/real.c (revision 228700)
+++ gcc/real.c (working copy)
@@ -545,6 +545,10 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
case CLASS2 (rvc_normal, rvc_inf):
/* R + Inf = Inf. */
*r = *b;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign ^ subtract_p;
return false;
@@ -558,6 +562,10 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
case CLASS2 (rvc_inf, rvc_normal):
/* Inf + R = Inf. */
*r = *a;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
return false;
case CLASS2 (rvc_inf, rvc_inf):
@@ -680,6 +688,10 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
case CLASS2 (rvc_nan, rvc_nan):
/* ANY * NaN = NaN. */
*r = *b;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -688,6 +700,10 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
case CLASS2 (rvc_nan, rvc_inf):
/* NaN * ANY = NaN. */
*r = *a;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -830,6 +846,10 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
case CLASS2 (rvc_nan, rvc_nan):
/* ANY / NaN = NaN. */
*r = *b;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -838,6 +858,10 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
case CLASS2 (rvc_nan, rvc_inf):
/* NaN / ANY = NaN. */
*r = *a;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -968,6 +992,10 @@ do_fix_trunc (REAL_VALUE_TYPE *r, const REAL_VALUE
case rvc_zero:
case rvc_inf:
case rvc_nan:
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
break;
case rvc_normal:
@@ -1026,7 +1054,13 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co
case MIN_EXPR:
if (op1->cl == rvc_nan)
+ {
*r = *op1;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
+ }
else if (do_compare (op0, op1, -1) < 0)
*r = *op0;
else
@@ -1035,7 +1069,13 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co
case MAX_EXPR:
if (op1->cl == rvc_nan)
+ {
*r = *op1;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
+ }
else if (do_compare (op0, op1, 1) < 0)
*r = *op1;
else
@@ -1166,6 +1206,10 @@ real_ldexp (REAL_VALUE_TYPE *r, const REAL_VALUE_T
case rvc_zero:
case rvc_inf:
case rvc_nan:
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
break;
case rvc_normal:
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c (revision 228700)
+++ gcc/simplify-rtx.c (working copy)
@@ -1801,16 +1801,25 @@ simplify_const_unary_operation (enum rtx_code code
d = real_value_negate (&d);
break;
case FLOAT_TRUNCATE:
- d = real_value_truncate (mode, d);
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (!(HONOR_SNANS (mode) && REAL_VALUE_ISNAN (d)))
+ d = real_value_truncate (mode, d);
break;
case FLOAT_EXTEND:
/* All this does is change the mode, unless changing
mode class. */
- if (GET_MODE_CLASS (mode) != GET_MODE_CLASS (GET_MODE (op)))
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (GET_MODE_CLASS (mode) != GET_MODE_CLASS (GET_MODE (op))
+ && !(HONOR_SNANS (mode) && REAL_VALUE_ISNAN (d)))
real_convert (&d, mode, &d);
break;
case FIX:
- real_arithmetic (&d, FIX_TRUNC_EXPR, &d, NULL);
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (!(HONOR_SNANS (mode) && REAL_VALUE_ISNAN (d)))
+ real_arithmetic (&d, FIX_TRUNC_EXPR, &d, NULL);
break;
case NOT:
{
Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c (revision 228700)
+++ gcc/tree-ssa-math-opts.c (working copy)
@@ -1486,6 +1486,13 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
if (TREE_CODE (arg1) != REAL_CST)
return NULL_TREE;
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
+ && (REAL_VALUE_ISNAN (TREE_REAL_CST (arg0))
+ || REAL_VALUE_ISNAN (TREE_REAL_CST (arg1))))
+ return NULL_TREE;
+
/* If the exponent is equivalent to an integer, expand to an optimal
multiplication sequence when profitable. */
c = TREE_REAL_CST (arg1);
Index: gcc/testsuite/gcc.dg/pr61441.c
===================================================================
--- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
@@ -0,0 +1,61 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -lm" } */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <math.h>
+
+void conversion()
+{
+ float sNaN = __builtin_nansf ("");
+ double x = (double) sNaN;
+ if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+}
+
+enum op {Add, Mult, Div, Abs};
+
+void operation(enum op t)
+{
+ float x, y;
+ float sNaN = __builtin_nansf ("");
+ switch (t)
+ {
+ case Abs:
+ x = fabs(sNaN);
+ break;
+ case Add:
+ x = sNaN + 2.0;
+ break;
+ case Mult:
+ x = sNaN * 2.0;
+ break;
+ case Div:
+ default:
+ x = sNaN / 2.0;
+ break;
+ }
+ if (t == Abs)
+ {
+ if (!issignaling(x))
+ {
+ __builtin_abort();
+ }
+ }
+ else if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+}
+
+int main (void)
+{
+ conversion();
+ operation(Add);
+ operation(Mult);
+ operation(Div);
+ operation(Abs);
+ return 0;
+}
On Wed, Sep 16, 2015 at 10:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 16 Sep 2015, Sujoy Saraswati wrote:
>
>> > If -fsignaling-nans, then folding of expressions involving sNaNs should be
>> > disabled, outside of static initializers - such expressions should not get
>> > folded to return an sNaN (it's incorrect to fold sNaN + 1 to sNaN, for
>> > example). I think existing code may ensure that (the HONOR_SNANS check in
>> > const_binop, for example).
>>
>> Yes, with -fsignaling-nans, the const_binop will not fold since the
>> HONOR_SNANS check is there. However, elsewhere, like const_unop, the
>> code doesn't do this check.
>
> Which would be a bug in the const_unop code, or the functions it calls
> (for operations for which such a check is appropriate - as noted, abs and
> negation should be folded unconditionally).
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-10-13 10:46 ` Sujoy Saraswati
@ 2015-10-28 17:19 ` Joseph Myers
2015-11-05 11:29 ` Sujoy Saraswati
0 siblings, 1 reply; 17+ messages in thread
From: Joseph Myers @ 2015-10-28 17:19 UTC (permalink / raw)
To: Sujoy Saraswati; +Cc: Richard Biener, GCC Patches
On Tue, 13 Oct 2015, Sujoy Saraswati wrote:
> Index: gcc/builtins.c
> ===================================================================
> --- gcc/builtins.c (revision 228700)
> +++ gcc/builtins.c (working copy)
> @@ -7357,7 +7357,11 @@ integer_valued_real_p (tree t)
> && integer_valued_real_p (TREE_OPERAND (t, 2));
>
> case REAL_CST:
> - return real_isinteger (TREE_REAL_CST_PTR (t), TYPE_MODE (TREE_TYPE (t)));
> + /* Return true for NaN values, since real_isinteger would
> + return false if the value is sNaN. */
> + return (REAL_VALUE_ISNAN (TREE_REAL_CST (t))
> + || real_isinteger (TREE_REAL_CST_PTR (t),
> + TYPE_MODE (TREE_TYPE (t))));
It seems this code has moved to fold-const.c, so the patch will need
updating.
Now, is it actually correct for integer_valued_real_single_p (now) to
treat sNaN as an integer? Looking at existing uses, they all appear to be
in match.pd: it's used for removing redundant trunc / floor / ceil / round
/ nearbyint on integer values. In such cases, it's *incorrect* to count
sNaN as an integer, because while those functions map qNaN quietly to
qNaN, they should map sNaN to qNaN and raise "invalid" in the process. So
I don't think you should have this change at all.
(I don't see why the handling of rint checks flag_errno_math. rint should
quietly return infinite and qNaN arguments just like nearbyint; it never
needs to set errno and distinguishing rint / nearbyint here is spurious;
the only difference is regarding the "inexact" exception.)
> @@ -7910,8 +7914,13 @@ fold_builtin_trunc (location_t loc, tree fndecl, t
> tree type = TREE_TYPE (TREE_TYPE (fndecl));
>
> x = TREE_REAL_CST (arg);
> - real_trunc (&r, TYPE_MODE (type), &x);
> - return build_real (type, r);
> + /* Avoid the folding if flag_signaling_nans is on. */
> + if (!(HONOR_SNANS (TYPE_MODE (type))
> + && REAL_VALUE_ISNAN (x)))
I realise this corresponds to some existing code (in const_binop, at
least), but I don't think that existing code is a good model.
If -fsignaling-nans, it's still perfectly fine to fold when the argument
is a quiet NaN. So what you actually want in such cases where you have a
known constant argument - as opposed to an unknown argument that might or
might not be constant - is to check whether that argument is itself a
signaling NaN. So you want to add a new REAL_VALUE_ISSIGNALING, or
something like that. And then find existing code that has the wrong (NaN
and sNaNs supported) check and convert it to checking (this value is
sNaN).
(This code has also moved to another function in fold-const.c, it seems.)
Cf. the match.pd handling of fmin / fmax checking for signaling NaN
arguments explicitly.
Some other places in this patch have similar issues with checking
HONOR_SNANS && REAL_VALUE_ISNAN when they should check if the particular
value is sNaN.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-10-28 17:19 ` Joseph Myers
@ 2015-11-05 11:29 ` Sujoy Saraswati
2015-11-05 16:58 ` Joseph Myers
0 siblings, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-11-05 11:29 UTC (permalink / raw)
To: Joseph Myers; +Cc: Richard Biener, GCC Patches
Hi,
> It seems this code has moved to fold-const.c, so the patch will need
> updating.
I have updated the patch and sending again.
> Now, is it actually correct for integer_valued_real_single_p (now) to
> treat sNaN as an integer? Looking at existing uses, they all appear to be
> in match.pd: it's used for removing redundant trunc / floor / ceil / round
> / nearbyint on integer values. In such cases, it's *incorrect* to count
> sNaN as an integer, because while those functions map qNaN quietly to
> qNaN, they should map sNaN to qNaN and raise "invalid" in the process. So
> I don't think you should have this change at all.
Ok. I have also updated the comments for the functions stating they
return false for sNaN.
> (I don't see why the handling of rint checks flag_errno_math. rint should
> quietly return infinite and qNaN arguments just like nearbyint; it never
> needs to set errno and distinguishing rint / nearbyint here is spurious;
> the only difference is regarding the "inexact" exception.)
With these changes, real_isinteger returns false for sNaN. This in
turn means that integer_valued_real_p would return false with sNaN.
This means that for sNaN, rint will not go to the path of checking
flag_errno_math. Anyway, I modified that also as part of this patch to
make rint similar to nearbyint.
> I realise this corresponds to some existing code (in const_binop, at
> least), but I don't think that existing code is a good model.
>
> If -fsignaling-nans, it's still perfectly fine to fold when the argument
> is a quiet NaN. So what you actually want in such cases where you have a
> known constant argument - as opposed to an unknown argument that might or
> might not be constant - is to check whether that argument is itself a
> signaling NaN. So you want to add a new REAL_VALUE_ISSIGNALING, or
> something like that. And then find existing code that has the wrong (NaN
> and sNaNs supported) check and convert it to checking (this value is
> sNaN).
>
> (This code has also moved to another function in fold-const.c, it seems.)
>
> Cf. the match.pd handling of fmin / fmax checking for signaling NaN
> arguments explicitly.
>
> Some other places in this patch have similar issues with checking
> HONOR_SNANS && REAL_VALUE_ISNAN when they should check if the particular
> value is sNaN.
I modified the code to check for sNaN specifically. The modified patch is below.
Bootstrap and regression tests on x86_64-linux-gnu passed with changes
done on trunk.
Is this fine ?
Regards,
Sujoy
2015-11-04 Sujoy Saraswati <ssaraswati@gmail.com>
PR tree-optimization/61441
* fold-const-call.c (fold_const_pow): Avoid the operation
if flag_signaling_nans is on and the operand is an sNaN.
(fold_const_call_sss): Same for BUILT_IN_POWI.
* fold-const.c (const_binop): Convert sNaN to qNaN when
flag_signaling_nans is off.
(const_unop): Avoid the operation, other than NEGATE and
ABS, if flag_signaling_nans is on and the operand is an sNaN.
(fold_convert_const_real_from_real): Avoid the operation if
flag_signaling_nans is on and the operand is an sNaN.
(integer_valued_real_unary_p): Update comment stating it
returns false for sNaN values.
(integer_valued_real_binary_p, integer_valued_real_call_p): Same.
(integer_valued_real_single_p): Same.
(integer_valued_real_invalid_p, integer_valued_real_p): Same.
* gimple-fold.c (gimple_assign_integer_valued_real_p): Same.
(gimple_call_integer_valued_real_p): Same.
(gimple_phi_integer_valued_real_p): Same.
(gimple_stmt_integer_valued_real_p): Same.
* match.pd (f(x) -> x): Removed flag_errno_math check for RINT.
* real.c (do_add): Make resulting NaN value to be qNaN.
(do_multiply, do_divide, do_fix_trunc): Same.
(real_arithmetic, real_ldexp): Same.
(real_issignaling_nan): New.
(real_isinteger): Updated comment stating it returns false for sNaN.
* real.h (real_issignaling_nan, REAL_VALUE_ISSIGNALING_NAN): New.
* simplify-rtx.c (simplify_const_unary_operation): Avoid the
operation if flag_signaling_nans is on and the operand is an sNaN.
(simplify_const_binary_operation): Check for sNaN instead of NaN.
* tree-ssa-math-opts.c (gimple_expand_builtin_pow): Avoid the
operation if flag_signaling_nans is on and the operand is an sNaN.
PR tree-optimization/61441
* gcc.dg/pr61441.c: New testcase.
Index: gcc/fold-const-call.c
===================================================================
--- gcc/fold-const-call.c (revision 229805)
+++ gcc/fold-const-call.c (working copy)
@@ -484,7 +484,11 @@ fold_const_pow (real_value *result, const real_val
|| !real_equal (arg0, &dconst0)))
{
bool inexact = real_powi (result, format, arg0, n1);
- if (flag_unsafe_math_optimizations || !inexact)
+ /* Avoid the folding if flag_signaling_nans is on. */
+ if (flag_unsafe_math_optimizations
+ || (!inexact
+ && !(flag_signaling_nans
+ && REAL_VALUE_ISSIGNALING_NAN (*result))))
return true;
}
@@ -1035,6 +1039,11 @@ fold_const_call_sss (real_value *result, built_in_
CASE_FLT_FN (BUILT_IN_POWI):
real_powi (result, format, arg0, arg1.to_shwi ());
+ /* Avoid the folding if flag_signaling_nans is on. */
+ if (!flag_unsafe_math_optimizations
+ && flag_signaling_nans
+ && REAL_VALUE_ISSIGNALING_NAN (*result))
+ return false;
return true;
default:
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 229805)
+++ gcc/fold-const.c (working copy)
@@ -1151,7 +1151,8 @@ const_binop (enum tree_code code, tree arg1, tree
/* Don't perform operation if we honor signaling NaNs and
either operand is a NaN. */
if (HONOR_SNANS (mode)
- && (REAL_VALUE_ISNAN (d1) || REAL_VALUE_ISNAN (d2)))
+ && (REAL_VALUE_ISSIGNALING_NAN (d1)
+ || REAL_VALUE_ISSIGNALING_NAN (d2)))
return NULL_TREE;
/* Don't perform operation if it would raise a division
@@ -1164,9 +1165,21 @@ const_binop (enum tree_code code, tree arg1, tree
/* If either operand is a NaN, just return it. Otherwise, set up
for floating-point trap; we return an overflow. */
if (REAL_VALUE_ISNAN (d1))
- return arg1;
+ {
+ /* Make resulting NaN value to be qNaN when flag_signaling_nans
+ is off. */
+ d1.signalling = 0;
+ t = build_real (type, d1);
+ return t;
+ }
else if (REAL_VALUE_ISNAN (d2))
- return arg2;
+ {
+ /* Make resulting NaN value to be qNaN when flag_signaling_nans
+ is off. */
+ d2.signalling = 0;
+ t = build_real (type, d2);
+ return t;
+ }
inexact = real_arithmetic (&value, code, &d1, &d2);
real_convert (&result, mode, &value);
@@ -1536,6 +1549,15 @@ const_binop (enum tree_code code, tree type, tree
tree
const_unop (enum tree_code code, tree type, tree arg0)
{
+ /* Don't perform the operation, other than NEGATE and ABS, if
+ flag_signaling_nans is on and the operand is a NaN. */
+ if (TREE_CODE (arg0) == REAL_CST
+ && HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0)))
+ && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg0))
+ && code != NEGATE_EXPR
+ && code != ABS_EXPR)
+ return NULL_TREE;
+
switch (code)
{
CASE_CONVERT:
@@ -1943,7 +1965,17 @@ fold_convert_const_real_from_real (tree type, cons
REAL_VALUE_TYPE value;
tree t;
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
+ && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
+ return NULL_TREE;
+
real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
+ /* Make resulting NaN value to be qNaN when flag_signaling_nans
+ is off. */
+ if (REAL_VALUE_ISSIGNALING_NAN (value))
+ value.signalling = 0;
t = build_real (type, value);
/* If converting an infinity or NAN to a representation that doesn't
@@ -13413,7 +13445,7 @@ tree_single_nonzero_warnv_p (tree t, bool *strict_
/* Return true if the floating point result of (CODE OP0) has an
integer value. We also allow +Inf, -Inf and NaN to be considered
- integer values.
+ integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -13446,7 +13478,7 @@ integer_valued_real_unary_p (tree_code code, tree
/* Return true if the floating point result of (CODE OP0 OP1) has an
integer value. We also allow +Inf, -Inf and NaN to be considered
- integer values.
+ integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -13470,8 +13502,8 @@ integer_valued_real_binary_p (tree_code code, tree
/* Return true if the floating point result of calling FNDECL with arguments
ARG0 and ARG1 has an integer value. We also allow +Inf, -Inf and NaN to be
- considered integer values. If FNDECL takes fewer than 2 arguments,
- the remaining ARGn are null.
+ considered integer values. Return false for signalling NaN.
+ If FNDECL takes fewer than 2 arguments, the remaining ARGn are null.
DEPTH is the current nesting depth of the query. */
@@ -13501,7 +13533,7 @@ integer_valued_real_call_p (tree fndecl, tree arg0
/* Return true if the floating point expression T (a GIMPLE_SINGLE_RHS)
has an integer value. We also allow +Inf, -Inf and NaN to be
- considered integer values.
+ considered integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -13535,7 +13567,7 @@ integer_valued_real_single_p (tree t, int depth)
/* Return true if the floating point expression T (a GIMPLE_INVALID_RHS)
has an integer value. We also allow +Inf, -Inf and NaN to be
- considered integer values.
+ considered integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -13563,6 +13595,7 @@ integer_valued_real_invalid_p (tree t, int depth)
/* Return true if the floating point expression T has an integer value.
We also allow +Inf, -Inf and NaN to be considered integer values.
+ Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c (revision 229805)
+++ gcc/gimple-fold.c (working copy)
@@ -6221,7 +6221,7 @@ gimple_stmt_nonnegative_warnv_p (gimple *stmt, boo
/* Return true if the floating-point value computed by assignment STMT
is known to have an integer value. We also allow +Inf, -Inf and NaN
- to be considered integer values.
+ to be considered integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -6250,7 +6250,7 @@ gimple_assign_integer_valued_real_p (gimple *stmt,
/* Return true if the floating-point value computed by call STMT is known
to have an integer value. We also allow +Inf, -Inf and NaN to be
- considered integer values.
+ considered integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -6269,7 +6269,7 @@ gimple_call_integer_valued_real_p (gimple *stmt, i
/* Return true if the floating-point result of phi STMT is known to have
an integer value. We also allow +Inf, -Inf and NaN to be considered
- integer values.
+ integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
@@ -6287,7 +6287,7 @@ gimple_phi_integer_valued_real_p (gimple *stmt, in
/* Return true if the floating-point value computed by STMT is known
to have an integer value. We also allow +Inf, -Inf and NaN to be
- considered integer values.
+ considered integer values. Return false for signalling NaN.
DEPTH is the current nesting depth of the query. */
Index: gcc/match.pd
===================================================================
--- gcc/match.pd (revision 229805)
+++ gcc/match.pd (working copy)
@@ -2572,16 +2572,10 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(fns (fns @0))
(fns @0)))
/* f(x) -> x if x is integer valued and f does nothing for such values. */
-(for fns (TRUNC FLOOR CEIL ROUND NEARBYINT)
+(for fns (TRUNC FLOOR CEIL ROUND NEARBYINT RINT)
(simplify
(fns integer_valued_real_p@0)
@0))
-/* Same for rint. We have to check flag_errno_math because
- integer_valued_real_p accepts +Inf, -Inf and NaNs as integers. */
-(if (!flag_errno_math)
- (simplify
- (RINT integer_valued_real_p@0)
- @0))
/* hypot(x,0) and hypot(0,x) -> abs(x). */
(simplify
Index: gcc/real.c
===================================================================
--- gcc/real.c (revision 229805)
+++ gcc/real.c (working copy)
@@ -541,6 +541,10 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
case CLASS2 (rvc_normal, rvc_inf):
/* R + Inf = Inf. */
*r = *b;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign ^ subtract_p;
return false;
@@ -554,6 +558,10 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE
case CLASS2 (rvc_inf, rvc_normal):
/* Inf + R = Inf. */
*r = *a;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
return false;
case CLASS2 (rvc_inf, rvc_inf):
@@ -676,6 +684,10 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
case CLASS2 (rvc_nan, rvc_nan):
/* ANY * NaN = NaN. */
*r = *b;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -684,6 +696,10 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_
case CLASS2 (rvc_nan, rvc_inf):
/* NaN * ANY = NaN. */
*r = *a;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -826,6 +842,10 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
case CLASS2 (rvc_nan, rvc_nan):
/* ANY / NaN = NaN. */
*r = *b;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -834,6 +854,10 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY
case CLASS2 (rvc_nan, rvc_inf):
/* NaN / ANY = NaN. */
*r = *a;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
r->sign = sign;
return false;
@@ -964,6 +988,10 @@ do_fix_trunc (REAL_VALUE_TYPE *r, const REAL_VALUE
case rvc_zero:
case rvc_inf:
case rvc_nan:
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
break;
case rvc_normal:
@@ -1022,7 +1050,13 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co
case MIN_EXPR:
if (op1->cl == rvc_nan)
+ {
*r = *op1;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
+ }
else if (do_compare (op0, op1, -1) < 0)
*r = *op0;
else
@@ -1031,7 +1065,13 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co
case MAX_EXPR:
if (op1->cl == rvc_nan)
+ {
*r = *op1;
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
+ }
else if (do_compare (op0, op1, 1) < 0)
*r = *op1;
else
@@ -1162,6 +1202,10 @@ real_ldexp (REAL_VALUE_TYPE *r, const REAL_VALUE_T
case rvc_zero:
case rvc_inf:
case rvc_nan:
+ /* Make resulting NaN value to be qNaN. The caller has the
+ responsibility to avoid the operation if flag_signaling_nans
+ is on. */
+ r->signalling = 0;
break;
case rvc_normal:
@@ -1195,6 +1239,14 @@ real_isnan (const REAL_VALUE_TYPE *r)
return (r->cl == rvc_nan);
}
+/* Determine whether a floating-point value X is a signalling NaN. */
+
+bool
+real_issignaling_nan (const REAL_VALUE_TYPE *r)
+{
+ return real_isnan (r) && r->signalling;
+}
+
/* Determine whether a floating-point value X is finite. */
bool
@@ -4937,7 +4989,8 @@ real_copysign (REAL_VALUE_TYPE *r, const REAL_VALU
r->sign = x->sign;
}
-/* Check whether the real constant value given is an integer. */
+/* Check whether the real constant value given is an integer.
+ Returns false for signalling NaN. */
bool
real_isinteger (const REAL_VALUE_TYPE *c, format_helper fmt)
Index: gcc/real.h
===================================================================
--- gcc/real.h (revision 229805)
+++ gcc/real.h (working copy)
@@ -262,6 +262,9 @@ extern bool real_isinf (const REAL_VALUE_TYPE *);
/* Determine whether a floating-point value X is a NaN. */
extern bool real_isnan (const REAL_VALUE_TYPE *);
+/* Determine whether a floating-point value X is a signalling NaN. */
+extern bool real_issignaling_nan (const REAL_VALUE_TYPE *);
+
/* Determine whether a floating-point value X is finite. */
extern bool real_isfinite (const REAL_VALUE_TYPE *);
@@ -357,6 +360,9 @@ extern const struct real_format arm_half_format;
/* Determine whether a floating-point value X is a NaN. */
#define REAL_VALUE_ISNAN(x) real_isnan (&(x))
+/* Determine whether a floating-point value X is a signalling NaN. */
+#define REAL_VALUE_ISSIGNALING_NAN(x) real_issignaling_nan (&(x))
+
/* Determine whether a floating-point value X is negative. */
#define REAL_VALUE_NEGATIVE(x) real_isneg (&(x))
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c (revision 229805)
+++ gcc/simplify-rtx.c (working copy)
@@ -1789,16 +1789,25 @@ simplify_const_unary_operation (enum rtx_code code
d = real_value_negate (&d);
break;
case FLOAT_TRUNCATE:
- d = real_value_truncate (mode, d);
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (!(HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d)))
+ d = real_value_truncate (mode, d);
break;
case FLOAT_EXTEND:
/* All this does is change the mode, unless changing
mode class. */
- if (GET_MODE_CLASS (mode) != GET_MODE_CLASS (GET_MODE (op)))
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (GET_MODE_CLASS (mode) != GET_MODE_CLASS (GET_MODE (op))
+ && !(HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d)))
real_convert (&d, mode, &d);
break;
case FIX:
- real_arithmetic (&d, FIX_TRUNC_EXPR, &d, NULL);
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (!(HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d)))
+ real_arithmetic (&d, FIX_TRUNC_EXPR, &d, NULL);
break;
case NOT:
{
@@ -3856,7 +3865,8 @@ simplify_const_binary_operation (enum rtx_code cod
real_convert (&f1, mode, CONST_DOUBLE_REAL_VALUE (op1));
if (HONOR_SNANS (mode)
- && (REAL_VALUE_ISNAN (f0) || REAL_VALUE_ISNAN (f1)))
+ && (REAL_VALUE_ISSIGNALING_NAN (f0)
+ || REAL_VALUE_ISSIGNALING_NAN (f1)))
return 0;
if (code == DIV
Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c (revision 229805)
+++ gcc/tree-ssa-math-opts.c (working copy)
@@ -1482,6 +1482,13 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
if (TREE_CODE (arg1) != REAL_CST)
return NULL_TREE;
+ /* Don't perform the operation if flag_signaling_nans is on
+ and the operand is a NaN. */
+ if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
+ && (REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg0))
+ || REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1))))
+ return NULL_TREE;
+
/* If the exponent is equivalent to an integer, expand to an optimal
multiplication sequence when profitable. */
c = TREE_REAL_CST (arg1);
Index: gcc/testsuite/gcc.dg/pr61441.c
===================================================================
--- gcc/testsuite/gcc.dg/pr61441.c (revision 0)
+++ gcc/testsuite/gcc.dg/pr61441.c (working copy)
@@ -0,0 +1,61 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -lm" } */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <math.h>
+
+void conversion()
+{
+ float sNaN = __builtin_nansf ("");
+ double x = (double) sNaN;
+ if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+}
+
+enum op {Add, Mult, Div, Abs};
+
+void operation(enum op t)
+{
+ float x, y;
+ float sNaN = __builtin_nansf ("");
+ switch (t)
+ {
+ case Abs:
+ x = fabs(sNaN);
+ break;
+ case Add:
+ x = sNaN + 2.0;
+ break;
+ case Mult:
+ x = sNaN * 2.0;
+ break;
+ case Div:
+ default:
+ x = sNaN / 2.0;
+ break;
+ }
+ if (t == Abs)
+ {
+ if (!issignaling(x))
+ {
+ __builtin_abort();
+ }
+ }
+ else if (issignaling(x))
+ {
+ __builtin_abort();
+ }
+}
+
+int main (void)
+{
+ conversion();
+ operation(Add);
+ operation(Mult);
+ operation(Div);
+ operation(Abs);
+ return 0;
+}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-11-05 11:29 ` Sujoy Saraswati
@ 2015-11-05 16:58 ` Joseph Myers
2015-11-06 4:56 ` Sujoy Saraswati
0 siblings, 1 reply; 17+ messages in thread
From: Joseph Myers @ 2015-11-05 16:58 UTC (permalink / raw)
To: Sujoy Saraswati; +Cc: Richard Biener, GCC Patches
On Thu, 5 Nov 2015, Sujoy Saraswati wrote:
> > Some other places in this patch have similar issues with checking
> > HONOR_SNANS && REAL_VALUE_ISNAN when they should check if the particular
> > value is sNaN.
>
> I modified the code to check for sNaN specifically. The modified patch
> is below.
This is looking pretty close to ready to go in, but the patch also seems
to be doing several things at once and it might be easier to review if
split into separate logical pieces. For example (this is an illustration,
not necessarily saying it should be exactly this way): (1) Add
REAL_VALUE_ISSIGNALING_NAN and convert existing logic that already does
something equivalent (so no semantic change, just cleanup). (2) Convert
existing code that checks for all NaNs in cases where
REAL_VALUE_ISSIGNALING_NAN would be enough. (3) Treat rint and nearbyint
the same. (4) Make real.c operations properly produce quiet NaNs for
operations with signaling NaN input. (5) Disable various transformations
for signaling NaN operands, where folding them loses exceptions.
This patch is also getting big enough that it would be a good idea to
complete the copyright assignment paperwork, certainly if you plan to make
more contributions in future, if you're not covered by a corporate
assignment.
> @@ -1943,7 +1965,17 @@ fold_convert_const_real_from_real (tree type, cons
> REAL_VALUE_TYPE value;
> tree t;
>
> + /* Don't perform the operation if flag_signaling_nans is on
> + and the operand is a NaN. */
> + if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
> + && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
> + return NULL_TREE;
> +
> real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
> + /* Make resulting NaN value to be qNaN when flag_signaling_nans
> + is off. */
> + if (REAL_VALUE_ISSIGNALING_NAN (value))
> + value.signalling = 0;
Shouldn't real_convert do this rather than the caller needing to do it?
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-11-05 16:58 ` Joseph Myers
@ 2015-11-06 4:56 ` Sujoy Saraswati
2015-11-06 13:09 ` Joseph Myers
0 siblings, 1 reply; 17+ messages in thread
From: Sujoy Saraswati @ 2015-11-06 4:56 UTC (permalink / raw)
To: Joseph Myers; +Cc: Richard Biener, GCC Patches
Hi,
> This is looking pretty close to ready to go in, but the patch also seems
> to be doing several things at once and it might be easier to review if
> split into separate logical pieces. For example (this is an illustration,
> not necessarily saying it should be exactly this way): (1) Add
> REAL_VALUE_ISSIGNALING_NAN and convert existing logic that already does
> something equivalent (so no semantic change, just cleanup). (2) Convert
> existing code that checks for all NaNs in cases where
> REAL_VALUE_ISSIGNALING_NAN would be enough. (3) Treat rint and nearbyint
> the same. (4) Make real.c operations properly produce quiet NaNs for
> operations with signaling NaN input. (5) Disable various transformations
> for signaling NaN operands, where folding them loses exceptions.
Thanks, I will break it up into multiple patches as you proposed.
> This patch is also getting big enough that it would be a good idea to
> complete the copyright assignment paperwork, certainly if you plan to make
> more contributions in future, if you're not covered by a corporate
> assignment.
I have a corporate assignment, I will post the patches from my
corporate email id.
>> @@ -1943,7 +1965,17 @@ fold_convert_const_real_from_real (tree type, cons
>> REAL_VALUE_TYPE value;
>> tree t;
>>
>> + /* Don't perform the operation if flag_signaling_nans is on
>> + and the operand is a NaN. */
>> + if (HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg1)))
>> + && REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg1)))
>> + return NULL_TREE;
>> +
>> real_convert (&value, TYPE_MODE (type), &TREE_REAL_CST (arg1));
>> + /* Make resulting NaN value to be qNaN when flag_signaling_nans
>> + is off. */
>> + if (REAL_VALUE_ISSIGNALING_NAN (value))
>> + value.signalling = 0;
>
> Shouldn't real_convert do this rather than the caller needing to do it?
Yes, it should be. I had started by doing this within real_convert but
then saw that there are quite a few callers where I should add the
check for flag_signaling_nans. This was making the patch bigger, so
instead decided to change the caller in this particular case. I will
try to make the change in real_convert now that we are planning to
break the patch.
Regards,
Sujoy
> --
> Joseph S. Myers
> joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Fix 61441
2015-11-06 4:56 ` Sujoy Saraswati
@ 2015-11-06 13:09 ` Joseph Myers
2015-11-26 8:28 ` Saraswati, Sujoy (OSTL)
0 siblings, 1 reply; 17+ messages in thread
From: Joseph Myers @ 2015-11-06 13:09 UTC (permalink / raw)
To: Sujoy Saraswati; +Cc: Richard Biener, GCC Patches
On Fri, 6 Nov 2015, Sujoy Saraswati wrote:
> > Shouldn't real_convert do this rather than the caller needing to do it?
>
> Yes, it should be. I had started by doing this within real_convert but
> then saw that there are quite a few callers where I should add the
> check for flag_signaling_nans. This was making the patch bigger, so
> instead decided to change the caller in this particular case. I will
> try to make the change in real_convert now that we are planning to
> break the patch.
I think the general principle is:
* The caller decides whether folding is desirable (whether it would lose
exceptions, for example).
* The real.c code is called only when the caller has decided that folding
is desirable, and should always produce the correct output (which for a
conversion means producing a quiet NaN from a signaling NaN).
So both places need changes, but real_convert is where the code that makes
it a quiet NaN should go.
Another place in the patch that looks incorrect: the changes to
fold-const-call.c calling real_powi and checking if the result is a
signaling NaN. The result of real_powi should never be a signaling NaN.
Rather, real_powi should produce a quiet NaN if its input is a signaling
NaN, and the callers should check if the argument is a signaling NaN when
deciding whether to fold, not if the result is.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: Fix 61441
2015-11-06 13:09 ` Joseph Myers
@ 2015-11-26 8:28 ` Saraswati, Sujoy (OSTL)
0 siblings, 0 replies; 17+ messages in thread
From: Saraswati, Sujoy (OSTL) @ 2015-11-26 8:28 UTC (permalink / raw)
To: Joseph Myers, Sujoy Saraswati; +Cc: Richard Biener, GCC Patches
Hi,
> I think the general principle is:
>
> * The caller decides whether folding is desirable (whether it would lose
> exceptions, for example).
>
> * The real.c code is called only when the caller has decided that folding is
> desirable, and should always produce the correct output (which for a
> conversion means producing a quiet NaN from a signaling NaN).
>
> So both places need changes, but real_convert is where the code that makes
> it a quiet NaN should go.
>
> Another place in the patch that looks incorrect: the changes to fold-const-
> call.c calling real_powi and checking if the result is a signaling NaN. The result
> of real_powi should never be a signaling NaN.
> Rather, real_powi should produce a quiet NaN if its input is a signaling NaN,
> and the callers should check if the argument is a signaling NaN when deciding
> whether to fold, not if the result is.
I made the changes accordingly and will post the patches now.
Regards,
Sujoy
> --
> Joseph S. Myers
> joseph@codesourcery.com
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-11-26 8:24 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-01 10:23 Fix 61441 Sujoy Saraswati
2015-09-01 12:21 ` Richard Biener
2015-09-02 11:36 ` Sujoy Saraswati
2015-09-02 11:56 ` Richard Biener
2015-09-02 12:16 ` Sujoy Saraswati
2015-09-10 7:33 ` Sujoy Saraswati
2015-09-14 13:50 ` Richard Biener
2015-09-14 20:39 ` Joseph Myers
2015-09-16 13:01 ` Sujoy Saraswati
2015-09-16 17:03 ` Joseph Myers
2015-10-13 10:46 ` Sujoy Saraswati
2015-10-28 17:19 ` Joseph Myers
2015-11-05 11:29 ` Sujoy Saraswati
2015-11-05 16:58 ` Joseph Myers
2015-11-06 4:56 ` Sujoy Saraswati
2015-11-06 13:09 ` Joseph Myers
2015-11-26 8:28 ` Saraswati, Sujoy (OSTL)
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).