public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew MacLeod <amacleod@redhat.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Cc: "hernandez, aldy" <aldyh@redhat.com>
Subject: [COMMITTED] Fix nan updating in range-ops.
Date: Mon, 17 Oct 2022 09:25:24 -0400	[thread overview]
Message-ID: <03ebe7bc-13bf-a37f-7f8d-d2146e2df918@redhat.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

There is a path in which clear_nan() is called on an UNDEFINED range, 
which is not allowed.  This patch simply makes sure VARYING is set 
before calling clear_nan().

In operator_not_equal, we should check if op1 == op1 AFTER the check for 
a singleton.

operator_ordered was also cealring the NAN on the false side, and should 
be setting it.

None of these paths were being executed to this point as GORI was not 
passing in the relation between op1 and op2, but the next patch changes 
that and would trigger these issues.

Bootstrapped on x86_64-pc-linux-gnu with no regressions.  Pushed.

Andrew



[-- Attachment #2: 0002-Fix-nan-updating-in-range-ops.patch --]
[-- Type: text/x-patch, Size: 2842 bytes --]

From 04874fedae8074b252abbd70fea68bf3dd0a605b Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Fri, 14 Oct 2022 09:29:23 -0400
Subject: [PATCH 2/4] Fix nan updating in range-ops.

Calling clean_nan on an undefined type traps, set_varying first. Other
tweaks for correctness.

	* range-op-float.cc (foperator_not_equal::op1_range): Check for
	VREL_EQ after singleton.
	(foperator_unordered::op1_range): Set VARYING before calling
	clear_nan().
	(foperator_ordered::op1_range): Set rather than clear NAN if both
	operands are the same.
---
 gcc/range-op-float.cc | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 23e0f5ef4e2..6cf2180ce59 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -510,12 +510,9 @@ foperator_not_equal::op1_range (frange &r, tree type,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      // The TRUE side of op1 != op1 implies op1 is NAN.
-      if (rel == VREL_EQ)
-	r.set_nan (type);
       // If the result is true, the only time we know anything is if
       // OP2 is a constant.
-      else if (op2.singleton_p ())
+      if (op2.singleton_p ())
 	{
 	  // This is correct even if op1 is NAN, because the following
 	  // range would be ~[tmp, tmp] with the NAN property set to
@@ -523,6 +520,9 @@ foperator_not_equal::op1_range (frange &r, tree type,
 	  REAL_VALUE_TYPE tmp = op2.lower_bound ();
 	  r.set (type, tmp, tmp, VR_ANTI_RANGE);
 	}
+      // The TRUE side of op1 != op1 implies op1 is NAN.
+      else if (rel == VREL_EQ)
+	r.set_nan (type);
       else
 	r.set_varying (type);
       break;
@@ -1045,22 +1045,18 @@ foperator_unordered::op1_range (frange &r, tree type,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      if (rel == VREL_EQ)
-	r.set_nan (type);
       // Since at least one operand must be NAN, if one of them is
       // not, the other must be.
-      else if (!op2.maybe_isnan ())
+      if (rel == VREL_EQ || !op2.maybe_isnan ())
 	r.set_nan (type);
       else
 	r.set_varying (type);
       break;
 
     case BRS_FALSE:
-      if (rel == VREL_EQ)
-	r.clear_nan ();
       // A false UNORDERED means both operands are !NAN, so it's
       // impossible for op2 to be a NAN.
-      else if (op2.known_isnan ())
+      if (op2.known_isnan ())
 	r.set_undefined ();
       else
 	{
@@ -1132,10 +1128,11 @@ foperator_ordered::op1_range (frange &r, tree type,
       break;
 
     case BRS_FALSE:
-      r.set_varying (type);
-      // The FALSE side of op1 ORDERED op1 implies op1 is !NAN.
+      // The FALSE side of op1 ORDERED op1 implies op1 is NAN.
       if (rel == VREL_EQ)
-	r.clear_nan ();
+	r.set_nan (type);
+      else
+	r.set_varying (type);
       break;
 
     default:
-- 
2.37.3


                 reply	other threads:[~2022-10-17 13:25 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=03ebe7bc-13bf-a37f-7f8d-d2146e2df918@redhat.com \
    --to=amacleod@redhat.com \
    --cc=aldyh@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).