[Bug middle-end/67052] New: tree_single_nonnegative_warnv_p and fold_relational_const are inconsistent with NaNs

[Bug middle-end/67052] New: tree_single_nonnegative_warnv_p and fold_relational_const are inconsistent with NaNs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67052

            Bug ID: 67052
           Summary: tree_single_nonnegative_warnv_p and
                    fold_relational_const are inconsistent with NaNs
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Keywords: missed-optimization, wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
                CC: jsm28 at gcc dot gnu.org
  Target Milestone: ---

int main ()
{
  double x = __builtin_nan ("");
  if (x < 0.0)
    return 1;
  return 0;
}

this simplifies via

      /* Convert ABS_EXPR<x> < 0 to false.  */
      strict_overflow_p = false;
      if (code == LT_EXPR
          && (integer_zerop (arg1) || real_zerop (arg1))
          && tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p))
        {
          if (strict_overflow_p)
            fold_overflow_warning (("assuming signed overflow does not occur "
                                    "when simplifying comparison of "
                                    "absolute value and zero"),
                                   WARN_STRICT_OVERFLOW_CONDITIONAL);
          return omit_one_operand_loc (loc, type,
                                       constant_boolean_node (false, type),
                                       arg0);
        }

(the ABS_EXPR<x> >= 0 case is guarded with ! HONOR_NANS)

but not via constant folding in fold_relational_const.

bool
tree_single_nonnegative_warnv_p (tree t, bool *strict_overflow_p)
{
...
    case REAL_CST:
      return ! REAL_VALUE_NEGATIVE (TREE_REAL_CST (t));

but

static tree
fold_relational_const (enum tree_code code, tree type, tree op0, tree op1)
{
...
      /* Handle the cases where either operand is a NaN.  */
      if (real_isnan (c0) || real_isnan (c1))
        {
...
            case LT_EXPR:
            case LE_EXPR:
            case GT_EXPR:
            case GE_EXPR:
            case LTGT_EXPR:
              if (flag_trapping_math)
                return NULL_TREE;

is this a missed optimization or wrong-code?

[Bug middle-end/67052] tree_single_nonnegative_warnv_p and fold_relational_const are inconsistent with NaNs

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67052

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
The uses of the *nonnegative* functions should be removed to determine 
what semantics are expected for for floating-point arguments.

If the semantics are "sign bit is 0", NaNs should return according to 
their sign bit (and -0 should return false).  If the semantics are "value
>= 0", NaNs should always return false, but -0 should return true.  If 
the semantics are "!(value < 0)", NaNs should return true, as should -0.  
It's possible different places expect different semantics.

>       /* Convert ABS_EXPR<x> < 0 to false.  */
>       strict_overflow_p = false;
>       if (code == LT_EXPR
>           && (integer_zerop (arg1) || real_zerop (arg1))
>           && tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p))
>         {
>           if (strict_overflow_p)
>             fold_overflow_warning (("assuming signed overflow does not occur "
>                                     "when simplifying comparison of "
>                                     "absolute value and zero"),
>                                    WARN_STRICT_OVERFLOW_CONDITIONAL);
>           return omit_one_operand_loc (loc, type,
>                                        constant_boolean_node (false, type),
>                                        arg0);

Omitting an ordered NaN comparison with 0 also loses an exception (if 
flag_trapping_math && HONOR_NANS) (as does an equality comparison in the 
case of signaling NaNs).  (Modulo the unresolved discussion in the thread 
starting at 
<https://gcc.gnu.org/ml/gcc-patches/2015-02/threads.html#00555> of which 
sort of comparison LTGT should be, and so which existing code handling it 
the other way is buggy.)

[Bug middle-end/67052] tree_single_nonnegative_warnv_p and fold_relational_const are inconsistent with NaNs

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67052

--- Comment #2 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
(s/removed/reviewed/)

[Bug middle-end/67052] tree_single_nonnegative_warnv_p and fold_relational_const are inconsistent with NaNs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67052

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
This means that the ABS_EXPR<x> < 0 code is correct in simplifying
NaN < 0 to true and it is correct to _not_ simplify NaN >= 0 to true.

But at the same time it has to do the same as fold_relational_const and
preserve traps with -ftrapping-math for the ordered compares.

So the following patch should fix the inconsistency and avoid the wrong-code
issue in the ABS_EXPR<x> < 0 folding:

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 226387)
+++ gcc/fold-const.c    (working copy)
@@ -11634,7 +11455,9 @@ fold_binary_loc (location_t loc,
       /* Convert ABS_EXPR<x> < 0 to false.  */
       strict_overflow_p = false;
       if (code == LT_EXPR
-         && (integer_zerop (arg1) || real_zerop (arg1))
+         && (integer_zerop (arg1)
+             || ((! HONOR_NANS (arg0) || !flag_trapping_math)
+                 && real_zerop (arg1)))
          && tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p))
        {
          if (strict_overflow_p)