Re: [PATCH][PR 57371] Remove useless floating point casts in comparisons

[Yuri Gribov <tetra2005 at gmail dot com>]

On Mon, Jul 3, 2017 at 4:38 PM, Jeff Law <law@redhat.com> wrote:
> On 07/02/2017 11:03 AM, Yuri Gribov wrote:
>> Hi all,
>>
>> This is initial patch for
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57371 . Per Joseph's
>> suggestion it optimizes
>>   (float)lhs CMP rhs
>>   (double)lhs CMP rhs
>> to
>>   lhs CMP (typeof(x))rhs
>> whenever typeof(x) can be precisely represented by floating-point type
>> (e.g. short by float or int by double) and rhs can be precisely
>> represented by typeof(x).
>>
>> Bootstrapped/regtested on x64. Ok for trunk?
>>
>> I'd like to extend this further in follow-up patches:
>> 1) fold always-false/always-true comparisons e.g.
>>   short x;
>>   (float)x > INT16_MAX;  // Always false
>> 2) get rid of cast in comparisons with zero regardless of typeof(lhs)
>> when -fno-trapping-math:
>>   (float_or_double)lhs CMP 0
>>
>> -Y
>>
>>
>> pr57371-1.patch
>>
>>
>> 2017-07-02  Yury Gribov  <tetra2005@gmail.com>
>>
>>       PR tree-optimization/57371
>>       * match.pd: New pattern.
>>       * testsuite/gcc.dg/pr57371-1.c: New test.
>>       * testsuite/gcc.dg/pr57371-2.c: New test.
>>
>> diff -rupN gcc/gcc/match.pd gcc-57371/gcc/match.pd
>> --- gcc/gcc/match.pd  2017-06-29 21:14:57.000000000 +0200
>> +++ gcc-57371/gcc/match.pd    2017-07-01 09:08:04.000000000 +0200
>> @@ -2802,7 +2802,35 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>>     (simplify
>>      (cmp (sq @0) (sq @1))
>>        (if (! HONOR_NANS (@0))
>> -     (cmp @0 @1))))))
>> +     (cmp @0 @1)))))
>> +
>> + /* Get rid of float cast in
>> +     (float_type)N CMP M
>> +    if N and M are within the range explicitly representable
>> +    by float type.
>> +
>> +    TODO: fold always true/false comparisons if M is outside valid range.  */
>> + (simplify
>> +  (cmp (float @0) REAL_CST@1)
>> +  (if (SCALAR_FLOAT_TYPE_P (TREE_TYPE (@1)))
>> +   (with
>> +    {
>> +      tree itype = TREE_TYPE (@0);
>> +
>> +      const real_format *fmt = REAL_MODE_FORMAT (TYPE_MODE (TREE_TYPE (@1)));
>> +
>> +      const REAL_VALUE_TYPE *rhs = TREE_REAL_CST_PTR (@1);
>> +      bool not_rhs_int_p = false;
>> +      wide_int rhs_int = real_to_integer (rhs, &not_rhs_int_p, TYPE_PRECISION (itype));
>> +    }
>> +    (if (!not_rhs_int_p
>> +         && !(TYPE_UNSIGNED (itype) && real_isneg (rhs))
>> +         && wi::ge_p (rhs_int, wi::min_value (itype), TYPE_SIGN (itype))
>> +         && wi::le_p (rhs_int, wi::max_value (itype), TYPE_SIGN (itype))
>> +         && TYPE_PRECISION (itype) <= significand_size (fmt))
>> +     (cmp @0 { wide_int_to_tree (itype, rhs_int); })
>> +    ))))
>> +)
> Seems like a nit, but instead of "not_rhs_int_p" use "fail" or something
> like that.  That makes it easier to mentally parse the conditional which
> uses the result.

Actually it's even worse than that, it should actually be overflow_p
and for not_rhs_int_p I need to use other APIs.

> What happens if @0 is a floating point type?  Based on the variable name
> "itype" and passing TYPE_PRECISION (itype) to real_to_integer, it seems
> like you're expecting @0 to be an integer.  If so, you should verify
> that it really is an integer type.  Seems like a good thing to verify
> with tests as well.

Right.

-Y