public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] [range-ops] Add ability to represent open intervals in frange.
@ 2022-11-11 18:11 Aldy Hernandez
  2022-11-11 19:25 ` Aldy Hernandez
  0 siblings, 1 reply; 4+ messages in thread
From: Aldy Hernandez @ 2022-11-11 18:11 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC patches, Andrew MacLeod, Aldy Hernandez

Currently we represent < and > with a closed interval.  So < 3.0 is
represented as [-INF, +3.0].  This means 3.0 is included in the range,
and though not ideal, is conservatively correct.  Jakub has found a
couple cases where properly representing < and > would help
optimizations and tests, and this patch allows representing open
intervals with real_nextafter.

There are a few caveats.

First, we leave MODE_COMPOSITE_P types pessimistically as a closed interval.

Second, for -ffinite-math-only, real_nextafter will will saturate the
maximum representable number into +INF.  However, this will still do
the right thing, as frange::set() will crop things appropriately.

Finally, and most frustratingly, representing < and > -+0.0 is
problematic because we flush denormals to zero.  Let me explain...

real_nextafter(+0.0, +INF) gives 0x0.8p-148 as expected, but setting a
range to this value yields [+0.0, 0x0.8p-148] because of the flushing.

On the other hand, real_nextafter(+0.0, -INF) (surprisingly) gives
-0.0.8p-148, but setting a range to that value yields [-0.0x8p-148,
-0.0].  I say surprising, because according to cppreference.com,
std::nextafter(+0.0, -INF) should give -0.0.  But that's neither here
nor there because our flushing denormals to zero prevents us from even
representing ranges involving small values around 0.0.  We ultimately
end up with ranges looking like this:

	> +0.0		=> [+0.0, INF]
	> -0.0		=> [+0.0, INF]
	< +0.0		=> [-INF, -0.0]
	< -0.0		=> [-INF, -0.0]

I suppose this is no worse off that what we had before with closed
intervals.  One could even argue that we're better because we at least
have the right sign now ;-).

All other (non-zero) values look sane.

Lightly tested.

Thoughts?

gcc/ChangeLog:

	* range-op-float.cc (build_lt): Adjust with frange_nextafter
	instead of default to a closed range.
	(build_gt): Same.
---
 gcc/range-op-float.cc | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
index 380142b4c14..402393097b2 100644
--- a/gcc/range-op-float.cc
+++ b/gcc/range-op-float.cc
@@ -381,9 +381,17 @@ build_lt (frange &r, tree type, const frange &val)
 	r.set_undefined ();
       return false;
     }
-  // We only support closed intervals.
+
   REAL_VALUE_TYPE ninf = frange_val_min (type);
-  r.set (type, ninf, val.upper_bound ());
+  REAL_VALUE_TYPE prev = val.upper_bound ();
+  machine_mode mode = TYPE_MODE (type);
+  // Default to the conservatively correct closed ranges for
+  // MODE_COMPOSITE_P, otherwise use nextafter.  Note that for
+  // !HONOR_INFINITIES, nextafter will yield -INF, but frange::set()
+  // will crop the range appropriately.
+  if (!MODE_COMPOSITE_P (mode))
+    frange_nextafter (mode, prev, ninf);
+  r.set (type, ninf, prev);
   return true;
 }
 
@@ -424,9 +432,16 @@ build_gt (frange &r, tree type, const frange &val)
       return false;
     }
 
-  // We only support closed intervals.
   REAL_VALUE_TYPE inf = frange_val_max (type);
-  r.set (type, val.lower_bound (), inf);
+  REAL_VALUE_TYPE next = val.lower_bound ();
+  machine_mode mode = TYPE_MODE (type);
+  // Default to the conservatively correct closed ranges for
+  // MODE_COMPOSITE_P, otherwise use nextafter.  Note that for
+  // !HONOR_INFINITIES, nextafter will yield +INF, but frange::set()
+  // will crop the range appropriately.
+  if (!MODE_COMPOSITE_P (mode))
+    frange_nextafter (mode, next, inf);
+  r.set (type, next, inf);
   return true;
 }
 
-- 
2.38.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [range-ops] Add ability to represent open intervals in frange.
  2022-11-11 18:11 [PATCH] [range-ops] Add ability to represent open intervals in frange Aldy Hernandez
@ 2022-11-11 19:25 ` Aldy Hernandez
  2022-11-12  8:53   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Aldy Hernandez @ 2022-11-11 19:25 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC patches, Andrew MacLeod

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

Passes tests for all languages. Passes lapack tests.

So.... ready to be installed unless you have any issues. Oh... I should
write some tests..

Aldy

On Fri, Nov 11, 2022, 19:11 Aldy Hernandez <aldyh@redhat.com> wrote:

> Currently we represent < and > with a closed interval.  So < 3.0 is
> represented as [-INF, +3.0].  This means 3.0 is included in the range,
> and though not ideal, is conservatively correct.  Jakub has found a
> couple cases where properly representing < and > would help
> optimizations and tests, and this patch allows representing open
> intervals with real_nextafter.
>
> There are a few caveats.
>
> First, we leave MODE_COMPOSITE_P types pessimistically as a closed
> interval.
>
> Second, for -ffinite-math-only, real_nextafter will will saturate the
> maximum representable number into +INF.  However, this will still do
> the right thing, as frange::set() will crop things appropriately.
>
> Finally, and most frustratingly, representing < and > -+0.0 is
> problematic because we flush denormals to zero.  Let me explain...
>
> real_nextafter(+0.0, +INF) gives 0x0.8p-148 as expected, but setting a
> range to this value yields [+0.0, 0x0.8p-148] because of the flushing.
>
> On the other hand, real_nextafter(+0.0, -INF) (surprisingly) gives
> -0.0.8p-148, but setting a range to that value yields [-0.0x8p-148,
> -0.0].  I say surprising, because according to cppreference.com,
> std::nextafter(+0.0, -INF) should give -0.0.  But that's neither here
> nor there because our flushing denormals to zero prevents us from even
> representing ranges involving small values around 0.0.  We ultimately
> end up with ranges looking like this:
>
>         > +0.0          => [+0.0, INF]
>         > -0.0          => [+0.0, INF]
>         < +0.0          => [-INF, -0.0]
>         < -0.0          => [-INF, -0.0]
>
> I suppose this is no worse off that what we had before with closed
> intervals.  One could even argue that we're better because we at least
> have the right sign now ;-).
>
> All other (non-zero) values look sane.
>
> Lightly tested.
>
> Thoughts?
>
> gcc/ChangeLog:
>
>         * range-op-float.cc (build_lt): Adjust with frange_nextafter
>         instead of default to a closed range.
>         (build_gt): Same.
> ---
>  gcc/range-op-float.cc | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/range-op-float.cc b/gcc/range-op-float.cc
> index 380142b4c14..402393097b2 100644
> --- a/gcc/range-op-float.cc
> +++ b/gcc/range-op-float.cc
> @@ -381,9 +381,17 @@ build_lt (frange &r, tree type, const frange &val)
>         r.set_undefined ();
>        return false;
>      }
> -  // We only support closed intervals.
> +
>    REAL_VALUE_TYPE ninf = frange_val_min (type);
> -  r.set (type, ninf, val.upper_bound ());
> +  REAL_VALUE_TYPE prev = val.upper_bound ();
> +  machine_mode mode = TYPE_MODE (type);
> +  // Default to the conservatively correct closed ranges for
> +  // MODE_COMPOSITE_P, otherwise use nextafter.  Note that for
> +  // !HONOR_INFINITIES, nextafter will yield -INF, but frange::set()
> +  // will crop the range appropriately.
> +  if (!MODE_COMPOSITE_P (mode))
> +    frange_nextafter (mode, prev, ninf);
> +  r.set (type, ninf, prev);
>    return true;
>  }
>
> @@ -424,9 +432,16 @@ build_gt (frange &r, tree type, const frange &val)
>        return false;
>      }
>
> -  // We only support closed intervals.
>    REAL_VALUE_TYPE inf = frange_val_max (type);
> -  r.set (type, val.lower_bound (), inf);
> +  REAL_VALUE_TYPE next = val.lower_bound ();
> +  machine_mode mode = TYPE_MODE (type);
> +  // Default to the conservatively correct closed ranges for
> +  // MODE_COMPOSITE_P, otherwise use nextafter.  Note that for
> +  // !HONOR_INFINITIES, nextafter will yield +INF, but frange::set()
> +  // will crop the range appropriately.
> +  if (!MODE_COMPOSITE_P (mode))
> +    frange_nextafter (mode, next, inf);
> +  r.set (type, next, inf);
>    return true;
>  }
>
> --
> 2.38.1
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [range-ops] Add ability to represent open intervals in frange.
  2022-11-11 19:25 ` Aldy Hernandez
@ 2022-11-12  8:53   ` Jakub Jelinek
  2022-11-12  8:55     ` Aldy Hernandez
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-11-12  8:53 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: GCC patches, Andrew MacLeod

On Fri, Nov 11, 2022 at 08:25:15PM +0100, Aldy Hernandez wrote:
> Passes tests for all languages. Passes lapack tests.
> 
> So.... ready to be installed unless you have any issues. Oh... I should
> write some tests..

LGTM.

Yeah, for tests we still need to decide whether we make tests in the
style like I've posted working or whether we add a plugin based tests.

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] [range-ops] Add ability to represent open intervals in frange.
  2022-11-12  8:53   ` Jakub Jelinek
@ 2022-11-12  8:55     ` Aldy Hernandez
  0 siblings, 0 replies; 4+ messages in thread
From: Aldy Hernandez @ 2022-11-12  8:55 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: GCC patches, Andrew MacLeod

On Sat, Nov 12, 2022 at 9:54 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Nov 11, 2022 at 08:25:15PM +0100, Aldy Hernandez wrote:
> > Passes tests for all languages. Passes lapack tests.
> >
> > So.... ready to be installed unless you have any issues. Oh... I should
> > write some tests..
>
> LGTM.
>
> Yeah, for tests we still need to decide whether we make tests in the
> style like I've posted working or whether we add a plugin based tests.

FWIW, I don't have any objections to the plugin other than I may not
have enough cycles to help out for a while.

Aldy


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-12  8:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11 18:11 [PATCH] [range-ops] Add ability to represent open intervals in frange Aldy Hernandez
2022-11-11 19:25 ` Aldy Hernandez
2022-11-12  8:53   ` Jakub Jelinek
2022-11-12  8:55     ` 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).