*[COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.@ 2022-10-14 14:26 Aldy Hernandez2022-10-14 14:26 ` [COMMITTED] Normalize ranges over the range for both bounds when -ffinite-math-only Aldy Hernandez ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Aldy Hernandez @ 2022-10-14 14:26 UTC (permalink / raw) To: GCC patches;+Cc:Andrew MacLeod, 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

*[COMMITTED] Normalize ranges over the range for both bounds when -ffinite-math-only.2022-10-14 14:26 [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Aldy Hernandez@ 2022-10-14 14:26 ` Aldy Hernandez2022-10-14 14:26 ` [COMMITTED] Replace CFN_BUILTIN_SIGNBIT* cases with CASE_FLT_FN Aldy Hernandez 2022-10-14 14:32 ` [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Jakub Jelinek 2 siblings, 0 replies; 7+ messages in thread From: Aldy Hernandez @ 2022-10-14 14:26 UTC (permalink / raw) To: GCC patches;+Cc:Andrew MacLeod, 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

*[COMMITTED] Replace CFN_BUILTIN_SIGNBIT* cases with CASE_FLT_FN.2022-10-14 14:26 [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Aldy Hernandez 2022-10-14 14:26 ` [COMMITTED] Normalize ranges over the range for both bounds when -ffinite-math-only Aldy Hernandez@ 2022-10-14 14:26 ` Aldy Hernandez2022-10-14 14:32 ` [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Jakub Jelinek 2 siblings, 0 replies; 7+ messages in thread From: Aldy Hernandez @ 2022-10-14 14:26 UTC (permalink / raw) To: GCC patches;+Cc:Andrew MacLeod, 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

*2022-10-14 14:26 [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Aldy Hernandez 2022-10-14 14:26 ` [COMMITTED] Normalize ranges over the range for both bounds when -ffinite-math-only Aldy Hernandez 2022-10-14 14:26 ` [COMMITTED] Replace CFN_BUILTIN_SIGNBIT* cases with CASE_FLT_FN Aldy HernandezRe: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.@ 2022-10-14 14:32 ` Jakub Jelinek2022-10-14 14:53 ` Aldy Hernandez 2 siblings, 1 reply; 7+ messages in thread From: Jakub Jelinek @ 2022-10-14 14:32 UTC (permalink / raw) To: Aldy Hernandez;+Cc:GCC patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.2022-10-14 14:32 ` [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Jakub Jelinek@ 2022-10-14 14:53 ` Aldy Hernandez2022-10-14 15:00 ` Jakub Jelinek 0 siblings, 1 reply; 7+ messages in thread From: Aldy Hernandez @ 2022-10-14 14:53 UTC (permalink / raw) To: Jakub Jelinek;+Cc:GCC patches [-- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.2022-10-14 14:53 ` Aldy Hernandez@ 2022-10-14 15:00 ` Jakub Jelinek2022-10-14 15:06 ` Aldy Hernandez 0 siblings, 1 reply; 7+ messages in thread From: Jakub Jelinek @ 2022-10-14 15:00 UTC (permalink / raw) To: Aldy Hernandez;+Cc:GCC patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

*Re: [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS.2022-10-14 15:00 ` Jakub Jelinek@ 2022-10-14 15:06 ` Aldy Hernandez0 siblings, 0 replies; 7+ messages in thread From: Aldy Hernandez @ 2022-10-14 15:06 UTC (permalink / raw) To: Jakub Jelinek;+Cc:GCC patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-10-14 15:06 UTC | newest]Thread overview:7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-14 14:26 [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Aldy Hernandez 2022-10-14 14:26 ` [COMMITTED] Normalize ranges over the range for both bounds when -ffinite-math-only Aldy Hernandez 2022-10-14 14:26 ` [COMMITTED] Replace CFN_BUILTIN_SIGNBIT* cases with CASE_FLT_FN Aldy Hernandez 2022-10-14 14:32 ` [COMMITTED] Drop -0.0 in frange::set() for !HONOR_SIGNED_ZEROS Jakub Jelinek 2022-10-14 14:53 ` Aldy Hernandez 2022-10-14 15:00 ` Jakub Jelinek 2022-10-14 15:06 ` 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).