Doh! Wrong patch2.txt file. Sorry. -----Original Message----- From: Roger Sayle <ro...@nextmovesoftware.com> Sent: 27 November 2020 10:52 To: 'Jakub Jelinek' <ja...@redhat.com>; 'Joseph S. Myers' <jos...@codesourcery.com> Cc: 'Richard Biener' <rguent...@suse.de>; 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org> Subject: RE: [PATCH] fold-const: Don't consider NaN non-negative [PR97965]
Hi Jakub, Technically, PR97965 doesn't explicitly mention equality/inequality, but you're right, it makes sense to tackle this missed optimization at the same time as we fix the wrong-code. On Thu, Nov 26, 2020, Jakub Jelinek wrote: >On Thu, Nov 26, 2020 at 01:56:03PM -0000, Roger Sayle wrote: >> --- 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) > >Shouldn't this one stay as is for cmp == EQ_EXPR || cmp == NE_EXPR? >I mean, == or != only raise exceptions on sNaNs, while other simple comparisons raise on both sNaNs and qNaNs. The case to be careful of here is that (x == qNaN) can still raise/trap if x is a sNaN. Hence, the full condition should include: && (!flag_trapping_math || ((cmp == EQ_EXPR || cmp == NE_EXPR) && !tree_expr_maybe_signaling_nan_p (@1) && !tree_expr_maybe_signaling_nan_p (@0))) Note: I repeat tree_expr_maybe_signaling_nan_p here for symmetry, though because we know @1 is a NaN, this could have been written as !tree_expr_signaling_nan_p (@1). >> --- 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; > > And here too (for NE and would need moving EQ later too. Yep/Agreed. These cases become: case NE: if (flag_trapping_math && (REAL_VALUE_ISSIGNALING_NAN (*d0) || REAL_VALUE_ISSIGANLING_NAN (*d1))) return 0; return const_true_rtx; case EQ: if (flag_trapping_math && (REAL_VALUE_ISSIGNALING_NAN (*d0) || REAL_VALUE_ISSIGANLING_NAN (*d1))) return 0; return const0_rtx; A revised patch is attached (which I've confirmed compiles but haven't regression tested). Many thanks again. Roger --
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 b6dfc24af88..ef1bfb42d12 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -4037,7 +4037,10 @@ 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 + || ((cmp == EQ_EXPR || cmp == NE_EXPR) + && !tree_expr_maybe_signaling_nan_p (@1) + && !tree_expr_maybe_signaling_nan_p (@0)))) { 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..dd97a353bff 100644 --- a/gcc/simplify-rtx.c +++ b/gcc/simplify-rtx.c @@ -5732,20 +5732,31 @@ simplify_const_relational_operation (enum rtx_code code, if (REAL_VALUE_ISNAN (*d0) || REAL_VALUE_ISNAN (*d1)) switch (code) { + case NE: + if (flag_trapping_math + && (REAL_VALUE_ISSIGNALING_NAN (*d0) + || REAL_VALUE_ISSIGNALING_NAN (*d1))) + return 0; + return const_true_rtx; case UNEQ: case UNLT: case UNGT: case UNLE: case UNGE: - case NE: case UNORDERED: return const_true_rtx; case EQ: + if (flag_trapping_math + && (REAL_VALUE_ISSIGNALING_NAN (*d0) + || REAL_VALUE_ISSIGNALING_NAN (*d1))) + return 0; + return const0_rtx; case LT: case GT: case LE: case GE: case LTGT: + return flag_trapping_math ? 0 : const0_rtx; case ORDERED: return const0_rtx; default: