[RFC] Clarify -ffinite-math-only documentation

Tue Apr 28 07:21:38 GMT 2020

On Mon, Apr 27, 2020 at 11:26 PM Matthias Kretz <m.kretz@gsi.de> wrote:
>
> On Montag, 27. April 2020 21:39:17 CEST Richard Sandiford wrote:
> > "Dr. Matthias Kretz" <m.kretz@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.kretz@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);
extern bool HONOR_NANS (const_tree);
extern bool HONOR_NANS (const_rtx);

/* Like HONOR_NANs, but true if we honor signaling NaNs (or sNaNs).  */
extern bool HONOR_SNANS (machine_mode);
extern bool HONOR_SNANS (const_tree);
extern bool HONOR_SNANS (const_rtx);

/* As for HONOR_NANS, but true if the mode can represent infinity and
   the treatment of infinite values is important.  */
extern bool HONOR_INFINITIES (machine_mode);
extern bool HONOR_INFINITIES (const_tree);
extern bool HONOR_INFINITIES (const_rtx);

/* Like HONOR_NANS, but true if the given mode distinguishes between
   positive and negative zero, and the sign of zero is important.  */
extern bool HONOR_SIGNED_ZEROS (machine_mode);
extern bool HONOR_SIGNED_ZEROS (const_tree);
extern bool HONOR_SIGNED_ZEROS (const_rtx);

note how the comment says "treatment of NaN operands is important"
and the example of simplifying x * 0 to 0 is about preserving NaNs
during expression simplification when the FPU would.  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.  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.

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

Richard.

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