On 8/15/20 5:10 AM, Roger Sayle wrote: > The motivation for this patch is PR middle-end/85811, a wrong-code > regression entitled "Invalid optimization with fmax, fabs and nan". > The optimization involves assuming max(x,y) is non-negative if (say) > y is non-negative, i.e. max(x,2.0). Unfortunately, this is an invalid > assumption in the presence of NaNs. Hence max(x,+qNaN), with IEEE fmax > semantics will always return x even though the qNaN is non-negative. > Worse, max(x,2.0) may return a negative value if x is -sNaN. > > I'll quote Joseph Myers (many thanks) who describes things clearly as: >> (a) When both arguments are NaNs, the return value should be a qNaN, >> but sometimes it is an sNaN if at least one argument is an sNaN. >> (b) Under TS 18661-1 semantics, if either argument is an sNaN then the >> result should be a qNaN (whereas if one argument is a qNaN and the >> other is not a NaN, the result should be the non-NaN argument). >> Various implementations treat sNaNs like qNaNs here. > Under this logic, the tree_expr_nonnegative_p for IEEE fmax should be: > > CASE_CFN_FMAX: > CASE_CFN_FMAX_FN: > /* Usually RECURSE (arg0) || RECURSE (arg1) but NaNs complicate > things. In the presence of sNaNs, we're only guaranteed to be > non-negative if both operands are non-negative. In the presence > of qNaNs, we're non-negative if either operand is non-negative > and can't be a qNaN, or if both operands are non-negative. */ > if (tree_expr_maybe_signaling_nan_p (arg0) || > tree_expr_maybe_signaling_nan_p (arg1)) > return RECURSE (arg0) && RECURSE (arg1); > return RECURSE (arg0) ? (!tree_expr_maybe_nan_p (arg0) > || RECURSE (arg1)) > : (RECURSE (arg1) > && !tree_expr_maybe_nan_p (arg1)); > > Which indeed resolves the wrong code in the PR. The infrastructure that > makes this possible are the two new functions tree_expr_maybe_nan_p and > tree_expr_maybe_signaling_nan_p which test whether a value may potentially > be a NaN or a signaling NaN respectively. In fact, this patch adds seven > new predicates to the middle-end: > > bool tree_expr_finite_p (const_tree); > bool tree_expr_infinite_p (const_tree); > bool tree_expr_maybe_infinite_p (const_tree); > bool tree_expr_signaling_nan_p (const_tree); > bool tree_expr_maybe_signaling_nan_p (const_tree); > bool tree_expr_nan_p (const_tree); > bool tree_expr_maybe_nan_p (const_tree); > > These functions correspond to the "must" and "may" operators in modal logic, > and allow us to triage expressions in the middle-end; definitely a NaN, > definitely not a NaN, and unknown at compile-time, etc. A prime example of > the utility of these functions is that a IEEE floating point value promoted > from an integer type can't be a NaN or infinite. Hence (double)i+0.0 where > i is an integer can be simplified to (double)i even with -fsignaling-nans. > Currently in GCC optimizations are enabled/disabled based on whether the > expression's type supports NaNs or sNaNs; with these new predicates they > can be controlled by whether the actual operands may or may not be NaNs. > > Having added these extremely useful helper functions to the middle-end, > I couldn't help by use then in a few places in fold-const.c, builtins.c > and match.pd. In the near term, these can/should be used in places > where the tree optimizers test for HONOR_NANS, HONOR_INFINITIES or > HONOR_SNANS, or explicitly test whether a REAL_CST is a NaN or Inf. > In the longer term (I'm not volunteering) these predicates could perhaps > be hooked into the middle-end's SSA chaining and/or VRP machinery, > allowing finiteness to propagated around the CFG, much like we > currently propagate value ranges. > > This patch has been tested on x86_64-pc-linux-gnu with a "make bootstrap" > and "make -k check". > Ok for mainline? > > > 2020-08-15 Roger Sayle <ro...@nextmovesoftware.com> > > gcc/ChangeLog > PR middle-end/85811 > * fold-const.c (tree_expr_finite_p): New function to test whether > a tree expression must be finite, i.e. not a FP NaN or infinity. > (tree_expr_infinite_p): New function to test whether a tree > expression must be infinite, i.e. a FP infinity. > (tree_expr_maybe_infinite_p): New function to test whether a tree > expression may be infinite, i.e. a FP infinity. > (tree_expr_signaling_nan_p): New function to test whether a tree > expression must evaluate to a signaling NaN (sNaN). > (tree_expr_maybe_signaling_nan_p): New function to test whether a > tree expression may be a signaling NaN (sNaN). > (tree_expr_nan_p): New function to test whether a tree expression > must evaluate to a (quiet or signaling) NaN. > (tree_expr_maybe_nan_p): New function to test whether a tree > expression me be a (quiet or signaling) NaN. > > (tree_binary_nonnegative_warnv_p) [MAX_EXPR]: In the presence > of NaNs, MAX_EXPR is only guaranteed to be non-negative, if both > operands are non-negative. > (tree_call_nonnegative_warnv_p) [CASE_CFN_FMAX,CASE_CFN_FMAX_FN]: > In the presence of signaling NaNs, fmax is only guaranteed to be > non-negative if both operands are negative. In the presence of > quiet NaNs, fmax is non-negative if either operand is non-negative > and not a qNaN, or both operands are non-negative. > > * fold-const.h (tree_expr_finite_p, tree_expr_infinite_p, > tree_expr_maybe_infinite_p, tree_expr_signaling_nan_p, > tree_expr_maybe_signaling_nan_p, tree_expr_nan_p, > tree_expr_maybe_nan_p): Prototype new functions here. > > * builtins.c (fold_builtin_classify) [BUILT_IN_ISINF]: Fold to > a constant if argument is known to be (or not to be) an Infinity. > [BUILT_IN_ISFINITE]: Fold to a constant if argument is known to > be (or not to be) finite. > [BUILT_IN_ISNAN]: Fold to a constant if argument is known to be > (or not to be) a NaN. > (fold_builtin_fpclassify): Check tree_expr_maybe_infinite_p and > tree_expr_maybe_nan_p instead of HONOR_INFINITIES and HONOR_NANS > respectively. > (fold_builtin_unordered_cmp): Fold UNORDERED_EXPR to a constant > when its arguments are known to be (or not be) NaNs. Check > tree_expr_maybe_nan_p instead of HONOR_NANS when choosing between > unordered and regular forms of comparison operators. > > * match.pd (ordered(x,y)->true/false): Constant fold ORDERED_EXPR > if its operands are known to be (or not to be) NaNs. > (unordered(x,y)->true/false): Constant fold UNORDERED_EXPR if its > operands are known to be (or not to be) NaNs. > (sqrt(x)*sqrt(x)->x): Check tree_expr_maybe_signaling_nan_p instead > of HONOR_SNANS. > > gcc/testsuite/ChangeLog > PR middle-end/85811 > * gcc.dg/pr85811.c: New test. > * gcc.dg/fold-isfinite-1.c: New test. > * gcc.dg/fold-isfinite-2.c: New test. > * gcc.dg/fold-isinf-1.c: New test. > * gcc.dg/fold-isinf-2.c: New test. > * gcc.dg/fold-isnan-1.c: New test. > * gcc.dg/fold-isnan-2.c: New test. So as you noted in your comments, it'd be be extremely useful to be able to track state of special FP values nan, zero, inf and the like in VRP. It's something we discussed repeatedly internally and we think it's possible in the ranger infrastructure that's recently landed. That information opens up all kinds of interesting possibilities that you've started to touch on in your patch. But that's all for another day :-) I'll pushed this to the trunk. Thanks! And sorry for the long delay. jeff