public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Aldy Hernandez <aldyh@redhat.com>
Cc: Andrew MacLeod <amacleod@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] range-op-float: Further comparison fixes
Date: Sat, 1 Apr 2023 09:39:10 +0200	[thread overview]
Message-ID: <ZCffnpXHlfKJ+Fy8@tucnak> (raw)
In-Reply-To: <e499e21b-b729-cd4b-5da1-7b02d073268f@redhat.com>

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


  reply	other threads:[~2023-04-01  7:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-31  7:57 [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
2023-03-31 10:45 ` [PATCH] range-op-float: Further comparison fixes Jakub Jelinek
2023-03-31 10:57   ` [PATCH] range-op-float: Further foperator_{,not_}equal::fold_range fix Jakub Jelinek
2023-03-31 11:28     ` Aldy Hernandez
2023-03-31 11:28   ` [PATCH] range-op-float: Further comparison fixes Aldy Hernandez
2023-04-01  7:39     ` Jakub Jelinek [this message]
2023-04-01 18:32       ` Aldy Hernandez
2023-03-31 11:28 ` [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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZCffnpXHlfKJ+Fy8@tucnak \
    --to=jakub@redhat.com \
    --cc=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).