[PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Matthias Kretz]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]


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


[-- Attachment #2: 0002-Let-numeric_limits-is_iec559-reflect-ffast-math.patch --]
[-- Type: text/x-patch, Size: 7125 bytes --]

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;

Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Jonathan Wakely]

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;

Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Marc Glisse]

On Thu, 21 May 2020, Jonathan Wakely wrote:

> 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?

I don't have a strong opinion on this. I thought we were refraining from 
changing numeric_limits based on flags (like -fwrapv for modulo) because 
that would lead to ODR violations when people link objects compiled with 
different flags. There is a value in libstdc++.so, which may have been 
compiled with different flags than the application.

Also, IIRC part of the effect of -ffast-math is at link time (linking some 
object that enables flush-to-zero). Anyway, as discussed in the PR, what 
numeric_limits says here is not very meaningful, and users can't rely on 
it 100%.

By default, numeric_limits gives yes if IEC support exists on the 
platform. The change would sometimes make it say no, when we know for sure 
that this support is not enabled at the beginning of the translation unit. 
Why not...

-- 
Marc Glisse

Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Jonathan Wakely]

On 21/05/20 17:46 +0200, Marc Glisse wrote:
>On Thu, 21 May 2020, Jonathan Wakely wrote:
>
>>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?
>
>I don't have a strong opinion on this. I thought we were refraining 
>from changing numeric_limits based on flags (like -fwrapv for modulo) 
>because that would lead to ODR violations when people link objects 
>compiled with different flags. There is a value in libstdc++.so, which 
>may have been compiled with different flags than the application.
>
>Also, IIRC part of the effect of -ffast-math is at link time (linking 
>some object that enables flush-to-zero). Anyway, as discussed in the 
>PR, what numeric_limits says here is not very meaningful, and users 
>can't rely on it 100%.
>
>By default, numeric_limits gives yes if IEC support exists on the 
>platform. The change would sometimes make it say no, when we know for 
>sure that this support is not enabled at the beginning of the 
>translation unit. Why not...

Good point about ODR violations.

Maybe we should just let numeric_limits fade away and be irrelevant,
and replace it with something better which can be more useful.

Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Matthias Kretz]

On Donnerstag, 21. Mai 2020 17:46:01 CEST Marc Glisse wrote:
> On Thu, 21 May 2020, Jonathan Wakely wrote:
> > 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?
> 
> I don't have a strong opinion on this. I thought we were refraining from
> changing numeric_limits based on flags (like -fwrapv for modulo) because
> that would lead to ODR violations when people link objects compiled with
> different flags. There is a value in libstdc++.so, which may have been
> compiled with different flags than the application.

But these ODR violations happen in any case: The floating-point types are 
different types with or without -ffast-math (and related) flags. They behave 
differently. Compiling a function in multiple TUs with different flags 
produces observably different results. Choosing a single one of them is 
obviously fragile and broken. That's the spirit of an ODR violation...

It would sometimes be useful to have different types:
float, float_no_nan, float_no_nan_no_signed_zero, ... 

-- 
──────────────────────────────────────────────────────────────────────────
 Dr. Matthias Kretz                           https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
 std::experimental::simd              https://github.com/VcDevel/std-simd
──────────────────────────────────────────────────────────────────────────

Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Jonathan Wakely]

On 22/05/20 09:49 +0200, Matthias Kretz wrote:
>On Donnerstag, 21. Mai 2020 17:46:01 CEST Marc Glisse wrote:
>> On Thu, 21 May 2020, Jonathan Wakely wrote:
>> > 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?
>>
>> I don't have a strong opinion on this. I thought we were refraining from
>> changing numeric_limits based on flags (like -fwrapv for modulo) because
>> that would lead to ODR violations when people link objects compiled with
>> different flags. There is a value in libstdc++.so, which may have been
>> compiled with different flags than the application.
>
>But these ODR violations happen in any case: The floating-point types are
>different types with or without -ffast-math (and related) flags. They behave
>differently. Compiling a function in multiple TUs with different flags
>produces observably different results. Choosing a single one of them is
>obviously fragile and broken. That's the spirit of an ODR violation...
>
>It would sometimes be useful to have different types:
>float, float_no_nan, float_no_nan_no_signed_zero, ...

Sure. There are ODR violations like that, and then there are ones
like:

   template<typename T>
   struct X
   {
     conditional_t<numeric_limits<T>::is_iec559, T, BigNum> val;
   };

I'm generally not concerned about ODR violations where one TU behaves
as requested by the flags used to compile that TU and another behaves
as requested by the flats used to compile that second TU. That happens
all the time with -fno-exceptions and -fno-rtti and such like. That
causes ODR violations too, but of the kind where each definition does
what was requested.

Constants defined by the library changing value is a bit more
concerning. But I don't know if it's really a problem in this case.

Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math

[Matthias Kretz]

On Freitag, 22. Mai 2020 18:39:42 CEST Jonathan Wakely wrote:
> On 22/05/20 09:49 +0200, Matthias Kretz wrote:
> >On Donnerstag, 21. Mai 2020 17:46:01 CEST Marc Glisse wrote:
> >> On Thu, 21 May 2020, Jonathan Wakely wrote:
> >> > 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?
> >> 
> >> I don't have a strong opinion on this. I thought we were refraining from
> >> changing numeric_limits based on flags (like -fwrapv for modulo) because
> >> that would lead to ODR violations when people link objects compiled with
> >> different flags. There is a value in libstdc++.so, which may have been
> >> compiled with different flags than the application.
> >
> >But these ODR violations happen in any case: The floating-point types are
> >different types with or without -ffast-math (and related) flags. They
> >behave differently. Compiling a function in multiple TUs with different
> >flags produces observably different results. Choosing a single one of them
> >is obviously fragile and broken. That's the spirit of an ODR violation...
> >
> >It would sometimes be useful to have different types:
> >float, float_no_nan, float_no_nan_no_signed_zero, ...
> 
> Sure. There are ODR violations like that, and then there are ones
> like:
> 
>    template<typename T>
>    struct X
>    {
>      conditional_t<numeric_limits<T>::is_iec559, T, BigNum> val;
>    };

Nice. ;-) If only the mangling of a struct could include the type of its 
members (recursively)... But at least val has a different type now. And 
correctly so. Yes, the ABI breaks possible via this change is real, though 
I'd guess there are zero or close-to-zero ABI dependencies on is_iec559 out in 
the wild (at this point - because it didn't work anyway).

> I'm generally not concerned about ODR violations where one TU behaves
> as requested by the flags used to compile that TU and another behaves
> as requested by the flats used to compile that second TU. That happens
> all the time with -fno-exceptions and -fno-rtti and such like. That
> causes ODR violations too, but of the kind where each definition does
> what was requested.

I am concerned. Showcase: https://godbolt.org/z/KzM3si. If you link those TUs, 
you get one of the two behaviors for both TUs. This can result in very hard to 
find Heisenbugs.

> Constants defined by the library changing value is a bit more
> concerning. But I don't know if it's really a problem in this case.

template <typename T, bool = numeric_limits<T>::is_iec559>
struct Float
{
  T val
};

Finally, the standard mechanism that can help resolve those silent ODR 
violations works. I.e. one can build float_559 and float_non559 types 
(overloading all operators is still rather tedious)

-- 
──────────────────────────────────────────────────────────────────────────
 Dr. Matthias Kretz                           https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research               https://gsi.de
 std::experimental::simd              https://github.com/VcDevel/std-simd
──────────────────────────────────────────────────────────────────────────