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

Reply via email to