public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: Matthias Kretz <m.kretz@gsi.de>
Cc: gcc-patches@gcc.gnu.org, libstdc++ <libstdc++@gcc.gnu.org>
Subject: Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
Date: Thu, 21 May 2020 15:24:49 +0100	[thread overview]
Message-ID: <20200521142449.GE2678@redhat.com> (raw)
In-Reply-To: <2471679.QqfNXX16uU@excalibur>

On 27/04/20 17:09 +0200, Matthias Kretz wrote:
>
>From: Matthias Kretz <kretz@kde.org>
>
>        PR libstdc++/84949
>        * include/std/limits: Let is_iec559 reflect whether
>        __GCC_IEC_559 says float and double support IEEE 754-2008.
>        * testsuite/18_support/numeric_limits/is_iec559.cc: Test IEC559
>        mandated behavior if is_iec559 is true.
>        * testsuite/18_support/numeric_limits/infinity.cc: Only test inf
>        behavior if is_iec559 is true, otherwise there is no guarantee
>        how arithmetic on inf behaves.
>        * testsuite/18_support/numeric_limits/quiet_NaN.cc: ditto for
>        NaN.
>        * testsuite/18_support/numeric_limits/denorm_min-1.cc: Compile
>        with -ffast-math.
>        * testsuite/18_support/numeric_limits/epsilon-1.cc: ditto.
>        * testsuite/18_support/numeric_limits/infinity-1.cc: ditto.
>        * testsuite/18_support/numeric_limits/is_iec559-1.cc: ditto.
>        * testsuite/18_support/numeric_limits/quiet_NaN-1.cc: ditto.

I'm inclined to go ahead and commit this (to master only, obviously).
It certainly seems more correct to me, and we'll probably never find
out if it's "safe" to do unless we actually change it and see what
happens.

Marc, do you have an opinion?

>---
> libstdc++-v3/include/std/limits               |  9 ++--
> .../18_support/numeric_limits/denorm_min-1.cc |  2 +
> .../18_support/numeric_limits/epsilon-1.cc    |  2 +
> .../18_support/numeric_limits/infinity-1.cc   |  2 +
> .../18_support/numeric_limits/infinity.cc     |  4 +-
> .../18_support/numeric_limits/is_iec559-1.cc  |  2 +
> .../18_support/numeric_limits/is_iec559.cc    | 44 ++++++++++++++-----
> .../18_support/numeric_limits/quiet_NaN-1.cc  |  2 +
> .../18_support/numeric_limits/quiet_NaN.cc    |  5 ++-
> 9 files changed, 51 insertions(+), 21 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/18_support/numeric_limits/
>denorm_min-1.cc
> create mode 100644 libstdc++-v3/testsuite/18_support/numeric_limits/
>epsilon-1.cc
> create mode 100644 libstdc++-v3/testsuite/18_support/numeric_limits/
>infinity-1.cc
> create mode 100644 libstdc++-v3/testsuite/18_support/numeric_limits/
>is_iec559-1.cc
> create mode 100644 libstdc++-v3/testsuite/18_support/numeric_limits/
>quiet_NaN-1.cc
>

>diff --git a/libstdc++-v3/include/std/limits b/libstdc++-v3/include/std/limits
>index 898406f91ee..c27350c9ec4 100644
>--- a/libstdc++-v3/include/std/limits
>+++ b/libstdc++-v3/include/std/limits
>@@ -1714,8 +1714,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       static _GLIBCXX_CONSTEXPR float
>       denorm_min() _GLIBCXX_USE_NOEXCEPT { return __FLT_DENORM_MIN__; }
> 
>-      static _GLIBCXX_USE_CONSTEXPR bool is_iec559
>-	= has_infinity && has_quiet_NaN && has_denorm == denorm_present;
>+      static _GLIBCXX_USE_CONSTEXPR bool is_iec559 = __GCC_IEC_559 >= 2;
>       static _GLIBCXX_USE_CONSTEXPR bool is_bounded = true;
>       static _GLIBCXX_USE_CONSTEXPR bool is_modulo = false;
> 
>@@ -1789,8 +1788,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       static _GLIBCXX_CONSTEXPR double
>       denorm_min() _GLIBCXX_USE_NOEXCEPT { return __DBL_DENORM_MIN__; }
> 
>-      static _GLIBCXX_USE_CONSTEXPR bool is_iec559
>-	= has_infinity && has_quiet_NaN && has_denorm == denorm_present;
>+      static _GLIBCXX_USE_CONSTEXPR bool is_iec559 = __GCC_IEC_559 >= 2;
>       static _GLIBCXX_USE_CONSTEXPR bool is_bounded = true;
>       static _GLIBCXX_USE_CONSTEXPR bool is_modulo = false;
> 
>@@ -1864,8 +1862,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       static _GLIBCXX_CONSTEXPR long double
>       denorm_min() _GLIBCXX_USE_NOEXCEPT { return __LDBL_DENORM_MIN__; }
> 
>-      static _GLIBCXX_USE_CONSTEXPR bool is_iec559
>-	= has_infinity && has_quiet_NaN && has_denorm == denorm_present;
>+      static _GLIBCXX_USE_CONSTEXPR bool is_iec559 = __GCC_IEC_559 >= 2;
>       static _GLIBCXX_USE_CONSTEXPR bool is_bounded = true;
>       static _GLIBCXX_USE_CONSTEXPR bool is_modulo = false;
> 
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/denorm_min-1.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/denorm_min-1.cc
>new file mode 100644
>index 00000000000..8ff2950d77e
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/denorm_min-1.cc
>@@ -0,0 +1,2 @@
>+// { dg-options "-ffast-math" }
>+#include "denorm_min.cc"
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/epsilon-1.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/epsilon-1.cc
>new file mode 100644
>index 00000000000..34548976bea
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/epsilon-1.cc
>@@ -0,0 +1,2 @@
>+// { dg-options "-ffast-math" }
>+#include "epsilon.cc"
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/infinity-1.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/infinity-1.cc
>new file mode 100644
>index 00000000000..7ff8ea81242
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/infinity-1.cc
>@@ -0,0 +1,2 @@
>+// { dg-options "-ffast-math" }
>+#include "infinity.cc"
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/infinity.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/infinity.cc
>index 3a5bce57c35..9585ab38216 100644
>--- a/libstdc++-v3/testsuite/18_support/numeric_limits/infinity.cc
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/infinity.cc
>@@ -33,10 +33,10 @@ test_infinity()
> {
>   bool test;
> 
>-  if (std::numeric_limits<T>::has_infinity)
>+  if (std::numeric_limits<T>::has_infinity && std::numeric_limits<T>::is_iec559)
>     {
>       T inf = std::numeric_limits<T>::infinity();
>-      test = (inf + inf == inf);
>+      test  = (inf + inf == inf);
>     }
>   else
>     test = true;
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559-1.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559-1.cc
>new file mode 100644
>index 00000000000..91dd8163126
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559-1.cc
>@@ -0,0 +1,2 @@
>+// { dg-options "-ffast-math" }
>+#include "is_iec559.cc"
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559.cc
>index d15ae5cae14..c71da192d1d 100644
>--- a/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559.cc
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/is_iec559.cc
>@@ -27,28 +27,50 @@
> #include <cwchar>
> #include <testsuite_hooks.h>
> 
>+// make these values "invisible" to the optimizer so that we really test the FPU
>+template <typename T>
>+T min = std::numeric_limits<T>::min();
>+template <typename T>
>+T denorm_min = std::numeric_limits<T>::denorm_min();
>+template <typename T>
>+T inf = std::numeric_limits<T>::infinity();
>+template <typename T>
>+T max = std::numeric_limits<T>::max();
>+template <typename T>
>+T qnan = std::numeric_limits<T>::quiet_NaN();
>+
> template<typename T>
> void
> test_is_iec559()
> {
>-  bool test;
>-
>   if (std::numeric_limits<T>::is_iec559)
>     {
>       // IEC 559 requires all of the following.
>-      test = (std::numeric_limits<T>::has_infinity
>-	      && std::numeric_limits<T>::has_quiet_NaN
>-	      && std::numeric_limits<T>::has_signaling_NaN);
>+      VERIFY(std::numeric_limits<T>::has_infinity &&
>+	     std::numeric_limits<T>::has_quiet_NaN &&
>+	     std::numeric_limits<T>::has_signaling_NaN);
>+
>+      // Test a few random IEC559 features
>+      VERIFY(min<T> / 2 > T());         // min/2 is denormal, but not 0
>+      VERIFY(denorm_min<T> / 3 == T()); // calculation with denormals works
>+      T x = denorm_min<T>;
>+      for (int i = 1; i < std::numeric_limits<T>::digits; ++i)
>+	x *= 2;
>+      VERIFY(x == min<T>);
>+      VERIFY(inf<T> * 2 == inf<T>); // infinity saturates
>+      VERIFY(max<T> * 2 == inf<T>); // max*2 saturates to infinity
>+      T inf0 = inf<T> * T();        // NaN
>+      VERIFY(inf0 != inf<T>);
>+      VERIFY(inf0 != 0);
>+      VERIFY((inf0 == inf0) == false);
>+      VERIFY(qnan<T> != qnan<T>);
>     }
>   else
>     {
>-      // If we had all of the following, why didn't we set IEC 559?
>-      test = (!std::numeric_limits<T>::has_infinity
>-	      || !std::numeric_limits<T>::has_quiet_NaN
>-	      || !std::numeric_limits<T>::has_signaling_NaN);
>+      // We could have representations for inf, NaN, and SNaN and still not be
>+      // IEC559 compliant. At this point, the meaning of operations on NaNs and
>+      // infinities is unspecified.
>     }
>-
>-  VERIFY (test);
> }
> 
> // libstdc++/8949
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN-1.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN-1.cc
>new file mode 100644
>index 00000000000..45bef204a65
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN-1.cc
>@@ -0,0 +1,2 @@
>+// { dg-options "-ffast-math" }
>+#include "quiet_NaN.cc"
>diff --git a/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN.cc b/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN.cc
>index 97406aed2b3..b17df1cc3d7 100644
>--- a/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN.cc
>+++ b/libstdc++-v3/testsuite/18_support/numeric_limits/quiet_NaN.cc
>@@ -33,10 +33,11 @@ test_qnan()
> {
>   bool test;
> 
>-  if (std::numeric_limits<T>::has_quiet_NaN)
>+  if (std::numeric_limits<T>::has_quiet_NaN &&
>+      std::numeric_limits<T>::is_iec559)
>     {
>       T nan = std::numeric_limits<T>::quiet_NaN();
>-      test = (nan != nan);
>+      test  = (nan != nan);
>     }
>   else
>     test = true;


  reply	other threads:[~2020-05-21 14:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-27 15:09 Matthias Kretz
2020-05-21 14:24 ` Jonathan Wakely [this message]
2020-05-21 15:46   ` Marc Glisse
2020-05-21 15:58     ` Jonathan Wakely
2020-05-22  7:49     ` Matthias Kretz
2020-05-22 16:39       ` Jonathan Wakely
2020-05-25  8:06         ` Matthias Kretz

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=20200521142449.GE2678@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    --cc=m.kretz@gsi.de \
    /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).