[PATCH] range-op-float, value-range: Fix up handling of UN{LT,LE,GT,GE,EQ}_EXPR and handle comparisons in get_tree_range [PR91645]

[PATCH] range-op-float, value-range: Fix up handling of UN{LT,LE,GT,GE,EQ}_EXPR and handle comparisons in get_tree_range [PR91645]

[Jakub Jelinek]

Hi!

When looking into PR91645, I've noticed we handle UN{LT,LE,GT,GE,EQ}_EXPR
comparisons incorrectly.
All those are unordered or ..., we correctly return [1, 1] if one or
both operands are known NANs, and correctly ask the non-UN prefixed
op to fold_range if neither operand may be NAN.
But for the case where one or both operands may be NAN, we always
return [0, 1].  The UN* fold_range tries to handle it by asking
the non-UN prefixed fold_range and if it returns [1, 1] return that,
if it returns [0, 0] or [0, 1] return [0, 1], which makes sense,
because the maybe NAN means that it is the non-UN prefixed fold_range
unioned with [1, 1] in case the maybe NAN is actually NAN at runtime.
The problem is that the non-UN prefixed fold_range always returns [0, 1]
because those fold_range implementations are like:
  if (op1.known_isnan () || op2.known_isnan ())
    r = range_false (type);
  else if (!maybe_isnan (op1, op2))
    {
...
    }
  else
    r = range_true_and_false (type);
and so if maybe_isnan, they always return [0, 1].  Now, thinking about it,
this is unnecessary pessimization, for the case where the ... block
returns range_false (type) we actually could do it also if maybe_isnan (op1,
op2), because if one or both operands are NAN, the comparison will be false,
and if neither is NAN, the comparison will be also false.  Will fix
incrementally today.
Anyway, the following patch fixes it by asking the non-UN prefixed
fold_range on ranges with NAN cleared, which I think does the right
thing in all cases.

Another change in the patch is that range_query::get_tree_range
always returned VARYING for comparisons, this patch allows to ask about
those as well (they are very much like binary ops, except they take
the important type from the types of the operands rather than result).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-03-31  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/91645
	* range-op-float.cc (foperator_unordered_lt::fold_range,
	foperator_unordered_le::fold_range,
	foperator_unordered_gt::fold_range,
	foperator_unordered_ge::fold_range,
	foperator_unordered_equal::fold_range): Call the ordered
	fold_range on ranges with cleared NaNs.
	* value-query.cc (range_query::get_tree_range): Handle also
	COMPARISON_CLASS_P trees.

--- gcc/range-op-float.cc.jj	2023-03-28 11:00:29.205986288 +0200
+++ gcc/range-op-float.cc	2023-03-30 15:14:12.219183066 +0200
@@ -1587,7 +1587,13 @@ public:
 	r = range_true (type);
 	return true;
       }
-    if (!fop_lt.fold_range (r, type, op1, op2, rel))
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_lt.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
       return false;
     // The result is the same as the ordered version when the
     // comparison is true or when the operands cannot be NANs.
@@ -1692,7 +1698,13 @@ public:
 	r = range_true (type);
 	return true;
       }
-    if (!fop_le.fold_range (r, type, op1, op2, rel))
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_le.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
       return false;
     // The result is the same as the ordered version when the
     // comparison is true or when the operands cannot be NANs.
@@ -1793,7 +1805,13 @@ public:
 	r = range_true (type);
 	return true;
       }
-    if (!fop_gt.fold_range (r, type, op1, op2, rel))
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_gt.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
       return false;
     // The result is the same as the ordered version when the
     // comparison is true or when the operands cannot be NANs.
@@ -1898,7 +1916,13 @@ public:
 	r = range_true (type);
 	return true;
       }
-    if (!fop_ge.fold_range (r, type, op1, op2, rel))
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_ge.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
       return false;
     // The result is the same as the ordered version when the
     // comparison is true or when the operands cannot be NANs.
@@ -2002,7 +2026,13 @@ public:
 	r = range_true (type);
 	return true;
       }
-    if (!fop_equal.fold_range (r, type, op1, op2, rel))
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_equal.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
       return false;
     // The result is the same as the ordered version when the
     // comparison is true or when the operands cannot be NANs.
--- gcc/value-query.cc.jj	2023-03-23 15:25:47.069740988 +0100
+++ gcc/value-query.cc	2023-03-30 15:20:58.733237311 +0200
@@ -230,15 +230,21 @@ range_query::get_tree_range (vrange &r,
     default:
       break;
     }
-  if (BINARY_CLASS_P (expr))
+  if (BINARY_CLASS_P (expr) || COMPARISON_CLASS_P (expr))
     {
-      range_op_handler op (TREE_CODE (expr), type);
+      tree op0 = TREE_OPERAND (expr, 0);
+      tree op1 = TREE_OPERAND (expr, 1);
+      if (COMPARISON_CLASS_P (expr)
+	  && !Value_Range::supports_type_p (TREE_TYPE (op0)))
+	return false;
+      range_op_handler op (TREE_CODE (expr),
+			   BINARY_CLASS_P (expr) ? type : TREE_TYPE (op0));
       if (op)
 	{
-	  Value_Range r0 (TREE_TYPE (TREE_OPERAND (expr, 0)));
-	  Value_Range r1 (TREE_TYPE (TREE_OPERAND (expr, 1)));
-	  range_of_expr (r0, TREE_OPERAND (expr, 0), stmt);
-	  range_of_expr (r1, TREE_OPERAND (expr, 1), stmt);
+	  Value_Range r0 (TREE_TYPE (op0));
+	  Value_Range r1 (TREE_TYPE (op1));
+	  range_of_expr (r0, op0, stmt);
+	  range_of_expr (r1, op1, stmt);
 	  if (!op.fold_range (r, type, r0, r1))
 	    r.set_varying (type);
 	}

	Jakub

[PATCH] range-op-float: Further comparison fixes

[Jakub Jelinek]

On Fri, Mar 31, 2023 at 09:57:54AM +0200, Jakub Jelinek via Gcc-patches wrote:
> and so if maybe_isnan, they always return [0, 1].  Now, thinking about it,
> this is unnecessary pessimization, for the case where the ... block
> returns range_false (type) we actually could do it also if maybe_isnan (op1,
> op2), because if one or both operands are NAN, the comparison will be false,
> and if neither is NAN, the comparison will be also false.  Will fix
> incrementally today.

Here it is.
1) the foperator_{,not_}equal::fold_range cases
   - we have correctly first a case handling known_isnan on either operand,
     return there range_false (type) - EQ, resp. range_true (type) - NE
   - next we handle the singleton cases, maybe_isnan + singleton isn't
     singleton, so these are ok
   - there is a missing case (not handled in this patch) where both operands
     are known to be zeros, but not singleton zeros
   - there is some !maybe_isnan (op1, op2) handling which tries to prove
     when the operands are certainly not equal and results in
     range_false (type) - EQ, resp. range_true (type) - NE
   - otherwise range_true_and_false (type)
   Now, I think (and this patch implements it) that the !maybe_isnan (op1, op2)
   check is unnecessary.  If we prove that when ignoring maybe NANs, the
   ranges don't intersect and so the comparison is always false for EQ or
   always true for NE, if NANs can appear, it doesn't change anything on
   that either, for EQ if NAN appears, the result is false like what we
   proved for the finite ranges, for NE if NAN appears, the result is true
   like what we proved for the finite ranges
2) foperator_{lt,le,gt,ge}::fold_range cases
   - these have correctly known_isnan on either operand first and return
     there range_false (type)
   - then !maybe_isnan (op1, op2) condition guarded checks
   - finally range_true_and_false (type) - so do that for
     maybe_isnan (op1, op2)
   Here in the !maybe_isnan (op1, op2) guarded code we have some condition
   which results in range_true (type), another condition which results in
   range_false (type) and otherwise range_true_and_false (type).
   Now, the condition which results in range_false (type) can be IMHO done
   also for the maybe_isnan (op1, op2) cases, because it is [0, 0]
   if both operands are finite or [0, 0] if either operand is NAN.
3) finally, LTGT_EXPR wasn't handled at all.  LTGT_EXPR is the inverse
   comparision to UNEQ_EXPR, I believe both raise exceptions only if
   either operand is sNaN, UNEQ_EXPR is true if both operands are
   either equal or either of the operands is NAN, while LTGT_EXPR is
   true if the operands compare either smaller or larger and is false if
   either of the operands is NAN.

Ok for trunk if this passes bootstrap/regtest?

2023-03-31  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (foperator_equal::fold_range): Perform the
	non-singleton handling regardless of maybe_isnan (op1, op2).
	(foperator_not_equal::fold_range): Likewise.
	(foperator_lt::fold_range, foperator_le::fold_range,
	foperator_gt::fold_range, foperator_ge::fold_range): Perform the
	real_* comparison check which results in range_false (type)
	even if maybe_isnan (op1, op2).  Simplify.
	(foperator_ltgt): New class.
	(fop_ltgt): New variable.
	(floating_op_table::floating_op_table): Handle LTGT_EXPR using
	fop_ltgt.

--- gcc/range-op-float.cc.jj	2023-03-31 09:30:11.245296618 +0200
+++ gcc/range-op-float.cc	2023-03-31 11:23:04.817876083 +0200
@@ -616,7 +616,7 @@ foperator_equal::fold_range (irange &r,
       else
 	r = range_false (type);
     }
-  else if (!maybe_isnan (op1, op2))
+  else
     {
       // If ranges do not intersect, we know the range is not equal,
       // otherwise we don't know anything for sure.
@@ -638,8 +638,6 @@ foperator_equal::fold_range (irange &r,
       else
 	r = range_true_and_false (type);
     }
-  else
-    r = range_true_and_false (type);
   return true;
 }
 
@@ -734,7 +732,7 @@ foperator_not_equal::fold_range (irange
       else
 	r = range_true (type);
     }
-  else if (!maybe_isnan (op1, op2))
+  else
     {
       // If ranges do not intersect, we know the range is not equal,
       // otherwise we don't know anything for sure.
@@ -756,8 +754,6 @@ foperator_not_equal::fold_range (irange
       else
 	r = range_true_and_false (type);
     }
-  else
-    r = range_true_and_false (type);
   return true;
 }
 
@@ -839,17 +835,13 @@ foperator_lt::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_LT))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_less (&op1.lower_bound (), &op2.upper_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_less (&op1.upper_bound (), &op2.lower_bound ()))
-	r = range_true (type);
-      else if (!real_less (&op1.lower_bound (), &op2.upper_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_less (&op1.upper_bound (), &op2.lower_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -959,17 +951,13 @@ foperator_le::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_LE))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
-	r = range_true (type);
-      else if (!real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -1073,17 +1061,13 @@ foperator_gt::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_GT))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_compare (GT_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_compare (GT_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
-	r = range_true (type);
-      else if (!real_compare (GT_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_compare (GT_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -1197,17 +1181,13 @@ foperator_ge::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_GE))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_compare (GE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_compare (GE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
-	r = range_true (type);
-      else if (!real_compare (GE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_compare (GE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -2092,6 +2072,87 @@ foperator_unordered_equal::op1_range (fr
   return true;
 }
 
+class foperator_ltgt : public range_operator_float
+{
+  using range_operator_float::fold_range;
+  using range_operator_float::op1_range;
+  using range_operator_float::op2_range;
+public:
+  bool fold_range (irange &r, tree type,
+		   const frange &op1, const frange &op2,
+		   relation_trio rel = TRIO_VARYING) const final override
+  {
+    if (op1.known_isnan () || op2.known_isnan ())
+      {
+	r = range_false (type);
+	return true;
+      }
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_not_equal.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
+      return false;
+    // The result is the same as the ordered version when the
+    // comparison is true or when the operands cannot be NANs.
+    if (!maybe_isnan (op1, op2) || r == range_false (type))
+      return true;
+    else
+      {
+	r = range_true_and_false (type);
+	return true;
+      }
+  }
+  bool op1_range (frange &r, tree type,
+		  const irange &lhs, const frange &op2,
+		  relation_trio = TRIO_VARYING) const final override;
+  bool op2_range (frange &r, tree type,
+		  const irange &lhs, const frange &op1,
+		  relation_trio rel = TRIO_VARYING) const final override
+  {
+    return op1_range (r, type, lhs, op1, rel.swap_op1_op2 ());
+  }
+} fop_ltgt;
+
+bool
+foperator_ltgt::op1_range (frange &r, tree type,
+			   const irange &lhs,
+			   const frange &op2,
+			   relation_trio) const
+{
+  switch (get_bool_state (r, lhs, type))
+    {
+    case BRS_TRUE:
+      // A true LTGT means both operands are !NAN, so it's
+      // impossible for op2 to be a NAN.
+      if (op2.known_isnan ())
+	r.set_undefined ();
+      else
+	{
+	  // The true side indicates !NAN and not equal.  We can at least
+	  // represent !NAN.
+	  r.set_varying (type);
+	  r.clear_nan ();
+	}
+      break;
+
+    case BRS_FALSE:
+      // If it's false, the result is the same as OP2 plus a NAN.
+      r = op2;
+      // Add both zeros if there's the possibility of zero equality.
+      frange_add_zeros (r, type);
+      // Add the possibility of a NAN.
+      r.update_nan ();
+      break;
+
+    default:
+      break;
+    }
+  return true;
+}
+
 // Final tweaks for float binary op op1_range/op2_range.
 // Return TRUE if the operation is performed and a valid range is available.
 
@@ -2767,6 +2828,7 @@ floating_op_table::floating_op_table ()
   set (UNEQ_EXPR, fop_unordered_equal);
   set (ORDERED_EXPR, fop_ordered);
   set (UNORDERED_EXPR, fop_unordered);
+  set (LTGT_EXPR, fop_ltgt);
 
   set (ABS_EXPR, fop_abs);
   set (NEGATE_EXPR, fop_negate);


	Jakub

[PATCH] range-op-float: Further foperator_{,not_}equal::fold_range fix

[Jakub Jelinek]

On Fri, Mar 31, 2023 at 12:45:10PM +0200, Jakub Jelinek via Gcc-patches wrote:
>    - there is a missing case (not handled in this patch) where both operands
>      are known to be zeros, but not singleton zeros

This patch adds those cases.

Ok for trunk if it passes bootstrap/regtest?

2023-03-31  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (foperator_equal::fold_range): If at least
	one of the op ranges is not singleton and neither is NaN and all
	4 bounds are zero, return [1, 1].
	(foperator_not_equal::fold_range): In the same case return [0, 0].

--- gcc/range-op-float.cc.jj	2023-03-31 11:23:04.817876083 +0200
+++ gcc/range-op-float.cc	2023-03-31 12:51:34.757480162 +0200
@@ -616,6 +616,13 @@ foperator_equal::fold_range (irange &r,
       else
 	r = range_false (type);
     }
+  else if (real_iszero (&op1.lower_bound ())
+	   && real_iszero (&op1.upper_bound ())
+	   && real_iszero (&op2.lower_bound ())
+	   && real_iszero (&op2.upper_bound ())
+	   && !maybe_isnan (op1, op2))
+    // [-0.0, 0.0] == [-0.0, 0.0] or similar.
+    r = range_true (type);
   else
     {
       // If ranges do not intersect, we know the range is not equal,
@@ -732,6 +739,13 @@ foperator_not_equal::fold_range (irange
       else
 	r = range_true (type);
     }
+  else if (real_iszero (&op1.lower_bound ())
+	   && real_iszero (&op1.upper_bound ())
+	   && real_iszero (&op2.lower_bound ())
+	   && real_iszero (&op2.upper_bound ())
+	   && !maybe_isnan (op1, op2))
+    // [-0.0, 0.0] != [-0.0, 0.0] or similar.
+    r = range_false (type);
   else
     {
       // If ranges do not intersect, we know the range is not equal,


	Jakub

Re: [PATCH] range-op-float: Further foperator_{,not_}equal::fold_range fix

[Aldy Hernandez]

On 3/31/23 12:57, Jakub Jelinek wrote:
> On Fri, Mar 31, 2023 at 12:45:10PM +0200, Jakub Jelinek via Gcc-patches wrote:
>>     - there is a missing case (not handled in this patch) where both operands
>>       are known to be zeros, but not singleton zeros
> 
> This patch adds those cases.
> 
> Ok for trunk if it passes bootstrap/regtest?

LGTM.

Thanks so much for taking care of all this.
Aldy

> 
> 2023-03-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (foperator_equal::fold_range): If at least
> 	one of the op ranges is not singleton and neither is NaN and all
> 	4 bounds are zero, return [1, 1].
> 	(foperator_not_equal::fold_range): In the same case return [0, 0].
> 
> --- gcc/range-op-float.cc.jj	2023-03-31 11:23:04.817876083 +0200
> +++ gcc/range-op-float.cc	2023-03-31 12:51:34.757480162 +0200
> @@ -616,6 +616,13 @@ foperator_equal::fold_range (irange &r,
>         else
>   	r = range_false (type);
>       }
> +  else if (real_iszero (&op1.lower_bound ())
> +	   && real_iszero (&op1.upper_bound ())
> +	   && real_iszero (&op2.lower_bound ())
> +	   && real_iszero (&op2.upper_bound ())
> +	   && !maybe_isnan (op1, op2))
> +    // [-0.0, 0.0] == [-0.0, 0.0] or similar.
> +    r = range_true (type);
>     else
>       {
>         // If ranges do not intersect, we know the range is not equal,
> @@ -732,6 +739,13 @@ foperator_not_equal::fold_range (irange
>         else
>   	r = range_true (type);
>       }
> +  else if (real_iszero (&op1.lower_bound ())
> +	   && real_iszero (&op1.upper_bound ())
> +	   && real_iszero (&op2.lower_bound ())
> +	   && real_iszero (&op2.upper_bound ())
> +	   && !maybe_isnan (op1, op2))
> +    // [-0.0, 0.0] != [-0.0, 0.0] or similar.
> +    r = range_false (type);
>     else
>       {
>         // If ranges do not intersect, we know the range is not equal,
> 
> 
> 	Jakub
> 

Re: [PATCH] range-op-float: Further comparison fixes

[Aldy Hernandez]

On 3/31/23 12:45, Jakub Jelinek wrote:
> On Fri, Mar 31, 2023 at 09:57:54AM +0200, Jakub Jelinek via Gcc-patches wrote:
>> and so if maybe_isnan, they always return [0, 1].  Now, thinking about it,
>> this is unnecessary pessimization, for the case where the ... block
>> returns range_false (type) we actually could do it also if maybe_isnan (op1,
>> op2), because if one or both operands are NAN, the comparison will be false,
>> and if neither is NAN, the comparison will be also false.  Will fix
>> incrementally today.
> 
> Here it is.
> 1) the foperator_{,not_}equal::fold_range cases
>     - we have correctly first a case handling known_isnan on either operand,
>       return there range_false (type) - EQ, resp. range_true (type) - NE
>     - next we handle the singleton cases, maybe_isnan + singleton isn't
>       singleton, so these are ok
>     - there is a missing case (not handled in this patch) where both operands
>       are known to be zeros, but not singleton zeros
>     - there is some !maybe_isnan (op1, op2) handling which tries to prove
>       when the operands are certainly not equal and results in
>       range_false (type) - EQ, resp. range_true (type) - NE
>     - otherwise range_true_and_false (type)
>     Now, I think (and this patch implements it) that the !maybe_isnan (op1, op2)
>     check is unnecessary.  If we prove that when ignoring maybe NANs, the
>     ranges don't intersect and so the comparison is always false for EQ or
>     always true for NE, if NANs can appear, it doesn't change anything on
>     that either, for EQ if NAN appears, the result is false like what we
>     proved for the finite ranges, for NE if NAN appears, the result is true
>     like what we proved for the finite ranges
> 2) foperator_{lt,le,gt,ge}::fold_range cases
>     - these have correctly known_isnan on either operand first and return
>       there range_false (type)
>     - then !maybe_isnan (op1, op2) condition guarded checks
>     - finally range_true_and_false (type) - so do that for
>       maybe_isnan (op1, op2)
>     Here in the !maybe_isnan (op1, op2) guarded code we have some condition
>     which results in range_true (type), another condition which results in
>     range_false (type) and otherwise range_true_and_false (type).
>     Now, the condition which results in range_false (type) can be IMHO done
>     also for the maybe_isnan (op1, op2) cases, because it is [0, 0]
>     if both operands are finite or [0, 0] if either operand is NAN.
> 3) finally, LTGT_EXPR wasn't handled at all.  LTGT_EXPR is the inverse
>     comparision to UNEQ_EXPR, I believe both raise exceptions only if
>     either operand is sNaN, UNEQ_EXPR is true if both operands are
>     either equal or either of the operands is NAN, while LTGT_EXPR is
>     true if the operands compare either smaller or larger and is false if
>     either of the operands is NAN.
> 
> Ok for trunk if this passes bootstrap/regtest?

LGTM
Aldy

> 
> 2023-03-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (foperator_equal::fold_range): Perform the
> 	non-singleton handling regardless of maybe_isnan (op1, op2).
> 	(foperator_not_equal::fold_range): Likewise.
> 	(foperator_lt::fold_range, foperator_le::fold_range,
> 	foperator_gt::fold_range, foperator_ge::fold_range): Perform the
> 	real_* comparison check which results in range_false (type)
> 	even if maybe_isnan (op1, op2).  Simplify.
> 	(foperator_ltgt): New class.
> 	(fop_ltgt): New variable.
> 	(floating_op_table::floating_op_table): Handle LTGT_EXPR using
> 	fop_ltgt.
> 
> --- gcc/range-op-float.cc.jj	2023-03-31 09:30:11.245296618 +0200
> +++ gcc/range-op-float.cc	2023-03-31 11:23:04.817876083 +0200
> @@ -616,7 +616,7 @@ foperator_equal::fold_range (irange &r,
>         else
>   	r = range_false (type);
>       }
> -  else if (!maybe_isnan (op1, op2))
> +  else
>       {
>         // If ranges do not intersect, we know the range is not equal,
>         // otherwise we don't know anything for sure.
> @@ -638,8 +638,6 @@ foperator_equal::fold_range (irange &r,
>         else
>   	r = range_true_and_false (type);
>       }
> -  else
> -    r = range_true_and_false (type);
>     return true;
>   }
>   
> @@ -734,7 +732,7 @@ foperator_not_equal::fold_range (irange
>         else
>   	r = range_true (type);
>       }
> -  else if (!maybe_isnan (op1, op2))
> +  else
>       {
>         // If ranges do not intersect, we know the range is not equal,
>         // otherwise we don't know anything for sure.
> @@ -756,8 +754,6 @@ foperator_not_equal::fold_range (irange
>         else
>   	r = range_true_and_false (type);
>       }
> -  else
> -    r = range_true_and_false (type);
>     return true;
>   }
>   
> @@ -839,17 +835,13 @@ foperator_lt::fold_range (irange &r, tre
>     if (frelop_early_resolve (r, type, op1, op2, rel, VREL_LT))
>       return true;
>   
> -  if (op1.known_isnan () || op2.known_isnan ())
> +  if (op1.known_isnan ()
> +      || op2.known_isnan ()
> +      || !real_less (&op1.lower_bound (), &op2.upper_bound ()))
>       r = range_false (type);
> -  else if (!maybe_isnan (op1, op2))
> -    {
> -      if (real_less (&op1.upper_bound (), &op2.lower_bound ()))
> -	r = range_true (type);
> -      else if (!real_less (&op1.lower_bound (), &op2.upper_bound ()))
> -	r = range_false (type);
> -      else
> -	r = range_true_and_false (type);
> -    }
> +  else if (!maybe_isnan (op1, op2)
> +	   && real_less (&op1.upper_bound (), &op2.lower_bound ()))
> +    r = range_true (type);
>     else
>       r = range_true_and_false (type);
>     return true;
> @@ -959,17 +951,13 @@ foperator_le::fold_range (irange &r, tre
>     if (frelop_early_resolve (r, type, op1, op2, rel, VREL_LE))
>       return true;
>   
> -  if (op1.known_isnan () || op2.known_isnan ())
> +  if (op1.known_isnan ()
> +      || op2.known_isnan ()
> +      || !real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
>       r = range_false (type);
> -  else if (!maybe_isnan (op1, op2))
> -    {
> -      if (real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
> -	r = range_true (type);
> -      else if (!real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
> -	r = range_false (type);
> -      else
> -	r = range_true_and_false (type);
> -    }
> +  else if (!maybe_isnan (op1, op2)
> +	   && real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
> +    r = range_true (type);
>     else
>       r = range_true_and_false (type);
>     return true;
> @@ -1073,17 +1061,13 @@ foperator_gt::fold_range (irange &r, tre
>     if (frelop_early_resolve (r, type, op1, op2, rel, VREL_GT))
>       return true;
>   
> -  if (op1.known_isnan () || op2.known_isnan ())
> +  if (op1.known_isnan ()
> +      || op2.known_isnan ()
> +      || !real_compare (GT_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
>       r = range_false (type);
> -  else if (!maybe_isnan (op1, op2))
> -    {
> -      if (real_compare (GT_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
> -	r = range_true (type);
> -      else if (!real_compare (GT_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
> -	r = range_false (type);
> -      else
> -	r = range_true_and_false (type);
> -    }
> +  else if (!maybe_isnan (op1, op2)
> +	   && real_compare (GT_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
> +    r = range_true (type);
>     else
>       r = range_true_and_false (type);
>     return true;
> @@ -1197,17 +1181,13 @@ foperator_ge::fold_range (irange &r, tre
>     if (frelop_early_resolve (r, type, op1, op2, rel, VREL_GE))
>       return true;
>   
> -  if (op1.known_isnan () || op2.known_isnan ())
> +  if (op1.known_isnan ()
> +      || op2.known_isnan ()
> +      || !real_compare (GE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
>       r = range_false (type);
> -  else if (!maybe_isnan (op1, op2))
> -    {
> -      if (real_compare (GE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
> -	r = range_true (type);
> -      else if (!real_compare (GE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
> -	r = range_false (type);
> -      else
> -	r = range_true_and_false (type);
> -    }
> +  else if (!maybe_isnan (op1, op2)
> +	   && real_compare (GE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
> +    r = range_true (type);
>     else
>       r = range_true_and_false (type);
>     return true;
> @@ -2092,6 +2072,87 @@ foperator_unordered_equal::op1_range (fr
>     return true;
>   }
>   
> +class foperator_ltgt : public range_operator_float
> +{
> +  using range_operator_float::fold_range;
> +  using range_operator_float::op1_range;
> +  using range_operator_float::op2_range;
> +public:
> +  bool fold_range (irange &r, tree type,
> +		   const frange &op1, const frange &op2,
> +		   relation_trio rel = TRIO_VARYING) const final override
> +  {
> +    if (op1.known_isnan () || op2.known_isnan ())
> +      {
> +	r = range_false (type);
> +	return true;
> +      }
> +    frange op1_no_nan = op1;
> +    frange op2_no_nan = op2;
> +    if (op1.maybe_isnan ())
> +      op1_no_nan.clear_nan ();
> +    if (op2.maybe_isnan ())
> +      op2_no_nan.clear_nan ();
> +    if (!fop_not_equal.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
> +      return false;
> +    // The result is the same as the ordered version when the
> +    // comparison is true or when the operands cannot be NANs.
> +    if (!maybe_isnan (op1, op2) || r == range_false (type))
> +      return true;
> +    else
> +      {
> +	r = range_true_and_false (type);
> +	return true;
> +      }
> +  }
> +  bool op1_range (frange &r, tree type,
> +		  const irange &lhs, const frange &op2,
> +		  relation_trio = TRIO_VARYING) const final override;
> +  bool op2_range (frange &r, tree type,
> +		  const irange &lhs, const frange &op1,
> +		  relation_trio rel = TRIO_VARYING) const final override
> +  {
> +    return op1_range (r, type, lhs, op1, rel.swap_op1_op2 ());
> +  }
> +} fop_ltgt;
> +
> +bool
> +foperator_ltgt::op1_range (frange &r, tree type,
> +			   const irange &lhs,
> +			   const frange &op2,
> +			   relation_trio) const
> +{
> +  switch (get_bool_state (r, lhs, type))
> +    {
> +    case BRS_TRUE:
> +      // A true LTGT means both operands are !NAN, so it's
> +      // impossible for op2 to be a NAN.
> +      if (op2.known_isnan ())
> +	r.set_undefined ();
> +      else
> +	{
> +	  // The true side indicates !NAN and not equal.  We can at least
> +	  // represent !NAN.
> +	  r.set_varying (type);
> +	  r.clear_nan ();
> +	}
> +      break;
> +
> +    case BRS_FALSE:
> +      // If it's false, the result is the same as OP2 plus a NAN.
> +      r = op2;
> +      // Add both zeros if there's the possibility of zero equality.
> +      frange_add_zeros (r, type);
> +      // Add the possibility of a NAN.
> +      r.update_nan ();
> +      break;
> +
> +    default:
> +      break;
> +    }
> +  return true;
> +}
> +
>   // Final tweaks for float binary op op1_range/op2_range.
>   // Return TRUE if the operation is performed and a valid range is available.
>   
> @@ -2767,6 +2828,7 @@ floating_op_table::floating_op_table ()
>     set (UNEQ_EXPR, fop_unordered_equal);
>     set (ORDERED_EXPR, fop_ordered);
>     set (UNORDERED_EXPR, fop_unordered);
> +  set (LTGT_EXPR, fop_ltgt);
>   
>     set (ABS_EXPR, fop_abs);
>     set (NEGATE_EXPR, fop_negate);
> 
> 
> 	Jakub
> 

Re: [PATCH] range-op-float: Further comparison fixes

[Jakub Jelinek]

On Fri, Mar 31, 2023 at 01:28:35PM +0200, Aldy Hernandez wrote:
> > Ok for trunk if this passes bootstrap/regtest?
> 
> LGTM

Unfortunately I ran into 4 tests where we run into the known bug
where if ranger finds out a range of some floating point operation
it folds it and throws away the trapping side-effects with it.
In particular, e.g. VARYING > __builtin_inf () is now folded to false
because no finite or infinite value is larger than +inf, and if
NAN is compared against it, the result is false as well.

Given that it is a known problem we need to find some solution for
in GCC 14, I've just changed those testcases to disable dom and vrp
passes which were optimizing it.  Another workaround would be
to hide the infinity from the optimizers, but given that the test was
added exactly to verify match.pd doesn't optimize this for -ftrapping-math,
I think disabling dom/vrp is better.

So, here is what I've committed (the other patch committed unchanged):

2023-04-01  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (foperator_equal::fold_range): Perform the
	non-singleton handling regardless of maybe_isnan (op1, op2).
	(foperator_not_equal::fold_range): Likewise.
	(foperator_lt::fold_range, foperator_le::fold_range,
	foperator_gt::fold_range, foperator_ge::fold_range): Perform the
	real_* comparison check which results in range_false (type)
	even if maybe_isnan (op1, op2).  Simplify.
	(foperator_ltgt): New class.
	(fop_ltgt): New variable.
	(floating_op_table::floating_op_table): Handle LTGT_EXPR using
	fop_ltgt.

	* gcc.dg/torture/inf-compare-1.c: Add dg-additional-options
	-fno-tree-dominator-opts -fno-tree-vrp.
	* gcc.dg/torture/inf-compare-1-float.c: Likewise.
	* gcc.dg/torture/inf-compare-2.c: Likewise.
	* gcc.dg/torture/inf-compare-2-float.c: Likewise.

--- gcc/range-op-float.cc.jj	2023-03-31 09:30:11.245296618 +0200
+++ gcc/range-op-float.cc	2023-03-31 11:23:04.817876083 +0200
@@ -616,7 +616,7 @@ foperator_equal::fold_range (irange &r,
       else
 	r = range_false (type);
     }
-  else if (!maybe_isnan (op1, op2))
+  else
     {
       // If ranges do not intersect, we know the range is not equal,
       // otherwise we don't know anything for sure.
@@ -638,8 +638,6 @@ foperator_equal::fold_range (irange &r,
       else
 	r = range_true_and_false (type);
     }
-  else
-    r = range_true_and_false (type);
   return true;
 }
 
@@ -734,7 +732,7 @@ foperator_not_equal::fold_range (irange
       else
 	r = range_true (type);
     }
-  else if (!maybe_isnan (op1, op2))
+  else
     {
       // If ranges do not intersect, we know the range is not equal,
       // otherwise we don't know anything for sure.
@@ -756,8 +754,6 @@ foperator_not_equal::fold_range (irange
       else
 	r = range_true_and_false (type);
     }
-  else
-    r = range_true_and_false (type);
   return true;
 }
 
@@ -839,17 +835,13 @@ foperator_lt::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_LT))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_less (&op1.lower_bound (), &op2.upper_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_less (&op1.upper_bound (), &op2.lower_bound ()))
-	r = range_true (type);
-      else if (!real_less (&op1.lower_bound (), &op2.upper_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_less (&op1.upper_bound (), &op2.lower_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -959,17 +951,13 @@ foperator_le::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_LE))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
-	r = range_true (type);
-      else if (!real_compare (LE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_compare (LE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -1073,17 +1061,13 @@ foperator_gt::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_GT))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_compare (GT_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_compare (GT_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
-	r = range_true (type);
-      else if (!real_compare (GT_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_compare (GT_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -1197,17 +1181,13 @@ foperator_ge::fold_range (irange &r, tre
   if (frelop_early_resolve (r, type, op1, op2, rel, VREL_GE))
     return true;
 
-  if (op1.known_isnan () || op2.known_isnan ())
+  if (op1.known_isnan ()
+      || op2.known_isnan ()
+      || !real_compare (GE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
     r = range_false (type);
-  else if (!maybe_isnan (op1, op2))
-    {
-      if (real_compare (GE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
-	r = range_true (type);
-      else if (!real_compare (GE_EXPR, &op1.upper_bound (), &op2.lower_bound ()))
-	r = range_false (type);
-      else
-	r = range_true_and_false (type);
-    }
+  else if (!maybe_isnan (op1, op2)
+	   && real_compare (GE_EXPR, &op1.lower_bound (), &op2.upper_bound ()))
+    r = range_true (type);
   else
     r = range_true_and_false (type);
   return true;
@@ -2092,6 +2072,87 @@ foperator_unordered_equal::op1_range (fr
   return true;
 }
 
+class foperator_ltgt : public range_operator_float
+{
+  using range_operator_float::fold_range;
+  using range_operator_float::op1_range;
+  using range_operator_float::op2_range;
+public:
+  bool fold_range (irange &r, tree type,
+		   const frange &op1, const frange &op2,
+		   relation_trio rel = TRIO_VARYING) const final override
+  {
+    if (op1.known_isnan () || op2.known_isnan ())
+      {
+	r = range_false (type);
+	return true;
+      }
+    frange op1_no_nan = op1;
+    frange op2_no_nan = op2;
+    if (op1.maybe_isnan ())
+      op1_no_nan.clear_nan ();
+    if (op2.maybe_isnan ())
+      op2_no_nan.clear_nan ();
+    if (!fop_not_equal.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
+      return false;
+    // The result is the same as the ordered version when the
+    // comparison is true or when the operands cannot be NANs.
+    if (!maybe_isnan (op1, op2) || r == range_false (type))
+      return true;
+    else
+      {
+	r = range_true_and_false (type);
+	return true;
+      }
+  }
+  bool op1_range (frange &r, tree type,
+		  const irange &lhs, const frange &op2,
+		  relation_trio = TRIO_VARYING) const final override;
+  bool op2_range (frange &r, tree type,
+		  const irange &lhs, const frange &op1,
+		  relation_trio rel = TRIO_VARYING) const final override
+  {
+    return op1_range (r, type, lhs, op1, rel.swap_op1_op2 ());
+  }
+} fop_ltgt;
+
+bool
+foperator_ltgt::op1_range (frange &r, tree type,
+			   const irange &lhs,
+			   const frange &op2,
+			   relation_trio) const
+{
+  switch (get_bool_state (r, lhs, type))
+    {
+    case BRS_TRUE:
+      // A true LTGT means both operands are !NAN, so it's
+      // impossible for op2 to be a NAN.
+      if (op2.known_isnan ())
+	r.set_undefined ();
+      else
+	{
+	  // The true side indicates !NAN and not equal.  We can at least
+	  // represent !NAN.
+	  r.set_varying (type);
+	  r.clear_nan ();
+	}
+      break;
+
+    case BRS_FALSE:
+      // If it's false, the result is the same as OP2 plus a NAN.
+      r = op2;
+      // Add both zeros if there's the possibility of zero equality.
+      frange_add_zeros (r, type);
+      // Add the possibility of a NAN.
+      r.update_nan ();
+      break;
+
+    default:
+      break;
+    }
+  return true;
+}
+
 // Final tweaks for float binary op op1_range/op2_range.
 // Return TRUE if the operation is performed and a valid range is available.
 
@@ -2767,6 +2828,7 @@ floating_op_table::floating_op_table ()
   set (UNEQ_EXPR, fop_unordered_equal);
   set (ORDERED_EXPR, fop_ordered);
   set (UNORDERED_EXPR, fop_unordered);
+  set (LTGT_EXPR, fop_ltgt);
 
   set (ABS_EXPR, fop_abs);
   set (NEGATE_EXPR, fop_negate);
--- gcc/testsuite/gcc.dg/torture/inf-compare-1.c.jj	2022-10-21 08:56:19.330180242 +0200
+++ gcc/testsuite/gcc.dg/torture/inf-compare-1.c	2023-04-01 09:26:24.385274181 +0200
@@ -3,6 +3,7 @@
 /* { dg-add-options ieee } */
 /* { dg-require-effective-target fenv_exceptions_double } */
 /* { dg-skip-if "fenv" { powerpc-ibm-aix* } } */
+/* { dg-additional-options "-fno-tree-dominator-opts -fno-tree-vrp" } */
 
 #include <fenv.h>
 
--- gcc/testsuite/gcc.dg/torture/inf-compare-1-float.c.jj	2022-10-21 08:56:19.330180242 +0200
+++ gcc/testsuite/gcc.dg/torture/inf-compare-1-float.c	2023-04-01 09:26:15.380403321 +0200
@@ -3,6 +3,7 @@
 /* { dg-add-options ieee } */
 /* { dg-require-effective-target fenv_exceptions } */
 /* { dg-skip-if "fenv" { powerpc-ibm-aix* } } */
+/* { dg-additional-options "-fno-tree-dominator-opts -fno-tree-vrp" } */
 
 #include <fenv.h>
 
--- gcc/testsuite/gcc.dg/torture/inf-compare-2.c.jj	2022-10-21 08:56:19.330180242 +0200
+++ gcc/testsuite/gcc.dg/torture/inf-compare-2.c	2023-04-01 09:26:43.555999258 +0200
@@ -3,6 +3,7 @@
 /* { dg-add-options ieee } */
 /* { dg-require-effective-target fenv_exceptions_double } */
 /* { dg-skip-if "fenv" { powerpc-ibm-aix* } } */
+/* { dg-additional-options "-fno-tree-dominator-opts -fno-tree-vrp" } */
 
 #include <fenv.h>
 
--- gcc/testsuite/gcc.dg/torture/inf-compare-2-float.c.jj	2022-10-21 08:56:19.330180242 +0200
+++ gcc/testsuite/gcc.dg/torture/inf-compare-2-float.c	2023-04-01 09:26:32.682155200 +0200
@@ -3,6 +3,7 @@
 /* { dg-add-options ieee } */
 /* { dg-require-effective-target fenv_exceptions } */
 /* { dg-skip-if "fenv" { powerpc-ibm-aix* } } */
+/* { dg-additional-options "-fno-tree-dominator-opts -fno-tree-vrp" } */
 
 #include <fenv.h>
 


	Jakub

Re: [PATCH] range-op-float: Further comparison fixes

[Aldy Hernandez]

On 4/1/23 09:39, Jakub Jelinek wrote:
> On Fri, Mar 31, 2023 at 01:28:35PM +0200, Aldy Hernandez wrote:
>>> Ok for trunk if this passes bootstrap/regtest?
>>
>> LGTM
> 
> Unfortunately I ran into 4 tests where we run into the known bug
> where if ranger finds out a range of some floating point operation
> it folds it and throws away the trapping side-effects with it.

Is VRP folding it, or is some other pass using ranger making a decision 
based on INF > INF being false?  Cause we'd ultimately like to keep 
ranger honest...that is returning FALSE for the above, so the onus is on 
the ranger callers on making correct decisions.

IMO.
Aldy

Re: [PATCH] range-op-float, value-range: Fix up handling of UN{LT,LE,GT,GE,EQ}_EXPR and handle comparisons in get_tree_range [PR91645]

[Aldy Hernandez]

On 3/31/23 09:57, Jakub Jelinek wrote:
> Hi!
> 
> When looking into PR91645, I've noticed we handle UN{LT,LE,GT,GE,EQ}_EXPR
> comparisons incorrectly.
> All those are unordered or ..., we correctly return [1, 1] if one or
> both operands are known NANs, and correctly ask the non-UN prefixed
> op to fold_range if neither operand may be NAN.
> But for the case where one or both operands may be NAN, we always
> return [0, 1].  The UN* fold_range tries to handle it by asking
> the non-UN prefixed fold_range and if it returns [1, 1] return that,
> if it returns [0, 0] or [0, 1] return [0, 1], which makes sense,
> because the maybe NAN means that it is the non-UN prefixed fold_range
> unioned with [1, 1] in case the maybe NAN is actually NAN at runtime.
> The problem is that the non-UN prefixed fold_range always returns [0, 1]
> because those fold_range implementations are like:
>    if (op1.known_isnan () || op2.known_isnan ())
>      r = range_false (type);
>    else if (!maybe_isnan (op1, op2))
>      {
> ...
>      }
>    else
>      r = range_true_and_false (type);
> and so if maybe_isnan, they always return [0, 1].  Now, thinking about it,
> this is unnecessary pessimization, for the case where the ... block
> returns range_false (type) we actually could do it also if maybe_isnan (op1,
> op2), because if one or both operands are NAN, the comparison will be false,
> and if neither is NAN, the comparison will be also false.  Will fix
> incrementally today.
> Anyway, the following patch fixes it by asking the non-UN prefixed
> fold_range on ranges with NAN cleared, which I think does the right
> thing in all cases.
> 
> Another change in the patch is that range_query::get_tree_range
> always returned VARYING for comparisons, this patch allows to ask about
> those as well (they are very much like binary ops, except they take
> the important type from the types of the operands rather than result).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

LGTM
Aldy
> 
> 2023-03-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/91645
> 	* range-op-float.cc (foperator_unordered_lt::fold_range,
> 	foperator_unordered_le::fold_range,
> 	foperator_unordered_gt::fold_range,
> 	foperator_unordered_ge::fold_range,
> 	foperator_unordered_equal::fold_range): Call the ordered
> 	fold_range on ranges with cleared NaNs.
> 	* value-query.cc (range_query::get_tree_range): Handle also
> 	COMPARISON_CLASS_P trees.
> 
> --- gcc/range-op-float.cc.jj	2023-03-28 11:00:29.205986288 +0200
> +++ gcc/range-op-float.cc	2023-03-30 15:14:12.219183066 +0200
> @@ -1587,7 +1587,13 @@ public:
>   	r = range_true (type);
>   	return true;
>         }
> -    if (!fop_lt.fold_range (r, type, op1, op2, rel))
> +    frange op1_no_nan = op1;
> +    frange op2_no_nan = op2;
> +    if (op1.maybe_isnan ())
> +      op1_no_nan.clear_nan ();
> +    if (op2.maybe_isnan ())
> +      op2_no_nan.clear_nan ();
> +    if (!fop_lt.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
>         return false;
>       // The result is the same as the ordered version when the
>       // comparison is true or when the operands cannot be NANs.
> @@ -1692,7 +1698,13 @@ public:
>   	r = range_true (type);
>   	return true;
>         }
> -    if (!fop_le.fold_range (r, type, op1, op2, rel))
> +    frange op1_no_nan = op1;
> +    frange op2_no_nan = op2;
> +    if (op1.maybe_isnan ())
> +      op1_no_nan.clear_nan ();
> +    if (op2.maybe_isnan ())
> +      op2_no_nan.clear_nan ();
> +    if (!fop_le.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
>         return false;
>       // The result is the same as the ordered version when the
>       // comparison is true or when the operands cannot be NANs.
> @@ -1793,7 +1805,13 @@ public:
>   	r = range_true (type);
>   	return true;
>         }
> -    if (!fop_gt.fold_range (r, type, op1, op2, rel))
> +    frange op1_no_nan = op1;
> +    frange op2_no_nan = op2;
> +    if (op1.maybe_isnan ())
> +      op1_no_nan.clear_nan ();
> +    if (op2.maybe_isnan ())
> +      op2_no_nan.clear_nan ();
> +    if (!fop_gt.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
>         return false;
>       // The result is the same as the ordered version when the
>       // comparison is true or when the operands cannot be NANs.
> @@ -1898,7 +1916,13 @@ public:
>   	r = range_true (type);
>   	return true;
>         }
> -    if (!fop_ge.fold_range (r, type, op1, op2, rel))
> +    frange op1_no_nan = op1;
> +    frange op2_no_nan = op2;
> +    if (op1.maybe_isnan ())
> +      op1_no_nan.clear_nan ();
> +    if (op2.maybe_isnan ())
> +      op2_no_nan.clear_nan ();
> +    if (!fop_ge.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
>         return false;
>       // The result is the same as the ordered version when the
>       // comparison is true or when the operands cannot be NANs.
> @@ -2002,7 +2026,13 @@ public:
>   	r = range_true (type);
>   	return true;
>         }
> -    if (!fop_equal.fold_range (r, type, op1, op2, rel))
> +    frange op1_no_nan = op1;
> +    frange op2_no_nan = op2;
> +    if (op1.maybe_isnan ())
> +      op1_no_nan.clear_nan ();
> +    if (op2.maybe_isnan ())
> +      op2_no_nan.clear_nan ();
> +    if (!fop_equal.fold_range (r, type, op1_no_nan, op2_no_nan, rel))
>         return false;
>       // The result is the same as the ordered version when the
>       // comparison is true or when the operands cannot be NANs.
> --- gcc/value-query.cc.jj	2023-03-23 15:25:47.069740988 +0100
> +++ gcc/value-query.cc	2023-03-30 15:20:58.733237311 +0200
> @@ -230,15 +230,21 @@ range_query::get_tree_range (vrange &r,
>       default:
>         break;
>       }
> -  if (BINARY_CLASS_P (expr))
> +  if (BINARY_CLASS_P (expr) || COMPARISON_CLASS_P (expr))
>       {
> -      range_op_handler op (TREE_CODE (expr), type);
> +      tree op0 = TREE_OPERAND (expr, 0);
> +      tree op1 = TREE_OPERAND (expr, 1);
> +      if (COMPARISON_CLASS_P (expr)
> +	  && !Value_Range::supports_type_p (TREE_TYPE (op0)))
> +	return false;
> +      range_op_handler op (TREE_CODE (expr),
> +			   BINARY_CLASS_P (expr) ? type : TREE_TYPE (op0));
>         if (op)
>   	{
> -	  Value_Range r0 (TREE_TYPE (TREE_OPERAND (expr, 0)));
> -	  Value_Range r1 (TREE_TYPE (TREE_OPERAND (expr, 1)));
> -	  range_of_expr (r0, TREE_OPERAND (expr, 0), stmt);
> -	  range_of_expr (r1, TREE_OPERAND (expr, 1), stmt);
> +	  Value_Range r0 (TREE_TYPE (op0));
> +	  Value_Range r1 (TREE_TYPE (op1));
> +	  range_of_expr (r0, op0, stmt);
> +	  range_of_expr (r1, op1, stmt);
>   	  if (!op.fold_range (r, type, r0, r1))
>   	    r.set_varying (type);
>   	}
> 
> 	Jakub
>