On Mon, Jul 17, 2017 at 9:29 AM, Yuri Gribov <[email protected]> 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