Re: Fix 61441

[Joseph Myers <joseph at codesourcery dot com>]

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