OK.

On Mon, Apr 3, 2023, 21:54 Jakub Jelinek <jakub@redhat.com> wrote:

> Hi!
>
> I've missed one of my recent range-op-float.cc changes (likely the
> r13-6967 one) caused
> FAIL: libphobos.phobos/std/math/algebraic.d execution test
> FAIL: libphobos.phobos_shared/std/math/algebraic.d execution test
> regressions, distilled into a C testcase below.
>
> In the testcase, we have
> !(u >= v)
> condition where both u and v are results of fabs*, which guards
> t1 = u u<= __FLT_MAX__;
> and
> t2 = v u<= __FLT_MAX__;
> comparisons.  From false u >= v where u and v have [0.0, +Inf] NAN
> ranges we (incorrectly deduce that one of them is [nextafterf (0.0, 1.0),
> +Inf] NAN
> and the other is [0.0, nextafterf (+Inf, 0.0)] NAN and from that deduce
> that
> one of the comparisons is always true, because UNLE_EXPR with the maximum
> representable number are false only if the value is +Inf and our ranges
> tell
> that is not possible.
>
> The bug is that the u >= v comparison determines a sensible range only when
> it is true - we then know neither operand can be NAN and it behaves
> correctly.  But when the comparison is false, our current code gives
> sensible answers only if the other op can't be NAN.  If it can be NAN,
> whenever it is NAN, the comparison is always false regardless of the other
> value, so the other value needs to be VARYING.
> Now, foperator_unordered_lt::op1_range etc. had code to deal with that
> for op?.known_nan (), but as the testcase shows, it is enough if it may be
> a
> NAN at runtime to make it VARYING.
>
> So, the following patch replaces for all those BRS_FALSE cases of the
> normal
> non-equality comparisons if (opOTHER.known_isnan ()) r.set_varying (type);
> to do it also if maybe_isnan ().
>
> For the unordered or ... comparisons, it is similar for BRS_TRUE.  Those
> comparisons are true whenever either operand is NAN, or if neither is NAN,
> the corresponding normal comparison.  So, if those comparisons are true
> and other operand might be a NAN, we can't tell (VARYING), if it is false,
> currently handling is correct.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, fixes those 2
> D testcases and the newly added one.  Ok for trunk?
>
> 2023-04-03  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/109386
>         * range-op-float.cc (foperator_lt::op1_range,
> foperator_lt::op2_range,
>         foperator_le::op1_range, foperator_le::op2_range,
>         foperator_gt::op1_range, foperator_gt::op2_range,
>         foperator_ge::op1_range, foperator_ge::op2_range): Make r varying
> for
>         BRS_FALSE case even if the other op is maybe_isnan, not just
>         known_isnan.
>         (foperator_unordered_lt::op1_range,
> foperator_unordered_lt::op2_range,
>         foperator_unordered_le::op1_range,
> foperator_unordered_le::op2_range,
>         foperator_unordered_gt::op1_range,
> foperator_unordered_gt::op2_range,
>         foperator_unordered_ge::op1_range,
> foperator_unordered_ge::op2_range):
>         Make r varying for BRS_TRUE case even if the other op is
> maybe_isnan,
>         not just known_isnan.
>
>         * gcc.c-torture/execute/ieee/pr109386.c: New test.
>
> --- gcc/range-op-float.cc.jj    2023-04-03 10:42:54.000000000 +0200
> +++ gcc/range-op-float.cc       2023-04-03 13:31:01.163216123 +0200
> @@ -889,7 +889,7 @@ foperator_lt::op1_range (frange &r,
>
>      case BRS_FALSE:
>        // On the FALSE side of x < NAN, we know nothing about x.
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else
>         build_ge (r, type, op2);
> @@ -926,7 +926,7 @@ foperator_lt::op2_range (frange &r,
>
>      case BRS_FALSE:
>        // On the FALSE side of NAN < x, we know nothing about x.
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else
>         build_le (r, type, op1);
> @@ -1005,7 +1005,7 @@ foperator_le::op1_range (frange &r,
>
>      case BRS_FALSE:
>        // On the FALSE side of x <= NAN, we know nothing about x.
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else
>         build_gt (r, type, op2);
> @@ -1038,7 +1038,7 @@ foperator_le::op2_range (frange &r,
>
>      case BRS_FALSE:
>        // On the FALSE side of NAN <= x, we know nothing about x.
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> @@ -1122,7 +1122,7 @@ foperator_gt::op1_range (frange &r,
>
>      case BRS_FALSE:
>        // On the FALSE side of x > NAN, we know nothing about x.
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else if (op2.undefined_p ())
>         return false;
> @@ -1161,7 +1161,7 @@ foperator_gt::op2_range (frange &r,
>
>      case BRS_FALSE:
>        // On The FALSE side of NAN > x, we know nothing about x.
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> @@ -1241,7 +1241,7 @@ foperator_ge::op1_range (frange &r,
>
>      case BRS_FALSE:
>        // On the FALSE side of x >= NAN, we know nothing about x.
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else if (op2.undefined_p ())
>         return false;
> @@ -1275,7 +1275,7 @@ foperator_ge::op2_range (frange &r, tree
>
>      case BRS_FALSE:
>        // On the FALSE side of NAN >= x, we know nothing about x.
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> @@ -1639,7 +1639,7 @@ foperator_unordered_lt::op1_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else if (op2.undefined_p ())
>         return false;
> @@ -1673,7 +1673,7 @@ foperator_unordered_lt::op2_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> @@ -1755,7 +1755,7 @@ foperator_unordered_le::op1_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else if (op2.undefined_p ())
>         return false;
> @@ -1788,7 +1788,7 @@ foperator_unordered_le::op2_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> @@ -1872,7 +1872,7 @@ foperator_unordered_gt::op1_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else if (op2.undefined_p ())
>         return false;
> @@ -1907,7 +1907,7 @@ foperator_unordered_gt::op2_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> @@ -1991,7 +1991,7 @@ foperator_unordered_ge::op1_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op2.known_isnan ())
> +      if (op2.known_isnan () || op2.maybe_isnan ())
>         r.set_varying (type);
>        else if (op2.undefined_p ())
>         return false;
> @@ -2025,7 +2025,7 @@ foperator_unordered_ge::op2_range (frang
>    switch (get_bool_state (r, lhs, type))
>      {
>      case BRS_TRUE:
> -      if (op1.known_isnan ())
> +      if (op1.known_isnan () || op1.maybe_isnan ())
>         r.set_varying (type);
>        else if (op1.undefined_p ())
>         return false;
> --- gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c.jj      2023-04-03
> 13:44:59.715981917 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/ieee/pr109386.c 2023-04-03
> 13:44:48.480145842 +0200
> @@ -0,0 +1,21 @@
> +/* PR tree-optimization/109386 */
> +
> +static inline float
> +foo (float x, float y)
> +{
> +  float u = __builtin_fabsf (x);
> +  float v = __builtin_fabsf (y);
> +  if (!(u >= v))
> +    {
> +      if (__builtin_isinf (v)) return v;
> +      if (__builtin_isinf (u)) return u;
> +    }
> +  return 42.0f;
> +}
> +
> +int
> +main ()
> +{
> +  if (!__builtin_isinf (foo (__builtin_inff (), __builtin_nanf (""))))
> +    __builtin_abort ();
> +}
>
>         Jakub
>
>