public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Aldy Hernandez <aldyh@redhat.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 09:37:22 +0200	[thread overview]
Message-ID: <CAFiYyc3yRBFpn1EyDyC0KuveVPkSyma17OYtivM8=fx2QXtFug@mail.gmail.com> (raw)
In-Reply-To: <20220917082403.1573721-1-aldyh@redhat.com>

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.

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;
     }
   else if (real_iszero (&m_min))
     m_min.sign = 0;

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  7:37 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 [this message]
2022-09-19  8:25   ` Jakub Jelinek
2022-09-19 13:04   ` Aldy Hernandez
2022-09-19 13:06     ` Aldy Hernandez
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='CAFiYyc3yRBFpn1EyDyC0KuveVPkSyma17OYtivM8=fx2QXtFug@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=aldyh@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.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).