diff mbox

Fix 61441 [5/5] Disable various transformations for signaling NaN operands

Message ID 5379BA8D7E9D7E4D87BF6749A92854C248FC90F6@G4W3297.americas.hpqcorp.net
State New
Headers show

Commit Message

Saraswati, Sujoy (OSTL) Nov. 26, 2015, 8:38 a.m. UTC 
Hi,
 This patch avoids various transformations with signaling NaN operands when flag_signaling_nans is on, to avoid folding which would lose exceptions. A test case for this change is also added as part of this patch.
Regards,
Sujoy
  
  2015-11-26  Sujoy Saraswati <sujoy.saraswati@hpe.com>
    
            PR tree-optimization/61441
            * 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.
            * fold-const-call.c (fold_const_pow): Avoid the operation
            if flag_signaling_nans is on and the operand is an sNaN.
            (fold_const_builtin_load_exponent) Same.
            (fold_const_call_sss): Same for BUILT_IN_POWI.
            * 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.
            * 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): Same.
            * 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.

Comments

Jeff Law Dec. 2, 2015, 7:02 p.m. UTC | #1 
On 11/26/2015 01:38 AM, Saraswati, Sujoy (OSTL) wrote:
> Hi,
>   This patch avoids various transformations with signaling NaN operands when flag_signaling_nans is on, to avoid folding which would lose exceptions. A test case for this change is also added as part of this patch.
> Regards,
> Sujoy
>
>    2015-11-26  Sujoy Saraswati <sujoy.saraswati@hpe.com>
>
>              PR tree-optimization/61441
>              * 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.
>              * fold-const-call.c (fold_const_pow): Avoid the operation
>              if flag_signaling_nans is on and the operand is an sNaN.
>              (fold_const_builtin_load_exponent) Same.
>              (fold_const_call_sss): Same for BUILT_IN_POWI.
>              * 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.
>              * 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): Same.
>              * 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.
>
> ===================================================================
> diff -u -p a/gcc/fold-const.c b/gcc/fold-const.c
> --- a/gcc/fold-const.c  2015-11-25 15:24:49.656116740 +0530
> +++ b/gcc/fold-const.c  2015-11-25 15:25:07.712117294 +0530
> @@ -1166,9 +1166,21 @@ const_binop (enum tree_code code, tree a
>         /* 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;
> +      }
I was a bit worried about this hunk, but by the time we get here we 
already know that at least one operand is a sNaN and that we're not 
honoring sNaNs.


>
>         inexact = real_arithmetic (&value, code, &d1, &d2);
>         real_convert (&result, mode, &value);
> @@ -1538,6 +1550,15 @@ const_binop (enum tree_code code, tree t
>   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;
Why let NEGATE_EXPR and ABS_EXPR pass through here?  I realize that 
these can often be implemented with bit-twiddling, so they're usually 
considered special.  BUt in this case aren't we just dealing with 
constants and wouldn't we want to still express the neg/abs so that we 
get a signal when the input value is sNaN rather than collapse down to a 
constant?





>
>   /* 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.
s/signalling/signaling/  I think it shows up in multiple places, so 
check for it using search/replace.

Also watch for using 8 spaces rather than a tab in your patches.  I 
think I see occurrences of that mistake in this patch.  SO just check 
everything you changed to be sure you aren't adding new instances of 
that nit.


I think a bit of explanation why you're letting NEGATE/ABS though in 
const_unop, fixing the spelling goof and the <tab> instead of 8 spaces 
nit need to be addressed before this can be committed.

jeff
Joseph Myers Dec. 2, 2015, 7:08 p.m. UTC | #2 
On Wed, 2 Dec 2015, Jeff Law wrote:

> Why let NEGATE_EXPR and ABS_EXPR pass through here?  I realize that these can
> often be implemented with bit-twiddling, so they're usually considered
> special.  BUt in this case aren't we just dealing with constants and wouldn't
> we want to still express the neg/abs so that we get a signal when the input
> value is sNaN rather than collapse down to a constant?

See IEEE 754-2008, 5.5.1.  "Implementations shall provide the following 
homogeneous quiet-computational sign bit operations for all supported 
arithmetic formats; they only affect the sign bit. The operations treat 
floating-point numbers and NaNs alike, and signal no exception. These 
operations may propagate non-canonical encodings.".
Jeff Law Dec. 2, 2015, 7:10 p.m. UTC | #3 
On 12/02/2015 12:08 PM, Joseph Myers wrote:
> On Wed, 2 Dec 2015, Jeff Law wrote:
>
>> Why let NEGATE_EXPR and ABS_EXPR pass through here?  I realize that these can
>> often be implemented with bit-twiddling, so they're usually considered
>> special.  BUt in this case aren't we just dealing with constants and wouldn't
>> we want to still express the neg/abs so that we get a signal when the input
>> value is sNaN rather than collapse down to a constant?
>
> See IEEE 754-2008, 5.5.1.  "Implementations shall provide the following
> homogeneous quiet-computational sign bit operations for all supported
> arithmetic formats; they only affect the sign bit. The operations treat
> floating-point numbers and NaNs alike, and signal no exception. These
> operations may propagate non-canonical encodings.".
Ah, in that case, nevermind :-)

So I think it's just the spelling and whitespace nits that need to be fixed.

jeff
Jakub Jelinek Dec. 31, 2015, 9:35 a.m. UTC | #4 
On Thu, Nov 26, 2015 at 08:38:55AM +0000, Saraswati, Sujoy (OSTL) wrote:
>             PR tree-optimization/61441
>             * gcc.dg/pr61441.c: New testcase.

Note the testcase fails on i686-linux, and even -fexcess-precision=standard
doesn't help (-ffloat-store works, but that is a big hammer and we really
don't want it for targets without excess precision).
At least with -fexcess-precision=standard, the problem is in the fabs case,
with -fexcess-precision=standard the value is then rounded from long double
to double and that rounding affects the signalling bit.
So, either the testcase should be compiled with -fexcess-precision=standard
and the operation(Abs) case be guarded with #if __FLT_EVAL_METHOD__ == 0, or something
similar.

	Jakub
diff mbox

Patch

===================================================================
diff -u -p a/gcc/fold-const.c b/gcc/fold-const.c
--- a/gcc/fold-const.c  2015-11-25 15:24:49.656116740 +0530
+++ b/gcc/fold-const.c  2015-11-25 15:25:07.712117294 +0530
@@ -1166,9 +1166,21 @@  const_binop (enum tree_code code, tree a
       /* 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);
@@ -1538,6 +1550,15 @@  const_binop (enum tree_code code, tree t
 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:
@@ -1949,6 +1970,12 @@  fold_convert_const_real_from_real (tree
   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));
   t = build_real (type, value);

@@ -13420,7 +13447,7 @@  tree_single_nonzero_warnv_p (tree t, boo

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

@@ -13453,7 +13480,7 @@  integer_valued_real_unary_p (tree_code c

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

@@ -13477,8 +13504,8 @@  integer_valued_real_binary_p (tree_code

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

@@ -13507,7 +13534,7 @@  integer_valued_real_call_p (combined_fn

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

@@ -13541,7 +13568,7 @@  integer_valued_real_single_p (tree t, in

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

@@ -13569,6 +13596,7 @@  integer_valued_real_invalid_p (tree t, i

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

===================================================================
diff -u -p a/gcc/fold-const-call.c b/gcc/fold-const-call.c
--- a/gcc/fold-const-call.c     2015-11-25 15:38:34.780142086 +0530
+++ b/gcc/fold-const-call.c     2015-11-25 15:38:05.840141197 +0530
@@ -512,7 +512,11 @@  fold_const_pow (real_value *result, cons
          || !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 (*arg0))))
        return true;
     }

@@ -541,6 +545,13 @@  fold_const_builtin_load_exponent (real_v
   if (wi::les_p (arg1, -max_exp_adj) || wi::ges_p (arg1, max_exp_adj))
     return false;

+  /* Don't perform operation if we honor signaling NaNs and
+     operand is a NaN.  */
+  if (!flag_unsafe_math_optimizations
+      && flag_signaling_nans
+      && REAL_VALUE_ISSIGNALING_NAN (*arg0))
+    return false;
+
   REAL_VALUE_TYPE initial_result;
   real_ldexp (&initial_result, arg0, arg1.to_shwi ());

@@ -1202,6 +1213,12 @@  fold_const_call_sss (real_value *result,
                                                   format));

     CASE_CFN_POWI:
+      /* Avoid the folding if flag_signaling_nans is on.  */
+      if (!flag_unsafe_math_optimizations
+          && flag_signaling_nans
+          && REAL_VALUE_ISSIGNALING_NAN (*arg0))
+        return false;
+
       real_powi (result, format, arg0, arg1.to_shwi ());
       return true;

===================================================================
diff -u -p a/gcc/gimple-fold.c b/gcc/gimple-fold.c
--- a/gcc/gimple-fold.c 2015-11-25 15:41:58.200148334 +0530
+++ b/gcc/gimple-fold.c 2015-11-25 15:41:33.100147563 +0530
@@ -6266,7 +6266,7 @@  gimple_stmt_nonnegative_warnv_p (gimple

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

@@ -6295,7 +6295,7 @@  gimple_assign_integer_valued_real_p (gim

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

@@ -6314,7 +6314,7 @@  gimple_call_integer_valued_real_p (gimpl

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

@@ -6332,7 +6332,7 @@  gimple_phi_integer_valued_real_p (gimple

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

===================================================================
diff -u -p a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
--- a/gcc/simplify-rtx.c        2015-11-25 15:49:46.740162727 +0530
+++ b/gcc/simplify-rtx.c        2015-11-25 15:49:03.200161389 +0530
@@ -1703,6 +1703,11 @@  simplify_const_unary_operation (enum rtx
        }

       real_from_integer (&d, mode, std::make_pair (op, op_mode), SIGNED);
+
+      /* Avoid the folding if flag_signaling_nans is on.  */
+      if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
+        return 0;
+
       d = real_value_truncate (mode, d);
       return const_double_from_real_value (d, mode);
     }
@@ -1721,6 +1726,11 @@  simplify_const_unary_operation (enum rtx
        }

       real_from_integer (&d, mode, std::make_pair (op, op_mode), UNSIGNED);
+
+      /* Avoid the folding if flag_signaling_nans is on.  */
+      if (HONOR_SNANS (mode) && REAL_VALUE_ISSIGNALING_NAN (d))
+        return 0;
+
       d = real_value_truncate (mode, d);
       return const_double_from_real_value (d, mode);
     }
@@ -1825,16 +1835,25 @@  simplify_const_unary_operation (enum rtx
          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:
          {
@@ -3886,16 +3905,20 @@  simplify_const_binary_operation (enum rt
       else
        {
          REAL_VALUE_TYPE f0, f1, value, result;
+          const REAL_VALUE_TYPE *opr0, *opr1;
          bool inexact;

-         real_convert (&f0, mode, CONST_DOUBLE_REAL_VALUE (op0));
-         real_convert (&f1, mode, CONST_DOUBLE_REAL_VALUE (op1));
+          opr0 = CONST_DOUBLE_REAL_VALUE (op0);
+          opr1 = CONST_DOUBLE_REAL_VALUE (op1);

          if (HONOR_SNANS (mode)
-              && (REAL_VALUE_ISSIGNALING_NAN (f0)
-                  || REAL_VALUE_ISSIGNALING_NAN (f1)))
+              && (REAL_VALUE_ISSIGNALING_NAN (*opr0)
+                  || REAL_VALUE_ISSIGNALING_NAN (*opr1)))
            return 0;

+         real_convert (&f0, mode, opr0);
+         real_convert (&f1, mode, opr1);
+
          if (code == DIV
              && real_equal (&f1, &dconst0)
              && (flag_trapping_math || ! MODE_HAS_INFINITIES (mode)))

===================================================================
diff -u -p a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
--- a/gcc/tree-ssa-math-opts.c  2015-11-25 15:52:00.564166838 +0530
+++ b/gcc/tree-ssa-math-opts.c  2015-11-23 16:10:39.694893171 +0530
@@ -1480,6 +1480,13 @@  gimple_expand_builtin_pow (gimple_stmt_i
   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;
+}