[COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.

[COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.

[Aldy Hernandez]

Similar to what we do for NANs when !HONOR_NANS and Inf when
flag_finite_math_only, we can remove -0.0 from the range at creation
time.

We were kinda sorta doing this because there is a bug in
real_isdenormal that is causing flush_denormals_to_zero to saturate
[x, -0.0] to [x, +0.0] when !HONOR_SIGNED_ZEROS.  Fixing this bug
(upcoming), causes us to leave -0.0 in places where we aren't
expecting it (the intersection code).

gcc/ChangeLog:

	* value-range.cc (frange::set): Drop -0.0 for !HONOR_SIGNED_ZEROS.
---
 gcc/value-range.cc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 26a2b782e2c..86550f158b8 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -324,6 +324,14 @@ frange::set (tree type,
       m_neg_nan = false;
     }
 
+  if (!HONOR_SIGNED_ZEROS (m_type))
+    {
+      if (real_iszero (&m_min, 1))
+	m_min.sign = 0;
+      if (real_iszero (&m_max, 1))
+	m_max.sign = 0;
+    }
+
   // For -ffinite-math-only we can drop ranges outside the
   // representable numbers to min/max for the type.
   if (flag_finite_math_only)
-- 
2.37.3

[COMMITTED] Normalize ranges over the range for both bounds when -ffinite-math-only.

[Aldy Hernandez]

[-Inf, +Inf] was being chopped correctly for -ffinite-math-only, but
[-Inf, -Inf] was not.  This was latent because a bug in
real_isdenormal is causing us to flush -Inf to zero.

gcc/ChangeLog:

	* value-range.cc (frange::set): Normalize ranges for both bounds.
---
 gcc/value-range.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 86550f158b8..6b4f3dddcd5 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -340,8 +340,12 @@ frange::set (tree type,
       REAL_VALUE_TYPE max_repr = frange_val_max (m_type);
       if (real_less (&m_min, &min_repr))
 	m_min = min_repr;
+      else if (real_less (&max_repr, &m_min))
+	m_min = max_repr;
       if (real_less (&max_repr, &m_max))
 	m_max = max_repr;
+      else if (real_less (&m_max, &min_repr))
+	m_max = min_repr;
     }
 
   // Check for swapped ranges.
-- 
2.37.3

[COMMITTED] Replace CFN_BUILTIN_SIGNBIT* cases with CASE_FLT_FN.

[Aldy Hernandez]

gcc/ChangeLog:

	* gimple-range-op.cc
	(gimple_range_op_handler::maybe_builtin_call): Replace
	CFN_BUILTIN_SIGNBIT* cases with CASE_FLT_FN.
---
 gcc/gimple-range-op.cc | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc
index 9bdef3d45c0..527893f66af 100644
--- a/gcc/gimple-range-op.cc
+++ b/gcc/gimple-range-op.cc
@@ -756,9 +756,7 @@ gimple_range_op_handler::maybe_builtin_call ()
 	m_valid = false;
       break;
 
-    case CFN_BUILT_IN_SIGNBIT:
-    case CFN_BUILT_IN_SIGNBITF:
-    case CFN_BUILT_IN_SIGNBITL:
+    CASE_FLT_FN (CFN_BUILT_IN_SIGNBIT):
       m_op1 = gimple_call_arg (call, 0);
       m_float = &op_cfn_signbit;
       m_valid = true;
-- 
2.37.3

Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.

[Jakub Jelinek]

On Fri, Oct 14, 2022 at 04:26:50PM +0200, Aldy Hernandez via Gcc-patches wrote:
> Similar to what we do for NANs when !HONOR_NANS and Inf when
> flag_finite_math_only, we can remove -0.0 from the range at creation
> time.
> 
> We were kinda sorta doing this because there is a bug in
> real_isdenormal that is causing flush_denormals_to_zero to saturate
> [x, -0.0] to [x, +0.0] when !HONOR_SIGNED_ZEROS.  Fixing this bug
> (upcoming), causes us to leave -0.0 in places where we aren't
> expecting it (the intersection code).
> 
> gcc/ChangeLog:
> 
> 	* value-range.cc (frange::set): Drop -0.0 for !HONOR_SIGNED_ZEROS.

This looks wrong to me.
!HONOR_NANS is different from !HONOR_SIGNED_ZEROS.
The former says that either NaNs aren't supported or if they appear,
it will be UB.
The latter says that either -0.0 doesn't exist, or user doesn't care
if -0.0 or 0.0 is used.

So, what you do is ok for !MODE_HAS_SIGNED_ZEROS (TYPE_MODE (m_type)),
but otherwise we want to canonicalize [x, -0.0] to [x, 0.0] and
[0.0, y] to [-0.0, y].

> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -324,6 +324,14 @@ frange::set (tree type,
>        m_neg_nan = false;
>      }
>  
> +  if (!HONOR_SIGNED_ZEROS (m_type))
> +    {
> +      if (real_iszero (&m_min, 1))
> +	m_min.sign = 0;
> +      if (real_iszero (&m_max, 1))
> +	m_max.sign = 0;
> +    }
> +
>    // For -ffinite-math-only we can drop ranges outside the
>    // representable numbers to min/max for the type.
>    if (flag_finite_math_only)
> -- 
> 2.37.3

	Jakub

Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.

[Aldy Hernandez]

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

On Fri, Oct 14, 2022 at 4:33 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Oct 14, 2022 at 04:26:50PM +0200, Aldy Hernandez via Gcc-patches wrote:
> > Similar to what we do for NANs when !HONOR_NANS and Inf when
> > flag_finite_math_only, we can remove -0.0 from the range at creation
> > time.
> >
> > We were kinda sorta doing this because there is a bug in
> > real_isdenormal that is causing flush_denormals_to_zero to saturate
> > [x, -0.0] to [x, +0.0] when !HONOR_SIGNED_ZEROS.  Fixing this bug
> > (upcoming), causes us to leave -0.0 in places where we aren't
> > expecting it (the intersection code).
> >
> > gcc/ChangeLog:
> >
> >       * value-range.cc (frange::set): Drop -0.0 for !HONOR_SIGNED_ZEROS.
>
> This looks wrong to me.
> !HONOR_NANS is different from !HONOR_SIGNED_ZEROS.
> The former says that either NaNs aren't supported or if they appear,
> it will be UB.
> The latter says that either -0.0 doesn't exist, or user doesn't care
> if -0.0 or 0.0 is used.
>
> So, what you do is ok for !MODE_HAS_SIGNED_ZEROS (TYPE_MODE (m_type)),
> but otherwise we want to canonicalize [x, -0.0] to [x, 0.0] and
> [0.0, y] to [-0.0, y].

If the user doesn't care, I would expect  they'd be ok with it being
+0.0, but I must say, this is way beyond my paygrade.

How does this patch in testing look?

Thanks.
Aldy

[-- Attachment #2: 0001-Implement-distinction-between-HONOR_SIGNED_ZEROS-and.patch --]
[-- Type: text/x-patch, Size: 1181 bytes --]

From 045d57b979722d15ce7fce733616bbf4ab0e0459 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Fri, 14 Oct 2022 16:49:33 +0200
Subject: [PATCH] Implement distinction between HONOR_SIGNED_ZEROS and
 MODE_HAS_SIGNED_ZEROS.

gcc/ChangeLog:

	* value-range.cc (frange::set): Implement distinction between
	HONOR_SIGNED_ZEROS and MODE_HAS_SIGNED_ZEROS.
---
 gcc/value-range.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index 6b4f3dddcd5..e5b4c1565d4 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -324,13 +324,20 @@ frange::set (tree type,
       m_neg_nan = false;
     }
 
-  if (!HONOR_SIGNED_ZEROS (m_type))
+  if (!MODE_HAS_SIGNED_ZEROS (TYPE_MODE (m_type)))
     {
       if (real_iszero (&m_min, 1))
 	m_min.sign = 0;
       if (real_iszero (&m_max, 1))
 	m_max.sign = 0;
     }
+  else if (!HONOR_SIGNED_ZEROS (m_type))
+    {
+      if (real_iszero (&m_max, 1))
+	m_max.sign = 0;
+      if (real_iszero (&m_min, 0))
+	m_min.sign = 1;
+    }
 
   // For -ffinite-math-only we can drop ranges outside the
   // representable numbers to min/max for the type.
-- 
2.37.3

Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.

[Jakub Jelinek]

On Fri, Oct 14, 2022 at 04:53:13PM +0200, Aldy Hernandez wrote:
> > This looks wrong to me.
> > !HONOR_NANS is different from !HONOR_SIGNED_ZEROS.
> > The former says that either NaNs aren't supported or if they appear,
> > it will be UB.
> > The latter says that either -0.0 doesn't exist, or user doesn't care
> > if -0.0 or 0.0 is used.
> >
> > So, what you do is ok for !MODE_HAS_SIGNED_ZEROS (TYPE_MODE (m_type)),
> > but otherwise we want to canonicalize [x, -0.0] to [x, 0.0] and
> > [0.0, y] to [-0.0, y].
> 
> If the user doesn't care, I would expect  they'd be ok with it being
> +0.0, but I must say, this is way beyond my paygrade.

Unlike the NaN case where they can (easily) arrange for NaNs not to appear
(say, avoid numerically undefined operations), for +/-0 if the hardware
supports it they don't have much choice, sometimes computation will yield
one, sometimes the other.  The option is "I don't use anything that depends
on the zero sign, which is e.g. copysign from zero, signbit or poking at the
bit patterns".

> How does this patch in testing look?

LGTM (perhaps some comment would be nice though).

> From 045d57b979722d15ce7fce733616bbf4ab0e0459 Mon Sep 17 00:00:00 2001
> From: Aldy Hernandez <aldyh@redhat.com>
> Date: Fri, 14 Oct 2022 16:49:33 +0200
> Subject: [PATCH] Implement distinction between HONOR_SIGNED_ZEROS and
>  MODE_HAS_SIGNED_ZEROS.
> 
> gcc/ChangeLog:
> 
> 	* value-range.cc (frange::set): Implement distinction between
> 	HONOR_SIGNED_ZEROS and MODE_HAS_SIGNED_ZEROS.
> ---
>  gcc/value-range.cc | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> index 6b4f3dddcd5..e5b4c1565d4 100644
> --- a/gcc/value-range.cc
> +++ b/gcc/value-range.cc
> @@ -324,13 +324,20 @@ frange::set (tree type,
>        m_neg_nan = false;
>      }
>  
> -  if (!HONOR_SIGNED_ZEROS (m_type))
> +  if (!MODE_HAS_SIGNED_ZEROS (TYPE_MODE (m_type)))
>      {
>        if (real_iszero (&m_min, 1))
>  	m_min.sign = 0;
>        if (real_iszero (&m_max, 1))
>  	m_max.sign = 0;
>      }
> +  else if (!HONOR_SIGNED_ZEROS (m_type))
> +    {
> +      if (real_iszero (&m_max, 1))
> +	m_max.sign = 0;
> +      if (real_iszero (&m_min, 0))
> +	m_min.sign = 1;
> +    }
>  
>    // For -ffinite-math-only we can drop ranges outside the
>    // representable numbers to min/max for the type.
> -- 
> 2.37.3
> 


	Jakub

Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.

[Aldy Hernandez]

On Fri, Oct 14, 2022 at 5:00 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Oct 14, 2022 at 04:53:13PM +0200, Aldy Hernandez wrote:
> > > This looks wrong to me.
> > > !HONOR_NANS is different from !HONOR_SIGNED_ZEROS.
> > > The former says that either NaNs aren't supported or if they appear,
> > > it will be UB.
> > > The latter says that either -0.0 doesn't exist, or user doesn't care
> > > if -0.0 or 0.0 is used.
> > >
> > > So, what you do is ok for !MODE_HAS_SIGNED_ZEROS (TYPE_MODE (m_type)),
> > > but otherwise we want to canonicalize [x, -0.0] to [x, 0.0] and
> > > [0.0, y] to [-0.0, y].
> >
> > If the user doesn't care, I would expect  they'd be ok with it being
> > +0.0, but I must say, this is way beyond my paygrade.
>
> Unlike the NaN case where they can (easily) arrange for NaNs not to appear
> (say, avoid numerically undefined operations), for +/-0 if the hardware
> supports it they don't have much choice, sometimes computation will yield
> one, sometimes the other.  The option is "I don't use anything that depends
> on the zero sign, which is e.g. copysign from zero, signbit or poking at the
> bit patterns".
>
> > How does this patch in testing look?
>
> LGTM (perhaps some comment would be nice though).

Ughh, can I trouble you for one, cause frankly I'm a bit lost on this one?

Aldy