On Thu, Sep 10, 2015 at 9:30 AM, Sujoy Saraswati <ssarasw...@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 <ssarasw...@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.guent...@gmail.com> wrote:
>> On Wed, Sep 2, 2015 at 1:36 PM, Sujoy Saraswati <ssarasw...@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 <ssarasw...@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;
>>>>> +}
