public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
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
> > >
> >


  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).