[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[pinskia at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

Andrew Pinski  changed:

   What|Removed |Added

 CC||Zahira.Ammarguellat at intel 
dot c
   ||om

--- Comment #10 from Andrew Pinski  ---
*** Bug 111759 has been marked as a duplicate of this bug. ***

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[rguenth at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

Richard Biener  changed:

   What|Removed |Added

 Status|NEW |SUSPENDED

--- Comment #9 from Richard Biener  ---
I think this is a good bug to dup to since it contains good information, so
lets suspend it.

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[kretz at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

--- Comment #8 from Matthias Kretz (Vir)  ---
I've been doing a lot of research into the numeric_limits intent/meaning
recently. I also implemented and used alternative interpretations of "has NaN"
and "is IEC559". My conclusion: std::numeric_limits means "has NaN bitpattern"
and "has IEC559 bit layout" not "has NaNs with NaN behavior" and "has IEC559
behavior". The former are still useful even if the latter don't hold. The C++
standard could be clearer on this matter though.
Thus, it seems the status quo is working as intended. It's just that we're
missing a standard interface to ask for behavior conformance.

Any progress on this issue must go via WG21.

Besides: my patch in #6 was not accepted (ABI concerns: specifically ABI
breakage from using different -f... flags), so should this PR be closed?

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[kretz at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

--- Comment #7 from Matthias Kretz  ---
Example showing the discrepancy: https://godbolt.org/z/D15m71

Also PR83875 is relevant wrt. giving different answers depending on function
attributes.

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[kretz at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

Matthias Kretz  changed:

   What|Removed |Added

 CC||kretz at kde dot org

--- Comment #6 from Matthias Kretz  ---
I'd like to make a case for numeric_limits::is_iec559 to follow
__STDC_IEC_559__. I.e. the following patch:

--- a/libstdc++-v3/include/std/limits
+++ b/libstdc++-v3/include/std/limits
@@ -1649,7 +1649,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   denorm_min() _GLIBCXX_USE_NOEXCEPT { return __FLT_DENORM_MIN__; }

   static _GLIBCXX_USE_CONSTEXPR bool is_iec559
+#ifdef __STDC_IEC_559__
   = has_infinity && has_quiet_NaN && has_denorm == denorm_present;
+#else
+  = false;
+#endif
   static _GLIBCXX_USE_CONSTEXPR bool is_bounded = true;
   static _GLIBCXX_USE_CONSTEXPR bool is_modulo = false;

@@ -1724,7 +1728,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   denorm_min() _GLIBCXX_USE_NOEXCEPT { return __DBL_DENORM_MIN__; }

   static _GLIBCXX_USE_CONSTEXPR bool is_iec559
+#ifdef __STDC_IEC_559__
   = has_infinity && has_quiet_NaN && has_denorm == denorm_present;
+#else
+  = false;
+#endif
   static _GLIBCXX_USE_CONSTEXPR bool is_bounded = true;
   static _GLIBCXX_USE_CONSTEXPR bool is_modulo = false;

@@ -1799,7 +1807,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   denorm_min() _GLIBCXX_USE_NOEXCEPT { return __LDBL_DENORM_MIN__; }

   static _GLIBCXX_USE_CONSTEXPR bool is_iec559
+#ifdef __STDC_IEC_559__
   = has_infinity && has_quiet_NaN && has_denorm == denorm_present;
+#else
+  = false;
+#endif
   static _GLIBCXX_USE_CONSTEXPR bool is_bounded = true;
   static _GLIBCXX_USE_CONSTEXPR bool is_modulo = false;


The __STDC_IEC_559__ macro is not defined when -ffast-math is active. is_iec559
requires "true if and only if the type adheres to ISO/IEC/IEEE 60559". I don't
have IEC559 at hand, but I believe assuming no NaN, inf, or -0 can occur makes
the floating point types not adhere to IEC559.
And IIUC, if __STDC_IEC_559__ is defined, then has_infinity, has_quiet_NaN and
has_denorm must all be true. So
+#ifdef __STDC_IEC_559__
+  = true;
+#else
+  = false;
+#endif
should be correct.

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

--- Comment #5 from rguenther at suse dot de  ---
On Thu, 3 May 2018, redi at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949
> 
> --- Comment #4 from Jonathan Wakely  ---
> std::numeric_limits defines:
> 
>   static _GLIBCXX_USE_CONSTEXPR bool has_infinity = __FLT_HAS_INFINITY__;
>   static _GLIBCXX_USE_CONSTEXPR bool has_quiet_NaN = 
> __FLT_HAS_QUIET_NAN__;
>   static _GLIBCXX_USE_CONSTEXPR bool has_signaling_NaN = has_quiet_NaN;
>   static _GLIBCXX_USE_CONSTEXPR float_denorm_style has_denorm
> = bool(__FLT_HAS_DENORM__) ? denorm_present : denorm_absent;
>   //...
>   static _GLIBCXX_USE_CONSTEXPR bool is_iec559
> = has_infinity && has_quiet_NaN && has_denorm == denorm_present;

^^^

note that this really can't work in a way to reflect the state
of flags like -ffinite-math-only because those can be different
_per function_.  Of course then the preprocessor macros do not
reflect reality either...

bool __attribute__((optimize("finite-math-only")))
isnan1 (double x, int )
{
  a = __FINITE_MATH_ONLY__;
  return __builtin_isnan(x);
}
bool __attribute__((optimize("no-finite-math-only")))
isnan2 (double x, int )
{
  a = __FINITE_MATH_ONLY__;
  return __builtin_isnan(x);
}
int main()
{
  int a;
  __builtin_printf ("%d %d\n", isnan1(__builtin_nan(""), a), a);
  __builtin_printf ("%d %d\n", isnan2(__builtin_nan(""), a), a);
}

> g++-7 t.c
> ./a.out 
0 0
1 0
> g++-7 t.c -ffinite-math-only
> ./a.out 
0 0
1 1

note that __FLT_HAS_QUIET_NAN__ is not affected by flags.

> And that seems to be the right thing to do. If the compiler tells us the type
> has infinities and NaNs then we expose that through std::numeric_limits.
> 
> I don't think we want the C++ library to be inconsistent with the compiler
> here. So maybe any change should be in the compiler not libstdc++.

So my points in comment#1 still stand.  It might be unfortunate
(even more so since you can't actually rely on __FINITE_MATH_ONLY__)
but it's not likely to change (people have repeatedly requested
that at least isnan() continues to return true for NaNs for example).

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

--- Comment #4 from Jonathan Wakely  ---
std::numeric_limits defines:

  static _GLIBCXX_USE_CONSTEXPR bool has_infinity = __FLT_HAS_INFINITY__;
  static _GLIBCXX_USE_CONSTEXPR bool has_quiet_NaN = __FLT_HAS_QUIET_NAN__;
  static _GLIBCXX_USE_CONSTEXPR bool has_signaling_NaN = has_quiet_NaN;
  static _GLIBCXX_USE_CONSTEXPR float_denorm_style has_denorm
= bool(__FLT_HAS_DENORM__) ? denorm_present : denorm_absent;
  //...
  static _GLIBCXX_USE_CONSTEXPR bool is_iec559
= has_infinity && has_quiet_NaN && has_denorm == denorm_present;

And that seems to be the right thing to do. If the compiler tells us the type
has infinities and NaNs then we expose that through std::numeric_limits.

I don't think we want the C++ library to be inconsistent with the compiler
here. So maybe any change should be in the compiler not libstdc++.

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

Jonathan Wakely  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2018-03-19
 Ever confirmed|0   |1

--- Comment #3 from Jonathan Wakely  ---
(In reply to Christoph Lipka from comment #0)
> (B)
> std::numeric_limits::has_quiet_NaN() is broken in that it claims that
> NaNs are supported.

Agreed. 

> Seen with both g++ 4.8.4 on Ubuntu 14.04, as well as g++ 5.4.0 on MS Windows
> Subsystem for Linux

Neither of those compilers is still supported by the GCC project.

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[christ...@lipka-koeln.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

--- Comment #2 from Christoph Lipka  ---
Having dug a bit deeper, I notice another way in which NaNs are broken in
-ffinite-math-only mode:

Normally, NaNs should always compare NON-EQUAL, even when compared to itself.

In -ffinite-math-only mode, NaNs ALWAYS compare EQUAL, even when compared to a
totally different value (such as, say, 0.0).


I would disagree with the claim that "NaNs are there" in -ffinite-math-only
mode - de facto they're not. What is there is one or more representations that
cause all sorts of undefined behaviour, which just so _happens_ to include
rendering as "nan" when converted to a string.

So my point stands that std::numeric_limits::has_quiet_NaN() should return
false when -ffinite-math-only is active.

[Bug libstdc++/84949] -ffast-math bugged with respect to NaNs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84949

Richard Biener  changed:

   What|Removed |Added

 CC||rguenth at gcc dot gnu.org
  Component|c++ |libstdc++

--- Comment #1 from Richard Biener  ---
-ffast-math implies -ffinite-math-only which causes GCC to assume no inputs to
isnan/fpclassify are NaN.  As a programmer using -ffast-math you give this
guarantee to the compiler, so what you report is a pilot error?

I'm not sure if std::numeric_limits is wrong with -ffinite-math-only given
NaNs are still "supported" as in

double nan = __builtin_nan("");
int main()
{
  __builtin_printf ("%f\n", nan);
  return __builtin_isnan (nan); 
}
> g++-7 t.C  -ffast-math -O2
> ./a.out 
nan
> echo $?
0

so NaNs are there.  But in the testcase I broke the contract with the compiler
with feeding a NaN into isnan().

I guess the compiler could have as well replaced the NaN generated by
__builtin_nan("") with zero.