Many thanks for including me on this discussion. It's extremely interesting...
NaNs have a sign-bit, so copyexpr (and negate and ABS_EXPR) are well-defined, and it's reasonable for nonnegative_p to reflect this. IMHO, the true bug is that we can't fold away (any) comparisons against NaN when flag_trapping_math, irrespective of qNaN, -qNaN, sNaN or -sNaN. My completely untested solution is the attached patch. My apologies, I'm not even set up to compile things on the laptop that I'm composing this e-mail on, but my notes/proposals on tackling PR97965 are easier expressed as the actual suggested changes/edits. [Forgive me if I've made a typo]. As Joseph correctly points out, signaling NaNs are a red herring here, as both sNaNs and qNaNs may trap during regular (ordered) comparisons. The piece that I'll leave to the (C++) front-end folks to argue, is whether constexpr implies -fno-trapping-math like initializers, c.f. START_FOLD_INIT/END_FOLD_INIT in fold-const.c. This controls whether the test case should be consistently true or consistently false, but the patches above address Jakub's concern in the PR that things should at least be consistent. I hope this helps. I'm happy to spin this patch myself but it may take a little while. Hopefully, this is sufficient to point folks in the right (or one possible) direction. Best regards, Roger -- Roger Sayle NextMove Software Limited Cambridge, UK -----Original Message----- From: Richard Biener <rguent...@suse.de> Sent: 26 November 2020 10:04 To: Jakub Jelinek <ja...@redhat.com> Cc: Joseph S. Myers <jos...@codesourcery.com>; Jason Merrill <ja...@redhat.com>; gcc-patches@gcc.gnu.org; ro...@nextmovesoftware.com Subject: Re: [PATCH] fold-const: Don't consider NaN non-negative [PR97965] On Thu, 26 Nov 2020, Jakub Jelinek wrote: > On Thu, Nov 26, 2020 at 09:16:29AM +0000, Richard Biener wrote: > > > So, I really don't know if we want this or not, posting it for discussions. > > > > Is copysign (x, NaN) supposed to be well-defined? We'd stop folding > > this then, no? > > Yes, we'd stop folding several cases with NaNs. > > > I think the ABS_EXPR<x> < 0 to false folding is simply incomplete > > and should first check whether the operands are ordered? That said, > > NaN is nonnegative but NaN < 0 isn't false(?) > > > > So I don't think the patch is good. > > Another possibility (if we have this optimization already in match.pd > too) would be to only optimize the < 0 case in GENERIC if !HONOR_NANS > like the >= 0 case is and only optimize it in GIMPLE. Though with the > default -ftrapping-math I think even optimizing qNaN < 0 to 0 is > incorrect, even that should raise invalid exception, shouldn't it? > So perhaps add a defaulted argument to the *nonnegative* APIs that > would say whether unordered is ok or not? Roger recently added some exhaustive changes in related areas, so let's see if he has anything to say here. Richard.
diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 632a241a964..b76e80c02a3 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -12007,8 +12007,8 @@ fold_binary_loc (location_t loc, enum tree_code code, tree type, strict_overflow_p = false; if (code == GE_EXPR && (integer_zerop (arg1) - || (! HONOR_NANS (arg0) - && real_zerop (arg1))) + || (real_zerop (arg1) + && !tree_expr_maybe_nan_p (arg0))) && tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p)) { if (strict_overflow_p) @@ -12024,7 +12024,10 @@ fold_binary_loc (location_t loc, enum tree_code code, tree type, /* Convert ABS_EXPR<x> < 0 to false. */ strict_overflow_p = false; if (code == LT_EXPR - && (integer_zerop (arg1) || real_zerop (arg1)) + && (integer_zerop (arg1) + || (real_zerop (arg1) + && (!flag_trapping_math + || !tree_expr_maybe_nan_p (arg0)))) && tree_expr_nonnegative_warnv_p (arg0, &strict_overflow_p)) { if (strict_overflow_p) diff --git a/gcc/match.pd b/gcc/match.pd index f8b65154a9e..dec19ed2d57 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -3998,7 +3998,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (cmp @0 { build_real (TREE_TYPE (@1), dconst0); })) /* x != NaN is always true, other ops are always false. */ (if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1)) - && ! HONOR_SNANS (@1)) + && ! flag_trapping_math) { constant_boolean_node (cmp == NE_EXPR, type); }) /* Fold comparisons against infinity. */ (if (REAL_VALUE_ISINF (TREE_REAL_CST (@1)) diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c index 47e7aebda8a..ab53570ac8c 100644 --- a/gcc/simplify-rtx.c +++ b/gcc/simplify-rtx.c @@ -5732,12 +5732,13 @@ simplify_const_relational_operation (enum rtx_code code, if (REAL_VALUE_ISNAN (*d0) || REAL_VALUE_ISNAN (*d1)) switch (code) { + case NE: + return flag_trapping_math ? 0 : const_true_rtx; case UNEQ: case UNLT: case UNGT: case UNLE: case UNGE: - case NE: case UNORDERED: return const_true_rtx; case EQ: @@ -5746,6 +5747,7 @@ simplify_const_relational_operation (enum rtx_code code, case LE: case GE: case LTGT: + return flag_trapping_math ? 0 : const0_rtx; case ORDERED: return const0_rtx; default: