On Mon, Jul 17, 2017 at 9:29 AM, Yuri Gribov <tetra2...@gmail.com> wrote: > Hi all, > > This is an updated version of patch in > https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00409.html . It prevents > optimization in presense of sNaNs (and qNaNs when comparison operator > is > >= < <=) to preserve FP exceptions. > > Note that I had to use -fsignaling-nans in pr57371-5.c test because by > default this option is off and some existing patterns in match.pd > happily optimize NaN comparisons, even with sNaNs (!). > > Bootstrapped and regtested on x64. Ok for trunk?
+ { + tree itype = TREE_TYPE (@0); + gcc_assert (INTEGRAL_TYPE_P (itype)); no need to spell out this assert. + + format_helper fmt (REAL_MODE_FORMAT (TYPE_MODE (TREE_TYPE (@1)))); + + const REAL_VALUE_TYPE *cst = TREE_REAL_CST_PTR (@1); + + /* Be careful to preserve any potential exceptions due to + NaNs. qNaNs are ok in == or != context. + + TODO: relax under -fno-trapping-math or + -fno-signaling-nans. */ + bool exception_p + = real_isnan (cst) && (cst->signalling + || (cmp != EQ_EXPR || cmp != NE_EXPR)); + + /* INT?_MIN is power-of-two so it takes + only one mantissa bit. */ please reduce vertical spacing + bool itype_fits_ftype_p + = TYPE_PRECISION (itype) - signed_p <= significand_size (fmt); watch spacing -- indent by two. + (with + { + REAL_VALUE_TYPE imin, imax; likewise. + if (!cst_int_p && cmp == GT_EXPR) + icmp = GE_EXPR; + else if (!cst_int_p && cmp == LT_EXPR) + icmp = LE_EXPR; ugh. Please do not assign to match.pd iterators, I'm going to commit a patch making them const. + } + + (switch + + /* Optimize cases when CST is outside of ITYPE's range. */ + (if (real_compare (LT_EXPR, cst, &imin)) + { constant_boolean_node (cmp == GT_EXPR || cmp == GE_EXPR || cmp == NE_EXPR, + type); }) + (if (real_compare (GT_EXPR, cst, &imax)) + { constant_boolean_node (cmp == LT_EXPR || cmp == LE_EXPR || cmp == NE_EXPR, + type); }) + + /* Remove cast if CST is an integer representable by ITYPE. */ + (if (cst_int_p) + (cmp @0 { gcc_assert (!overflow_p); + wide_int_to_tree (itype, icst_val); }) + ) + reduce vertical spacing, watch long lines. + /* Otherwise replace with sensible integer constant. */ + (with + { + gcc_assert (!overflow_p); + gcc_assert (real_compare (GE_EXPR, &icst, &imin) + && real_compare (LE_EXPR, &icst, &imax)); + gcc_assert (wi::ge_p (icst_val, wi::min_value (itype), isign) + && wi::le_p (icst_val, wi::max_value (itype), isign)); ugh. IFF then gcc_checking_assert but are those really necessary? Thanks, Richard. > -Y