public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fold __builtin_signbit to nonzero instead of 1.
@ 2022-09-05  6:26 Aldy Hernandez
  0 siblings, 0 replies; only message in thread
From: Aldy Hernandez @ 2022-09-05  6:26 UTC (permalink / raw)
  To: GCC patches; +Cc: Andrew MacLeod, Jakub Jelinek, Joseph Myers, Aldy Hernandez

After Joseph's comment wrt __builtin_signbit, I have been unable to
convince myself that arbitrarily folding __builtin_signbit () of a
negative number to 1 is correct.

For example, on the true side of x < -5.0 we know the sign bit is set,
but on the false side, we know nothing because X may be a NAN.  I
don't want to put ourselves in a position where the same call to
__builtin_signbit can return two different things (1 or negative
whatever), so I'm going to return nonzero which is correct across the
board until someone can convince me otherwise.

Setting the range to nonzero still allows us to fold conditionals
checking __fold_builtin, while not actually propagating a 1.  Zero
propagation still works.

That being said, I still think we should be folding
__builtin_signbit's of negative numbers to whatever the hardware
returns, instead of 1/0 like what the front ends do.

I am going to push this if tests succeed.

gcc/ChangeLog:

	* gimple-range-fold.cc
        (fold_using_range::range_of_builtin_int_call): Fold a set signbit
	in __builtin_signbit to nonzero.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/vrp-float-signbit-2.c: New test.
---
 gcc/gimple-range-fold.cc                      |  5 +---
 .../gcc.dg/tree-ssa/vrp-float-signbit-2.c     | 24 +++++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-2.c

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index d8497fc9be7..3543f0980b8 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1032,10 +1032,7 @@ fold_using_range::range_of_builtin_int_call (irange &r, gcall *call,
 	    if (tmp.get_signbit ().varying_p ())
 	      return false;
 	    if (tmp.get_signbit ().yes_p ())
-	      {
-		tree one = build_one_cst (type);
-		r.set (one, one);
-	      }
+	      r.set_nonzero (type);
 	    else
 	      r.set_zero (type);
 	    return true;
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-2.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-2.c
new file mode 100644
index 00000000000..954c7ebd4f8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-2.c
@@ -0,0 +1,24 @@
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-evrp" }
+
+// Test that the only thing we know about the signbit about negative number is
+// that it's not 0.
+
+void link_error ();
+
+int num;
+
+void func(float x)
+{
+  if (x < -5.0)
+    {
+      num = __builtin_signbit (x);
+
+      // We may not know the exact signbit, but we know it's not 0.
+      if (!__builtin_signbit (x))
+	link_error ();
+    }
+}
+
+// { dg-final { scan-tree-dump-not "num = \[-0-9\];" "evrp" } }
+// { dg-final { scan-tree-dump-not "link_error" "evrp" } }
-- 
2.37.1


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-09-05  6:27 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05  6:26 [PATCH] Fold __builtin_signbit to nonzero instead of 1 Aldy Hernandez

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).