From: Aldy Hernandez <aldyh@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Jakub Jelinek <jakub@redhat.com>,
Richard Henderson <rth@gcc.gnu.org>,
GCC patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] frange: flush denormals to zero for -funsafe-math-optimizations.
Date: Mon, 19 Sep 2022 15:06:51 +0200 [thread overview]
Message-ID: <CAGm3qMU6yC1bcrPhHPc583ZrKzuiNJ24bcQdvohRqLfnxMZzpw@mail.gmail.com> (raw)
In-Reply-To: <CAGm3qMVWu2rAcj-=CeLiujUWQbEbOgfrQLDmc9WmX4iYbVzLjQ@mail.gmail.com>
On Mon, Sep 19, 2022 at 3:04 PM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> On Mon, Sep 19, 2022 at 9:37 AM Richard Biener
> <richard.guenther@gmail.com> wrote:
> >
> > On Sat, Sep 17, 2022 at 10:25 AM Aldy Hernandez via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> > >
> > > Jakub has mentioned that for -funsafe-math-optimizations we may flush
> > > denormals to zero, in which case we need to be careful to extend the
> > > ranges to the appropriate zero. This patch does exactly that. For a
> > > range of [x, -DENORMAL] we flush to [x, -0.0] and for [+DENORMAL, x]
> > > we flush to [+0.0, x].
> > >
> > > It is unclear whether we should do this for Alpha, since I believe
> > > flushing to zero is the default, and the port requires -mieee for IEEE
> > > sanity. If so, perhaps we should add a target hook so backends are
> > > free to request flushing to zero.
> > >
> > > Thoughts?
> >
> > I'm not sure what the intention of this is - it effectively results in
> > more conservative ranges for -funsafe-math-optimizations. That is,
> > if -funsafe-math-optimizations says that denormals don't exist and
> > are all zero then doesn't that mean we can instead use the smalles
> > non-denormal number less than (or greater than) zero here?
> >
> > That said, the flushing you do is also "valid" for
> > -fno-unsafe-math-optimizations
> > in case we don't want to deal with denormals in range boundaries.
> >
> > It might also be a correctness requirement in case we don't know how
> > targets handle denormals (IIRC even OS defaults might matter here) so
> > we conservatively have to treat them as being flushed to zero.
>
> Actually, rth suggested we always flush to zero because we don't know
> what the target would do. Again, I'm happy to do whatever you agree
More specifically, we don't know what the OS will do, so we either
should have a flag of some kind set to TRUE by default when unsafe
math optimizations, or always assume denormal flushing could happen.
Again, no opinion. Up to y'all.
Aldy
> on. I have no opinion. My main goal here is correctness.
>
> >
> > So ... if we want to be on the "safe" side then please always do this.
> >
> > At the same point you could do
> >
> > if (!HONOR_SIGNED_ZEROS ())
> > if (real_iszero (&m_max))
> > {
> > if (real_iszero (&m_min))
> > m.min.sign = 1;
> > m_max.sign = 1;
> > }
>
> But wouldn't that set [-0.0, -0.0] when encountering [+0, +0] ??
>
> > else if (real_iszero (&m_min))
> > m_min.sign = 0;
>
> Jakub actually suggested something completely different...just set +0
> always for !HONOR_SIGNED_ZEROS.
>
> Aldy
>
> >
> > so that we canonicalize a zero bound so that the sign is known for a range.
> >
> > Richard.
> >
> > > gcc/ChangeLog:
> > >
> > > * value-range.cc (frange::flush_denormals_to_zero): New.
> > > (frange::set): Call flush_denormals_to_zero.
> > > * value-range.h (class frange): Add flush_denormals_to_zero.
> > > ---
> > > gcc/value-range.cc | 24 ++++++++++++++++++++++++
> > > gcc/value-range.h | 1 +
> > > 2 files changed, 25 insertions(+)
> > >
> > > diff --git a/gcc/value-range.cc b/gcc/value-range.cc
> > > index 67d5d7fa90f..f285734f0e0 100644
> > > --- a/gcc/value-range.cc
> > > +++ b/gcc/value-range.cc
> > > @@ -267,6 +267,26 @@ tree_compare (tree_code code, tree op1, tree op2)
> > > return !integer_zerop (fold_build2 (code, integer_type_node, op1, op2));
> > > }
> > >
> > > +// Flush denormal endpoints to the appropriate 0.0.
> > > +
> > > +void
> > > +frange::flush_denormals_to_zero ()
> > > +{
> > > + if (undefined_p () || known_isnan ())
> > > + return;
> > > +
> > > + // Flush [x, -DENORMAL] to [x, -0.0].
> > > + if (real_isdenormal (&m_max) && real_isneg (&m_max))
> > > + {
> > > + m_max = dconst0;
> > > + if (HONOR_SIGNED_ZEROS (m_type))
> > > + m_max.sign = 1;
> > > + }
> > > + // Flush [+DENORMAL, x] to [+0.0, x].
> > > + if (real_isdenormal (&m_min) && !real_isneg (&m_min))
> > > + m_min = dconst0;
> > > +}
> > > +
> > > // Setter for franges.
> > >
> > > void
> > > @@ -317,6 +337,10 @@ frange::set (tree min, tree max, value_range_kind kind)
> > > gcc_checking_assert (tree_compare (LE_EXPR, min, max));
> > >
> > > normalize_kind ();
> > > +
> > > + if (flag_unsafe_math_optimizations)
> > > + flush_denormals_to_zero ();
> > > +
> > > if (flag_checking)
> > > verify_range ();
> > > }
> > > diff --git a/gcc/value-range.h b/gcc/value-range.h
> > > index 3a401f3e4e2..795b1f00fdc 100644
> > > --- a/gcc/value-range.h
> > > +++ b/gcc/value-range.h
> > > @@ -327,6 +327,7 @@ private:
> > > bool union_nans (const frange &);
> > > bool intersect_nans (const frange &);
> > > bool combine_zeros (const frange &, bool union_p);
> > > + void flush_denormals_to_zero ();
> > >
> > > tree m_type;
> > > REAL_VALUE_TYPE m_min;
> > > --
> > > 2.37.1
> > >
> >
next prev parent reply other threads:[~2022-09-19 13:07 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-17 8:24 Aldy Hernandez
2022-09-19 7:37 ` Richard Biener
2022-09-19 8:25 ` Jakub Jelinek
2022-09-19 13:04 ` Aldy Hernandez
2022-09-19 13:06 ` Aldy Hernandez [this message]
2022-09-19 13:44 ` Richard Biener
2022-09-20 5:22 ` Aldy Hernandez
2022-09-20 8:26 ` Jakub Jelinek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGm3qMU6yC1bcrPhHPc583ZrKzuiNJ24bcQdvohRqLfnxMZzpw@mail.gmail.com \
--to=aldyh@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=richard.guenther@gmail.com \
--cc=rth@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).