diff mbox series

[COMMITTED] Implement ranger folder for __builtin_signbit.

Message ID 20220901111851.2595874-2-aldyh@redhat.com
State New, archived
Headers
Series [COMMITTED] Implement ranger folder for __builtin_signbit. |

Commit Message

Aldy Hernandez Sept. 1, 2022, 11:18 a.m. UTC 
  Now that we keep track of the signbit, we can use it to fold __builtin_signbit.

I am assuming I don't have try too hard to get the actual signbit
number and 1 will do.  Especially, since we're inconsistent in trunk whether
we fold the builtin or whether we calculate it at runtime.

abulafia:~$ cat a.c
float nzero = -0.0;

main(){
    printf("0x%x\n", __builtin_signbit(-0.0));
    printf("0x%x\n", __builtin_signbit(nzero));
}
abulafia:~$ gcc a.c -w && ./a.out
0x1
0x80000000

It is amazing that we've been failing to fold something as simple as
this:

	if (x > 5.0)
	  num = __builtin_signbit (x);

It does the right thing now :-P.

gcc/ChangeLog:

	* gimple-range-fold.cc
	(fold_using_range::range_of_builtin_int_call): Add case for
	CFN_BUILT_IN_SIGNBIT.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/vrp-float-signbit-1.c: New test.
---
 gcc/gimple-range-fold.cc                      | 20 +++++++++++++++++++
 .../gcc.dg/tree-ssa/vrp-float-signbit-1.c     | 12 +++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-1.c

Comments

Joseph Myers Sept. 1, 2022, 4:41 p.m. UTC | #1 
On Thu, 1 Sep 2022, Aldy Hernandez via Gcc-patches wrote:

> Now that we keep track of the signbit, we can use it to fold __builtin_signbit.
> 
> I am assuming I don't have try too hard to get the actual signbit
> number and 1 will do.  Especially, since we're inconsistent in trunk whether
> we fold the builtin or whether we calculate it at runtime.

The main thing to watch out for is cases where, in the abstract machine, 
there is a single call executed to __builtin_signbit, but after code 
transformations, some uses of the result of that call are optimized to use 
a 0 or 1 value and other uses end up using a runtime value - inconsistency 
between different calls is fine, inconsistency where only a single call is 
executed in the abstract machine isn't.  (Cf. bugs 102930, 85957, 93681, 
93806, 93682, for example; the test in bug 93806 comment 27 is the sort of 
thing to try.)
Aldy Hernandez Sept. 1, 2022, 6:25 p.m. UTC | #2 
On Thu, Sep 1, 2022 at 6:41 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 1 Sep 2022, Aldy Hernandez via Gcc-patches wrote:
>
> > Now that we keep track of the signbit, we can use it to fold __builtin_signbit.
> >
> > I am assuming I don't have try too hard to get the actual signbit
> > number and 1 will do.  Especially, since we're inconsistent in trunk whether
> > we fold the builtin or whether we calculate it at runtime.
>
> The main thing to watch out for is cases where, in the abstract machine,
> there is a single call executed to __builtin_signbit, but after code
> transformations, some uses of the result of that call are optimized to use
> a 0 or 1 value and other uses end up using a runtime value - inconsistency
> between different calls is fine, inconsistency where only a single call is
> executed in the abstract machine isn't.  (Cf. bugs 102930, 85957, 93681,
> 93806, 93682, for example; the test in bug 93806 comment 27 is the sort of
> thing to try.)

Can't we just be consistent with the runtime?  I'm happy to return
whatever value is appropriate for the architecture.  It doesn't have
to be 1.  Though ISTM that if we know the sign on one side of a
conditional, we should know the sign on the other side of the
conditional, so we should fold all uses of __builtin_signbit in that
case.  So maybe we're ok.

On the other hand, we could narrow the range to nonzero, which we can
model perfectly for integers (and __builtin_signbit returns one).  If
we take this approach it means we can't fold:

  if (x < -5.0)
    num = __builtin_signbit (x);

but we could fold:

  if (x > 5.0)
    num = __builtin_signbit (x);

since that's always 0.

And it also means we could fold conditional checks against zero/nonzero:

void func(float x)
{
  if (x < -5.0 && !__builtin_signbit (x))
    link_error ();
}

Whatever works for y'all.
Aldy

p.s. My head exploded after reading half of PR93806.  I should just go
back to integers.
diff mbox series

Patch

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index b0b22106320..d8497fc9be7 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -1023,6 +1023,26 @@  fold_using_range::range_of_builtin_int_call (irange &r, gcall *call,
 	break;
       }
 
+    case CFN_BUILT_IN_SIGNBIT:
+      {
+	arg = gimple_call_arg (call, 0);
+	frange tmp;
+	if (src.get_operand (tmp, arg))
+	  {
+	    if (tmp.get_signbit ().varying_p ())
+	      return false;
+	    if (tmp.get_signbit ().yes_p ())
+	      {
+		tree one = build_one_cst (type);
+		r.set (one, one);
+	      }
+	    else
+	      r.set_zero (type);
+	    return true;
+	  }
+	break;
+      }
+
     case CFN_BUILT_IN_TOUPPER:
       {
 	arg = gimple_call_arg (call, 0);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-1.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-1.c
new file mode 100644
index 00000000000..3fa783ec460
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-signbit-1.c
@@ -0,0 +1,12 @@ 
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-evrp" }
+
+int num;
+
+void func(float x)
+{
+  if (x > 5.0)
+    num = __builtin_signbit (x);
+}
+
+// { dg-final { scan-tree-dump-times "num = 0;" 1 "evrp" } }