* [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
@ 2020-04-27 15:09 Matthias Kretz
2020-05-21 14:24 ` Jonathan Wakely
0 siblings, 1 reply; 7+ messages in thread
From: Matthias Kretz @ 2020-04-27 15:09 UTC (permalink / raw)
To: gcc-patches; +Cc: libstdc++
[-- 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;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
2020-04-27 15:09 [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math Matthias Kretz
@ 2020-05-21 14:24 ` Jonathan Wakely
2020-05-21 15:46 ` Marc Glisse
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2020-05-21 14:24 UTC (permalink / raw)
To: Matthias Kretz; +Cc: gcc-patches, libstdc++
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;
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
2020-05-21 14:24 ` Jonathan Wakely
@ 2020-05-21 15:46 ` Marc Glisse
2020-05-21 15:58 ` Jonathan Wakely
2020-05-22 7:49 ` Matthias Kretz
0 siblings, 2 replies; 7+ messages in thread
From: Marc Glisse @ 2020-05-21 15:46 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Matthias Kretz, gcc-patches, libstdc++
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
2020-05-21 15:46 ` Marc Glisse
@ 2020-05-21 15:58 ` Jonathan Wakely
2020-05-22 7:49 ` Matthias Kretz
1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2020-05-21 15:58 UTC (permalink / raw)
To: libstdc++; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
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
1 sibling, 1 reply; 7+ messages in thread
From: Matthias Kretz @ 2020-05-22 7:49 UTC (permalink / raw)
To: Jonathan Wakely, libstdc++; +Cc: gcc-patches
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
──────────────────────────────────────────────────────────────────────────
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
2020-05-22 7:49 ` Matthias Kretz
@ 2020-05-22 16:39 ` Jonathan Wakely
2020-05-25 8:06 ` Matthias Kretz
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2020-05-22 16:39 UTC (permalink / raw)
To: Matthias Kretz; +Cc: libstdc++, gcc-patches
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.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math
2020-05-22 16:39 ` Jonathan Wakely
@ 2020-05-25 8:06 ` Matthias Kretz
0 siblings, 0 replies; 7+ messages in thread
From: Matthias Kretz @ 2020-05-25 8:06 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: libstdc++, gcc-patches
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
──────────────────────────────────────────────────────────────────────────
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-25 8:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-27 15:09 [PATCH] Let numeric_limits::is_iec559 reflect -ffast-math Matthias Kretz
2020-05-21 14:24 ` Jonathan Wakely
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
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).