Hi Jakub,
Technically, PR97965 doesn't explicitly mention equality/inequality, but
you're
right, it makes sense to tackle this missed optimization at the same time as
we
fix the wrong-code.

On Thu, Nov 26, 2020, Jakub Jelinek wrote:
>On Thu, Nov 26, 2020 at 01:56:03PM -0000, Roger Sayle wrote:
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -3998,7 +3998,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>>      (cmp @0 { build_real (TREE_TYPE (@1), dconst0); }))
>>     /* x != NaN is always true, other ops are always false.  */
>>     (if (REAL_VALUE_ISNAN (TREE_REAL_CST (@1))
>> -    && ! HONOR_SNANS (@1))
>> +    && ! flag_trapping_math)
>
>Shouldn't this one stay as is for cmp == EQ_EXPR || cmp == NE_EXPR?
>I mean, == or != only raise exceptions on sNaNs, while other simple
comparisons raise on both sNaNs and qNaNs.

The case to be careful of here is that (x == qNaN) can still raise/trap if x
is a sNaN.
Hence, the full condition should include:

&& (!flag_trapping_math
    || ((cmp == EQ_EXPR || cmp == NE_EXPR)
        && !tree_expr_maybe_signaling_nan_p (@1)
        && !tree_expr_maybe_signaling_nan_p (@0)))

Note: I repeat tree_expr_maybe_signaling_nan_p here for symmetry, though
because
we know @1 is a NaN, this could have been written as
!tree_expr_signaling_nan_p (@1).

>> --- a/gcc/simplify-rtx.c
>> +++ b/gcc/simplify-rtx.c
>> @@ -5732,12 +5732,13 @@ simplify_const_relational_operation (enum
rtx_code code,
>>        if (REAL_VALUE_ISNAN (*d0) || REAL_VALUE_ISNAN (*d1))
>>      switch (code)
>>        {
>> +      case NE:
>> +        return flag_trapping_math ? 0 : const_true_rtx;
>
> And here too (for NE and would need moving EQ later too.

Yep/Agreed.  These cases become:

case NE:
        if (flag_trapping_math
            && (REAL_VALUE_ISSIGNALING_NAN (*d0)
                || REAL_VALUE_ISSIGANLING_NAN (*d1)))
          return 0;
        return const_true_rtx;

case EQ:
        if (flag_trapping_math
            && (REAL_VALUE_ISSIGNALING_NAN (*d0)
                || REAL_VALUE_ISSIGANLING_NAN (*d1)))
          return 0;
        return const0_rtx;


A revised patch is attached (which I've confirmed compiles but
haven't regression tested).

Many thanks again.

Roger
--

&& (!flag_trapping_math
    || ((cmp == EQ_EXPR || cmp == NE_EXPR)
        && !tree_expr_maybe_signaling_nan_p (@1)
        && !tree_expr_maybe_signaling_nan_p (@0)))

case NE:
        if (flag_trapping_math
            && (REAL_VALUE_ISSIGNALING_NAN (*d0)
                || REAL_VALUE_ISSIGANLING_NAN (*d1)))
          return 0;
        return const_true_rtx;

case EQ:
        if (flag_trapping_math
            && (REAL_VALUE_ISSIGNALING_NAN (*d0)
                || REAL_VALUE_ISSIGANLING_NAN (*d1)))
          return 0;
        return const0_rtx;

Reply via email to