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

[Matthias Kretz <m.kretz@gsi.de>]

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