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