public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Aldy Hernandez <aldyh@redhat.com>
To: GCC patches <gcc-patches@gcc.gnu.org>
Cc: Jakub Jelinek <jakub@redhat.com>,
	Andrew MacLeod <amacleod@redhat.com>,
	Aldy Hernandez <aldyh@redhat.com>
Subject: [PATCH] Do not specify NAN sign in frange::set_nonnegative.
Date: Thu, 10 Nov 2022 16:03:46 +0100	[thread overview]
Message-ID: <20221110150345.157116-1-aldyh@redhat.com> (raw)

[Jakub, how's this?  Do you agree?]

After further reading of the IEEE 754 standard, it has become clear
that there are no guarantees with regards to the sign of a NAN when it
comes to any operation other than copy, copysign, abs, and negate.

Currently, set_nonnegative() is only used in one place in ranger
applicable to floating point values, when expanding unknown calls.
Since we already specially handle copy, copysign, abs, and negate, all
the calls to set_nonnegative() must be NAN-sign agnostic.

The cleanest solution is to leave the sign unspecificied in
frange::set_nonnegative().  Any special case, must be handled by the
caller.

gcc/ChangeLog:

	* value-range.cc (frange::set_nonnegative): Remove NAN sign handling.
	(range_tests_signed_zeros): Adjust test.
---
 gcc/value-range.cc | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 852ac09f2c4..d55d85846c1 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -797,14 +797,17 @@ frange::zero_p () const
 	  && real_iszero (&m_max));
 }
 
+// Set the range to non-negative numbers, that is [+0.0, +INF].
+//
+// The NAN in the resulting range (if HONOR_NANS) has a varying sign
+// as there are no guarantees in IEEE 754 wrt to the sign of a NAN,
+// except for copy, abs, and copysign.  It is the responsibility of
+// the caller to set the NAN's sign if desired.
+
 void
 frange::set_nonnegative (tree type)
 {
   set (type, dconst0, frange_val_max (type));
-
-  // Set +NAN as the only possibility.
-  if (HONOR_NANS (type))
-    update_nan (/*sign=*/false);
 }
 
 // Here we copy between any two irange's.  The ranges can be legacy or
@@ -3923,7 +3926,6 @@ range_tests_signed_zeros ()
     ASSERT_TRUE (r0.undefined_p ());
 
   r0.set_nonnegative (float_type_node);
-  ASSERT_TRUE (r0.signbit_p (signbit) && !signbit);
   if (HONOR_NANS (float_type_node))
     ASSERT_TRUE (r0.maybe_isnan ());
 }
-- 
2.38.1


             reply	other threads:[~2022-11-10 15:04 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10 15:03 Aldy Hernandez [this message]
2022-11-10 15:40 ` Jakub Jelinek

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=20221110150345.157116-1-aldyh@redhat.com \
    --to=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /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).