[PATCH] Handle > INF and < INF correctly in range-op-float.cc

[PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Aldy Hernandez]

The gfortran.dg/minlocval*.f90 tests are generating conditionals past
the infinities.  For example:

	if (x <= +Inf)
	  foo (x);
	else
	  bar (x);

It seems to me that the only possible value for x on the false side is
either NAN or undefined (for !HONOR_NANS).

Is this correct, or is there some other FP nuance I'm unaware of?

gcc/ChangeLog:

	* range-op-float.cc (build_lt): Handle < -INF.
	(build_gt): Handle > +INF.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/vrp-float-inf-1.c: New test.
---
 gcc/range-op-float.cc                          | 18 ++++++++++++++++++
 .../gcc.dg/tree-ssa/vrp-float-inf-1.c          | 15 +++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 050f07a9867..4515bbf0b7e 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -235,6 +235,15 @@ build_le (frange &r, tree type, const REAL_VALUE_TYPE &val)
 static void
 build_lt (frange &r, tree type, const REAL_VALUE_TYPE &val)
 {
+  if (real_isinf (&val, 1))
+    {
+      if (HONOR_NANS (type))
+	frange_set_nan (r, type);
+      else
+	r.set_undefined ();
+      return;
+    }
+
   // Hijack LE because we only support closed intervals.
   build_le (r, type, val);
 }
@@ -252,6 +261,15 @@ build_ge (frange &r, tree type, const REAL_VALUE_TYPE &val)
 static void
 build_gt (frange &r, tree type, const REAL_VALUE_TYPE &val)
 {
+  if (real_isinf (&val, 0))
+    {
+      if (HONOR_NANS (type))
+	frange_set_nan (r, type);
+      else
+	r.set_undefined ();
+      return;
+    }
+
   // Hijack GE because we only support closed intervals.
   build_ge (r, type, val);
 }
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c
new file mode 100644
index 00000000000..1d21cce41e6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c
@@ -0,0 +1,15 @@
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-evrp-details" }
+
+void foo ();
+void bar (double);
+
+void funky(double f, double g)
+{
+  if (f <= __builtin_inf ())
+    foo ();
+  else
+    bar (f);
+}
+
+// { dg-final { scan-tree-dump-not " Inf,  Inf" "evrp" } }
-- 
2.37.1

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Jakub Jelinek]

On Tue, Sep 06, 2022 at 09:29:01AM +0200, Aldy Hernandez wrote:
> The gfortran.dg/minlocval*.f90 tests are generating conditionals past
> the infinities.  For example:
> 
> 	if (x <= +Inf)
> 	  foo (x);
> 	else
> 	  bar (x);
> 
> It seems to me that the only possible value for x on the false side is
> either NAN or undefined (for !HONOR_NANS).

No, none of the ==, <, <=, >, >= comparisons are ever true if one
or both operands are NaN (only != will be true in those cases from the
standard comparisons, when not counting UNORDERED_EXPR and the likes).
So, x < -Inf or x > +Inf are always false, we just can't optimize those
away without -ffast-math because they could raise an exception on sNaN.
But I think not optimizing such operations away if we care about exceptions
is the duty of DCE etc.

	Jakub

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Aldy Hernandez]

On Tue, Sep 6, 2022 at 9:35 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Sep 06, 2022 at 09:29:01AM +0200, Aldy Hernandez wrote:
> > The gfortran.dg/minlocval*.f90 tests are generating conditionals past
> > the infinities.  For example:
> >
> >       if (x <= +Inf)
> >         foo (x);
> >       else
> >         bar (x);
> >
> > It seems to me that the only possible value for x on the false side is
> > either NAN or undefined (for !HONOR_NANS).
>
> No, none of the ==, <, <=, >, >= comparisons are ever true if one
> or both operands are NaN (only != will be true in those cases from the
> standard comparisons, when not counting UNORDERED_EXPR and the likes).
> So, x < -Inf or x > +Inf are always false, we just can't optimize those
> away without -ffast-math because they could raise an exception on sNaN.
> But I think not optimizing such operations away if we care about exceptions
> is the duty of DCE etc.

No, no. I'm not talking about optimizing them away.  I'm talking about
representing what is going on in the IL.

For example, for:

if (x > 5.0)
  foo (x);     // {5.0, +Inf] !NAN
else
  bar (x);     // [-Inf, 5.0] ?NAN

So on the true side we know x is in the {5.0, +Inf] range, plus we
know it can't be a NAN.  On the false side we know x is either [-Inf,
5.0] or a NAN.

What I'm trying to do is represent the possible ranges for the false side of:

if (x <= Inf)
...

Aldy

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Jakub Jelinek]

On Tue, Sep 06, 2022 at 09:40:59AM +0200, Aldy Hernandez wrote:
> if (x <= Inf)

This will be [-Inf, Inf] !NAN on the true side and
NAN (either sign) on the false side indeed.

> if (x < -Inf)

will be NAN (either sign) on the true side and
[-Inf, Inf] !NAN on the false side.

	Jakub

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Aldy Hernandez]

On Tue, Sep 6, 2022 at 9:44 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Sep 06, 2022 at 09:40:59AM +0200, Aldy Hernandez wrote:
> > if (x <= Inf)
>
> This will be [-Inf, Inf] !NAN on the true side and
> NAN (either sign) on the false side indeed.
>
> > if (x < -Inf)
>
> will be NAN (either sign) on the true side and
> [-Inf, Inf] !NAN on the false side.

Sweet, that's exactly what I thought, thus the patch.

Furthermore, for !HONOR_NANS I would expect the NAN sides above to be
UNDEFINED/unreachable.  That is, the false side of x <= Inf when
!HONOR_NANS is unreachable.

Agreed?

Aldy

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Jakub Jelinek]

On Tue, Sep 06, 2022 at 09:49:55AM +0200, Aldy Hernandez wrote:
> On Tue, Sep 6, 2022 at 9:44 AM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Tue, Sep 06, 2022 at 09:40:59AM +0200, Aldy Hernandez wrote:
> > > if (x <= Inf)
> >
> > This will be [-Inf, Inf] !NAN on the true side and
> > NAN (either sign) on the false side indeed.
> >
> > > if (x < -Inf)
> >
> > will be NAN (either sign) on the true side and
> > [-Inf, Inf] !NAN on the false side.
> 
> Sweet, that's exactly what I thought, thus the patch.
> 
> Furthermore, for !HONOR_NANS I would expect the NAN sides above to be
> UNDEFINED/unreachable.  That is, the false side of x <= Inf when
> !HONOR_NANS is unreachable.

In practice, there is no real format that has NaNs and doesn't have Infs
or vice versa and similarly we have just one switch to cover both Infinities
and NaNs, so either both are supported, or neither of them, or both
are supported but neither of them should appear in a valid program
(-ffinite-math-only on most floating point formats).
So the answer in that case is a little bit fuzzy because one shouldn't
compare against infinity in that case (or for !MODE_HAS_INFINITIES even
can't).  But sure, if NaNs aren't possible or can't appear and you compare
x <= Largest_possible_float, then it is always true and so UNDEFINED on the
false edge.

	Jakub

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Aldy Hernandez]

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

On Tue, Sep 6, 2022 at 9:59 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Sep 06, 2022 at 09:49:55AM +0200, Aldy Hernandez wrote:
> > On Tue, Sep 6, 2022 at 9:44 AM Jakub Jelinek <jakub@redhat.com> wrote:
> > >
> > > On Tue, Sep 06, 2022 at 09:40:59AM +0200, Aldy Hernandez wrote:
> > > > if (x <= Inf)
> > >
> > > This will be [-Inf, Inf] !NAN on the true side and
> > > NAN (either sign) on the false side indeed.
> > >
> > > > if (x < -Inf)
> > >
> > > will be NAN (either sign) on the true side and
> > > [-Inf, Inf] !NAN on the false side.
> >
> > Sweet, that's exactly what I thought, thus the patch.
> >
> > Furthermore, for !HONOR_NANS I would expect the NAN sides above to be
> > UNDEFINED/unreachable.  That is, the false side of x <= Inf when
> > !HONOR_NANS is unreachable.
>
> In practice, there is no real format that has NaNs and doesn't have Infs
> or vice versa and similarly we have just one switch to cover both Infinities
> and NaNs, so either both are supported, or neither of them, or both
> are supported but neither of them should appear in a valid program
> (-ffinite-math-only on most floating point formats).
> So the answer in that case is a little bit fuzzy because one shouldn't
> compare against infinity in that case (or for !MODE_HAS_INFINITIES even
> can't).  But sure, if NaNs aren't possible or can't appear and you compare
> x <= Largest_possible_float, then it is always true and so UNDEFINED on the
> false edge.

OK, let's leave it as undefined to be consistent.

Come to think of it, perhaps we could represent the endpoints
(varying, [x, +INF], etc) as the min/max representable values for the
type (for !HONOR_NANS).  I don't think it would make a big difference,
but we might get better results for some corner cases.

Question...for !HONOR_NANS or !HONOR_INFINITIES or whatever, say the
range for the domain is [-MIN, +MAX] for the min and max representable
numbers.  What happens for MAX+1?  Is that undefined?  I wonder what
real.cc does for that.

Attached is the final version of the patch I'm pushing.  Tested (+mpfr
tests) on x86-64 Linux.

Aldy

[-- Attachment #2: 0001-Handle-INF-and-INF-correctly-in-range-op-float.cc.patch --]
[-- Type: text/x-patch, Size: 6112 bytes --]

From fd52ffb9997becafa31c5109b50fe274da12aed8 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Tue, 6 Sep 2022 08:20:54 +0200
Subject: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

The gfortran.dg/minlocval*.f90 tests are generating conditionals past
the infinities.  For example:

	if (x <= +Inf)
	  foo (x);
	else
	  bar (x);

It seems to me that the only possible value for x on the false side is
either NAN or undefined (for !HONOR_NANS).

gcc/ChangeLog:

	* range-op-float.cc (build_le): Handle NANs and going past infinity.
	(build_lt): Same.
	(build_ge): Same.
	(build_gt): Same.
	(foperator_lt::op1_range): Avoid adjustments to range if build_*
	returned false.
	(foperator_lt::op2_range): Same.
	(foperator_le::op1_range): Same.
	(foperator_le::op2_range): Same.
	(foperator_gt::op1_range): Same.
	(foperator_gt::op2_range): Same.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/vrp-float-inf-1.c: New test.
---
 gcc/range-op-float.cc                         | 99 ++++++++++++++-----
 .../gcc.dg/tree-ssa/vrp-float-inf-1.c         | 15 +++
 2 files changed, 90 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 050f07a9867..5fbbaa1fb36 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -224,36 +224,79 @@ frange_drop_ninf (frange &r, tree type)
 
 // (X <= VAL) produces the range of [-INF, VAL].
 
-static void
+static bool
 build_le (frange &r, tree type, const REAL_VALUE_TYPE &val)
 {
+  if (real_isnan (&val))
+    {
+      r.set_undefined ();
+      return false;
+    }
   r.set (type, dconstninf, val);
+  return true;
 }
 
 // (X < VAL) produces the range of [-INF, VAL).
 
-static void
+static bool
 build_lt (frange &r, tree type, const REAL_VALUE_TYPE &val)
 {
+  if (real_isnan (&val))
+    {
+      r.set_undefined ();
+      return false;
+    }
+  // < -INF is outside the range.
+  if (real_isinf (&val, 1))
+    {
+      if (HONOR_NANS (type))
+	frange_set_nan (r, type);
+      else
+	r.set_undefined ();
+      return false;
+    }
   // Hijack LE because we only support closed intervals.
   build_le (r, type, val);
+  return true;
 }
 
 // (X >= VAL) produces the range of [VAL, +INF].
 
-static void
+static bool
 build_ge (frange &r, tree type, const REAL_VALUE_TYPE &val)
 {
+  if (real_isnan (&val))
+    {
+      r.set_undefined ();
+      return false;
+    }
   r.set (type, val, dconstinf);
+  return true;
 }
 
 // (X > VAL) produces the range of (VAL, +INF].
 
-static void
+static bool
 build_gt (frange &r, tree type, const REAL_VALUE_TYPE &val)
 {
+  if (real_isnan (&val))
+    {
+      r.set_undefined ();
+      return false;
+    }
+  // > +INF is outside the range.
+  if (real_isinf (&val, 0))
+    {
+      if (HONOR_NANS (type))
+	frange_set_nan (r, type);
+      else
+	r.set_undefined ();
+      return false;
+    }
+
   // Hijack GE because we only support closed intervals.
   build_ge (r, type, val);
+  return true;
 }
 
 
@@ -520,10 +563,12 @@ foperator_lt::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_lt (r, type, op2.upper_bound ());
-      r.set_nan (fp_prop::NO);
-      // x < y implies x is not +INF.
-      frange_drop_inf (r, type);
+      if (build_lt (r, type, op2.upper_bound ()))
+	{
+	  r.set_nan (fp_prop::NO);
+	  // x < y implies x is not +INF.
+	  frange_drop_inf (r, type);
+	}
       break;
 
     case BRS_FALSE:
@@ -546,10 +591,12 @@ foperator_lt::op2_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_gt (r, type, op1.lower_bound ());
-      r.set_nan (fp_prop::NO);
-      // x < y implies y is not -INF.
-      frange_drop_ninf (r, type);
+      if (build_gt (r, type, op1.lower_bound ()))
+	{
+	  r.set_nan (fp_prop::NO);
+	  // x < y implies y is not -INF.
+	  frange_drop_ninf (r, type);
+	}
       break;
 
     case BRS_FALSE:
@@ -618,8 +665,8 @@ foperator_le::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_le (r, type, op2.upper_bound ());
-      r.set_nan (fp_prop::NO);
+      if (build_le (r, type, op2.upper_bound ()))
+	r.set_nan (fp_prop::NO);
       break;
 
     case BRS_FALSE:
@@ -642,8 +689,8 @@ foperator_le::op2_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_ge (r, type, op1.lower_bound ());
-      r.set_nan (fp_prop::NO);
+      if (build_ge (r, type, op1.lower_bound ()))
+	r.set_nan (fp_prop::NO);
       break;
 
     case BRS_FALSE:
@@ -712,10 +759,12 @@ foperator_gt::op1_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_gt (r, type, op2.lower_bound ());
-      r.set_nan (fp_prop::NO);
-      // x > y implies x is not -INF.
-      frange_drop_ninf (r, type);
+      if (build_gt (r, type, op2.lower_bound ()))
+	{
+	  r.set_nan (fp_prop::NO);
+	  // x > y implies x is not -INF.
+	  frange_drop_ninf (r, type);
+	}
       break;
 
     case BRS_FALSE:
@@ -738,10 +787,12 @@ foperator_gt::op2_range (frange &r,
   switch (get_bool_state (r, lhs, type))
     {
     case BRS_TRUE:
-      build_lt (r, type, op1.upper_bound ());
-      r.set_nan (fp_prop::NO);
-      // x > y implies y is not +INF.
-      frange_drop_inf (r, type);
+      if (build_lt (r, type, op1.upper_bound ()))
+	{
+	  r.set_nan (fp_prop::NO);
+	  // x > y implies y is not +INF.
+	  frange_drop_inf (r, type);
+	}
       break;
 
     case BRS_FALSE:
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c
new file mode 100644
index 00000000000..1d21cce41e6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-float-inf-1.c
@@ -0,0 +1,15 @@
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-evrp-details" }
+
+void foo ();
+void bar (double);
+
+void funky(double f, double g)
+{
+  if (f <= __builtin_inf ())
+    foo ();
+  else
+    bar (f);
+}
+
+// { dg-final { scan-tree-dump-not " Inf,  Inf" "evrp" } }
-- 
2.37.1

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Jakub Jelinek]

On Tue, Sep 06, 2022 at 01:47:43PM +0200, Aldy Hernandez wrote:
> Question...for !HONOR_NANS or !HONOR_INFINITIES or whatever, say the
> range for the domain is [-MIN, +MAX] for the min and max representable
> numbers.  What happens for MAX+1?  Is that undefined?  I wonder what
> real.cc does for that.

I'm afraid I have no idea.

The formats without Inf/NaN are:
spu_single_format
vax_{f,d,g}_format
arm_half_format

Never had the "pleasure" to work with any of these.
Looking at encode_vax_*, it seems both GCC internal inf and nan are
most likely are encoded as maximum or minimum representable numbers
(depending on sign), and encode_ieee_half for !fmt->has_inf does too
(for !fmt->has_nans it seems to "encode" the nan mantissa bits into
highest possible exponent).  encode_ieee_single (for spu) uses
maximum or minimum representable numbers for any infinities or nans.
What they actually do at runtime is something I can't really check,
but one would hope it is saturating...

	Jakub

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Richard Biener]

On Tue, Sep 6, 2022 at 2:06 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Tue, Sep 06, 2022 at 01:47:43PM +0200, Aldy Hernandez wrote:
> > Question...for !HONOR_NANS or !HONOR_INFINITIES or whatever, say the
> > range for the domain is [-MIN, +MAX] for the min and max representable
> > numbers.  What happens for MAX+1?  Is that undefined?  I wonder what
> > real.cc does for that.
>
> I'm afraid I have no idea.
>
> The formats without Inf/NaN are:
> spu_single_format
> vax_{f,d,g}_format
> arm_half_format
>
> Never had the "pleasure" to work with any of these.
> Looking at encode_vax_*, it seems both GCC internal inf and nan are
> most likely are encoded as maximum or minimum representable numbers
> (depending on sign), and encode_ieee_half for !fmt->has_inf does too
> (for !fmt->has_nans it seems to "encode" the nan mantissa bits into
> highest possible exponent).  encode_ieee_single (for spu) uses
> maximum or minimum representable numbers for any infinities or nans.
> What they actually do at runtime is something I can't really check,
> but one would hope it is saturating...

I'd simply do what we do for -ffinite-math-only - just pretend you don't
cross the maximum/minmum value and effectively saturate the range.

Richard.

>         Jakub
>

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Aldy Hernandez]

On Tue, Sep 6, 2022 at 2:17 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Sep 6, 2022 at 2:06 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Tue, Sep 06, 2022 at 01:47:43PM +0200, Aldy Hernandez wrote:
> > > Question...for !HONOR_NANS or !HONOR_INFINITIES or whatever, say the
> > > range for the domain is [-MIN, +MAX] for the min and max representable
> > > numbers.  What happens for MAX+1?  Is that undefined?  I wonder what
> > > real.cc does for that.
> >
> > I'm afraid I have no idea.
> >
> > The formats without Inf/NaN are:
> > spu_single_format
> > vax_{f,d,g}_format
> > arm_half_format
> >
> > Never had the "pleasure" to work with any of these.
> > Looking at encode_vax_*, it seems both GCC internal inf and nan are
> > most likely are encoded as maximum or minimum representable numbers
> > (depending on sign), and encode_ieee_half for !fmt->has_inf does too
> > (for !fmt->has_nans it seems to "encode" the nan mantissa bits into
> > highest possible exponent).  encode_ieee_single (for spu) uses
> > maximum or minimum representable numbers for any infinities or nans.
> > What they actually do at runtime is something I can't really check,
> > but one would hope it is saturating...
>
> I'd simply do what we do for -ffinite-math-only - just pretend you don't
> cross the maximum/minmum value and effectively saturate the range.

Yeah, that sounds reasonable.  I take it I can assume that MAX+1 is
undefined for -ffinite-math-only?  So I'm free to saturate, etc?

Ok, crazy hypothetical talk...

if (x > y)

For -ffinite-math-only, on the true side, can I assume:

x: MIN + 1 ULP
y: MAX - 1 ULP

range-op for integers did all sorts of stuff like that, and we ended
up optimizing some interesting cases...as well as driving the middle
end warnings crazy :-P.

Aldy

Re: [PATCH] Handle > INF and < INF correctly in range-op-float.cc

[Koning, Paul]

> On Sep 6, 2022, at 8:06 AM, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> On Tue, Sep 06, 2022 at 01:47:43PM +0200, Aldy Hernandez wrote:
>> Question...for !HONOR_NANS or !HONOR_INFINITIES or whatever, say the
>> range for the domain is [-MIN, +MAX] for the min and max representable
>> numbers.  What happens for MAX+1?  Is that undefined?  I wonder what
>> real.cc does for that.
> 
> I'm afraid I have no idea.
> 
> The formats without Inf/NaN are:
> spu_single_format
> vax_{f,d,g}_format
> arm_half_format

Actually, DEC (VAX and PDP-11) float does have NaN; signaling NaN to be precise.

	paul