On Tue, Apr 28, 2020 at 10:15 AM Matthias Kretz <m.kr...@gsi.de> wrote:
>
> On Dienstag, 28. April 2020 09:21:38 CEST Richard Biener wrote:
> > On Mon, Apr 27, 2020 at 11:26 PM Matthias Kretz <m.kr...@gsi.de> wrote:
> > > On Montag, 27. April 2020 21:39:17 CEST Richard Sandiford wrote:
> > > > "Dr. Matthias Kretz" <m.kr...@gsi.de> writes:
> > > > > On Montag, 27. April 2020 18:59:08 CEST Richard Sandiford wrote:
> > > > >> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > > > >> > On Mon, Apr 27, 2020 at 6:09 PM Matthias Kretz <m.kr...@gsi.de>
> wrote:
> > > > >> >> Hi,
> > > > >> >>
> > > > >> >> This documentation change clarifies the effect of
> > > > >> >> -ffinite-math-only.
> > > > >> >> With the current documentation, it is unclear what the presence of
> > > > >> >> NaN
> > > > >> >> and Inf representations means if (arithmetic) operations on such
> > > > >> >> values
> > > > >> >> are unspecified and even classification functions like isnan are
> > > > >> >> unreliable. If the hardware thinks a certain bit pattern is a NaN,
> > > > >> >> but
> > > > >> >> the software assumes a NaN value cannot ever exist, it is
> > > > >> >> questionable
> > > > >> >> whether, from a language viewpoint, a representation for NaNs
> > > > >> >> really
> > > > >> >> exists. Because, a NaN is defined by its behavior. This change
> > > > >> >> also
> > > > >> >> clarifies that isnan(nan) returning false is fine.
> > > > >> >>
> > > > >> >> This relates to PR84949.
> > > > >> >>
> > > > >> >> * doc/invoke.texi: Clarify the effects of
> > > > >> >> -ffinite-math-only.
> > > > >> >>
> > > > >> >> ---
> > > > >> >>
> > > > >> >> gcc/doc/invoke.texi | 6 ++++--
> > > > >> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >> >>
> > > > >> >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > >> >> index a37a2ee9c19..9e76ab057a9 100644
> > > > >> >> --- a/gcc/doc/invoke.texi
> > > > >> >> +++ b/gcc/doc/invoke.texi
> > > > >> >> @@ -11619,8 +11619,10 @@ The default is
> > > > >> >> @option{-fno-reciprocal-math}.
> > > > >> >>
> > > > >> >> @item -ffinite-math-only
> > > > >> >> @opindex ffinite-math-only
> > > > >> >>
> > > > >> >> -Allow optimizations for floating-point arithmetic that assume
> > > > >> >> -that arguments and results are not NaNs or +-Infs.
> > > > >> >> +Assume that floating-point types in the language do not have
> > > > >> >> representations for
> > > > >> >> +NaNs and +-Inf. Whether floating-point hardware supports and acts
> > > > >> >> on
> > > > >> >> NaNs and ++-Inf is not affected. The behavior of a program that
> > > > >> >> uses a
> > > > >> >> NaN or +-Inf value
> > > > >> >> +as function argument, macro argument, or operand is undefined.
> > > > >> >
> > > > >> > Minor nit here - I'd avoid the 'undefined' word which has bad
> > > > >> > connotation
> > > > >> > and use 'unspecified'. Maybe we can even use ISO C language
> > > > >> > specification
> > > > >> > terms but I'm not sure which one is most appropriate here.
> > > > >
> > > > > I'm an ISO C++ person, and unspecified sounds too reliable to me:
> > > > > https://wg21.link/intro.defs#defns.unspecified.
> > > > >
> > > > >> > Curiously __builtin_nan ("nan") still gets you a NaN representation
> > > > >> > but isnan(__builtin_nan("nan")) is resolved to false.
> > > > >
> > > > > Right, that's because only the hardware thinks __builtin_nan ("nan")
> > > > > is a
> > > > > NaN representation. With -ffinite-math-only, the double data type in
> > > > > C/C++ can either hold a finite real value, or an invalid value (i.e. a
> > > > > value that the optimizer unconditionally excludes as a possible value
> > > > > for
> > > > > any object of floating-point type). FWIW, with -ffinite-math-only,
> > > > > ubsan
> > > > > should flag isnan(__builtin_nan("nan")) or any f(constexpr nan).
> > > > >
> > > > > With the above documentation change, it is clear that with
> > > > > https://wg21.link/ P1841 std::numbers::quiet_NaN<float> would be
> > > > > ill-formed under -ffinite-math- only. Without the documentation
> > > > > change,
> > > > > it can be argued either way.
> > > > >
> > > > > There's another interesting observation resulting from the above:
> > > > > double
> > > > > and double under -ffinite-math-only are different types. Any function
> > > > > call from one world to the other is dangerous. Inline functions
> > > > > translated in different TUs compiled with different math flags violate
> > > > > the ODR. But that's all the more reason to have a very precise
> > > > > documentation/understanding of what -ffinite-math-only does. Because
> > > > > this
> > > > > gotcha is already the status quo.>
> > > > >
> > > > >> Yeah, for that and other reasons, I think it would be good to avoid
> > > > >> giving the impression that -ffinite-math-only can be relied on to
> > > > >> make
> > > > >> the assumption above. Wouldn't it be more accurate to say that the
> > > > >> compiler is allowed to make the assumption, at any point that it
> > > > >> seems
> > > > >> convenient?
> > > > >
> > > > > I think undefined behavior does what you're asking for while
> > > > > unspecified
> > > > > behavior does what you want to avoid. I.e. its an undocumented
> > > > > behavior,
> > > > > but it can be relied on with a given implementation (compiler).
> > > >
> > > > Although it wasn't obvious from my quoting, I was worried more about
> > > > the wording of the first "Assume X" sentence than the unspecified vs.
> > > > undefined thing. I think saying "Assume X" loses an important but
> > > > correct nuance of the original wording: that completely ignoring the
> > > > option would be a valid (if not very useful) implementation. "Assume X"
> > > > instead makes it sound like the compiler is required to do something
> > > > (at least to me).
> > >
> > > Fair point. However, I want to make it clear what value to expect from
> > > std::numeric_limits<fp-type>::has_quiet_NaN. Currently that's up for
> > > interpretation. Reading C17 5.2.4.2.2/4, which defines quiet NaNs to have
> > > a
> > > certain behavior, I believe has_quiet_NaN has always had to be false with
> > > -ffinite-math-only. Anyway, only talking about more freedom for the
> > > optimizer is not enough to make that clarification.
> > >
> > > I believe my suggested wording is not incorrect even in the case where the
> > > compiler acts like -ffinite-math-only was not used. Because interpreting
> > > the "invalid values" as IEC559 NaNs and Infs is one possible behavior
> > > that can result from undefined behavior.
> > >
> > > > E.g., does making it undefined behaviour mean that:
> > > > constexpr bool e = __builtin_isnan(__builtin_nan("nan"));
> > > >
> > > > must be rejected for -ffinite-math-only, since AIUI constexprs aren't
> > > > allowed to invoke UB? (Sorry if this has already been covered in the
> > > > earlier conversation.)
> > >
> > > Nit: __builtin_nan("nan") isn't a constant expression. __builtin_nan("")
> > > is, though. numeric_limits<fp-type>::quiet_NaN() is implemented via
> > > __builtin_nan("").
> > > https://wg21.link/numeric.limits.members#lib:numeric_limits,quiet_NaN says
> > > quiet_NaN is not "meaningful" if has_quiet_NaN == false. That's no
> > > guidance.
> > >
> > > If my documentation change means to say C++ UB (which I intended), then
> > > yes, both
> > >
> > > constexpr bool e = __builtin_isnan(__builtin_nan(""));
> > > constexpr auto x = __builtin_inf() + 1;
> > >
> > > have to be ill-formed. That sounds like a reasonable feature: Asking for a
> > > NaN/Inf value while saying the program (well, TU) only does finite math is
> > > an inconsistency that should at least be warned about, if not be
> > > ill-formed.
> > >
> > > I believe I'm not changing the status quo (of the documentation), only
> > > clarifying the consequences. However, since GCC does not implement all
> > > that
> > > freedom yet, we might want to call out more details in the documentation.
> > >
> > > Some more random thoughts:
> > >
> > > * The builtins don't have to return NaN and Inf (i.e. invalid values that
> > > invoke UB on use). The current documentation gives the freedom under
> > > -ffinite- math-only to assume any return value is not NaN/Inf.
> > > Consequently translating __builtin_nan and __builtin_inf right now as 0
> > > seems plausible.
> > >
> > > * The documentation could expand on return values and say that *if the
> > > value of an expression is not a finite value it is an unspecified value*.
> > > At least for the non-constexpr case this means: "we don't know whether
> > > `__builtin_inf() + 1` is UB; it could be 1; it could be 2; let's see what
> > > the FPU makes of it". This important because UB allows to imply
> > > `__builtin_unreachable()`.
> > >
> > > * Why not disable NaN and Inf independently? Inf is just a reciprocal 0.
> > > Inf is as far away from numeric_limits::max as 0 is from
> > > numeric_limits::min (infinitely many steps on a floating-point "type"
> > > with infinite exponent).
> >
> > Just a comment on the last point - internally it's already split into
> > HONOR_NANS and HONOR_INFINITIES that there's just
> > a single user-facing option -ffinite-math-only is historical (IMHO more
> > options are more opportunities for users to get confused ;)).
> >
> > Maybe also interesting to the discussion the internal comments say
> >
> > /* True if the given mode has a NaN representation and the treatment of
> > NaN operands is important. Certain optimizations, such as folding
> > x * 0 into 0, are not correct for NaN operands, and are normally
> > disabled for modes with NaNs. The user can ask for them to be
> > done anyway using the -funsafe-math-optimizations switch. */
> > extern bool HONOR_NANS (machine_mode);
> > [...]
> > note how the comment says "treatment of NaN operands is important"
>
> ... if HONOR_NANS(mode) == true. So with -ffinite-math-only that implies
> "treatment of NaN operands is not important", right?
>
> > and the example of simplifying x * 0 to 0 is about preserving NaNs
> > during expression simplification when the FPU would.
>
> Yes, the `nan * 0 -> 0` simplification with -ffinite-math-only is why I'm
> saying the status quo already has no NaN representation *in C/C++* anymore.
> Because a float with NaN bits doesn't behave like C17 5.2.4.2.2/4 requires.
> And thus it's an invalid/unspecified/undefined value, not a NaN. If there are
> no NaNs (as defined by C), then there is no NaN representation and
> __FLT_HAS_QUIET_NAN__ must be 0.
>
> > I think these
> > kind of optimizations are what originally was intended to be allowed
> > with -ffinite-math-only - that we started to simplify isnan(x) to false
> > was extending the scope and that change wasn't uncontested since
> > it makes -ffinite-math-only less useful to some people.
>
> I understand and agree with this point. However I think the problem with the
> current -ffinite-math-only is that it doesn't do enough. I.e. it's a dangerous
> optimization for people who rely on nan+inf in some way. But in many cases
> people don't realize they do, and it may be a latent issue that turns into a
> bug on unrelated code changes (or compiler changes). By going all the way,
> aggressively optimizing assuming there can't ever be nan/inf in the program,
> making the use of inf/nan ill-formed NDR, and integrating with ubsan, I think
> the option becomes less dangerous and more useful. That would be a bold move,
> certainly.
>
> One should be able to debug NaNs via `-ffast-math -fno-finite-math-only` or
> via FP traps and code compiled with -ffinite-math-only. I.e. whenever an
> instruction produces a NaN the CPU would still trap, independent of the
> optimization.
>
> > So there's
> > the choice to go back, possibly by doing a comprehensive audit
> > of flag_finite_math_only usage (mainly uses of HONOR_*).
> > Changing (clarifying) the documentation and changing the libstdc++
> > implementatin would make this harder.
>
> Yes. And having two options -ffinite-math-only and -freally-only-finite-math
> is out of the question. ;-)
> But you already have my opinion on which way is more useful and safer.
>
> > In inlining we for example allow inlining a function compiled with
> > -ffinite-math-only into a function compiled without (but not the
> > other way around).
>
> The problem is if an inline function is not inlined:
>
> a.h:
> inline float f(float x) {
> return x * 0;
> }
>
> b.cc (-fno-finite-math-only):
> void f0() {
> assert(isnan(f(__builtin_nan(""))));
> }
>
> c.cc (-ffinite-math-only):
> float f1() {
> return f(1.f);
> }
>
> b.o now contains a function f(float) that returns after fp mul. c.o contains a
> function f(float) that unconditionally returns 0. On linking b.o and c.o one
> of them is discarded. By realizing that -ffinite-math-only modifies the float
> type, we see that f was translated with different types and thus this is an
> ODR violation.
>
> Tangent: It'd be interesting if compiler flags that effectively modify types
> and those that change available ISA extensions were part of the mangling. In
> general, right now, programs consisting of code compiled with different
> compiler flags is full of ODR violations.
Yep, this most visibly hits people compiling some CUs with -march=XYZ
with functions whose calls are properly guarded by cpu capability checks
and unknowingly end up with template instantiations from the STL in that ...
I think we don't have some -fno-comdats or so forcing all instantiated templates
to be really private (hidden visibility doesn't help here).
But that's a really separate issue... even enumerating all options that
possibly affect things this way is hard.
Richard.
>
> --
> ──────────────────────────────────────────────────────────────────────────
> 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
> ──────────────────────────────────────────────────────────────────────────
>
>
>
