Re: [PATCHv4][PING][PR 57371] Remove useless floating point casts in comparisons
On Tue, Jul 25, 2017 at 9:32 PM, Jeff Law wrote:
> On 07/25/2017 08:10 AM, Richard Biener wrote:
>> On Mon, Jul 17, 2017 at 9:29 AM, Yuri Gribov 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.
> Right. I think Yuri added this in response to a comment from me.
> However, I think the subsequent discussion made it clear that we don't
> actually need to check that @0 is an integral type.
I was initially scared to rely on particular semantics of
verify_gimple_assign_unary so left the assert in place, especially
given that it puzzled others as well. I'll remove it.
-Y
Re: [PATCHv4][PING][PR 57371] Remove useless floating point casts in comparisons
On 07/25/2017 08:10 AM, Richard Biener wrote:
> On Mon, Jul 17, 2017 at 9:29 AM, Yuri Gribov 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.
Right. I think Yuri added this in response to a comment from me.
However, I think the subsequent discussion made it clear that we don't
actually need to check that @0 is an integral type.
Jeff
Re: [PATCHv4][PING][PR 57371] Remove useless floating point casts in comparisons
On Mon, Jul 17, 2017 at 9:29 AM, Yuri Gribov 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
[PATCHv4][PING][PR 57371] Remove useless floating point casts in comparisons
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? -Y pr57371-4.patch Description: Binary data
