[PATCH] D67517: Create UsersManual section entitled 'Controlling Floating Point Behavior'

[John McCall via Phabricator via llvm-commits]

rjmccall added inline comments.


================
Comment at: clang/docs/UsersManual.rst:1152
+
+  This option implies:
+
----------------
mibintc wrote:
> rjmccall wrote:
> > cameron.mcinally wrote:
> > > mibintc wrote:
> > > > rjmccall wrote:
> > > > > To be clear, fast math is not *equivalent* to just setting all of these separately, right?
> > > > I think in fact ffast-math is the same as selecting all the invidivdual settings, part of the logic checks if all the features are enabled and then also sets the fast-math macro. 
> > > Just checked for crtfastmath.o on the link line and they do not seem to be "equivalent", unless I made a mistake.
> > > 
> > > That said, I think the language "implies" is acceptable here.
> > Even if the only difference was the macro, that would be quite important — the macros can result in substantially different behavior from math libraries, IIRC.  So I would appreciate a wording that carries less of a connotation that this is equivalent to this fixed set of other options.  Maybe "Some of the individual optimizations performed by fast-math can be separately enabled or disabled with the following options:"?
> > 
> > Melanie, are you saying that `__FAST_MATH__` is defined only if *all* of these options are set?  I think that's important to cover as well; perhaps:
> > 
> >   "``-ffast-math`` causes the macro ``__FAST_MATH__`` to be defined.  Some math libraries recognize this macro and change their behavior.  Using any of the options below to disable any of the individual optimizations in ``-ffast-math`` will cause ``__FAST_MATH__`` to no longer be set."
> i made a correction to the text, the code doesn't check of -ffp-contract=fast, it checks if the other bits are set a certain way, then it pushes -ffast-math into the args. 
Thanks.  Let me suggest a reorganization of the new wording; it's currently a bit misleading because it leads with the bit about `__FAST_MATH__`, which is of course not the primary function of the option.

```
.. option:: -ffast-math

   Enable fast-math mode.  This option allows the compiler to make a set of
   aggressive assumptions about floating-point math; the resulting code may
   execute faster but may also have decreased precision or even incorrect
   behavior for corner cases such as ```-0``.  These assumptions include:

   * Floating-point math obeys regular algebraic rules for real numbers (e.g.
     ``+`` and ``*`` are associative, ``x/y == x * (1/y)``, and
     ``(a + b) * c == a * c + b * c``),
   * operands to floating-point operations are not equal to ``NaN`` and
     ``Inf``, and
   * ``+0`` and ``-0`` are interchangeable.

   ``-ffast-math`` also defines the ``__FAST_MATH__`` preprocessor
   macro. Some math libraries recognize this macro and change their behavior.
   With the exception of ``-ffp-contract=fast``, disabling any of the individual
   optimizations in ``-ffast-math`` will cause ``__FAST_MATH__`` to no longer
   be set.

   ``-ffast-math`` implies the following options:
```


================
Comment at: clang/docs/UsersManual.rst:1188
+   With the 'no-strict' option, Clang attempts to match the overflowing behavior
+   of the target's native float-to-int conversion instructions.
+
----------------
mibintc wrote:
> rjmccall wrote:
> > "According to the C standard, when a floating-point value is converted to
> > integer type, the fractional component of the value is discarded, and if the
> > resulting integer is not expressible in the destination type, the conversion
> > has undefined behavior.  By default, Clang follows this rule strictly and does
> > not guarantee any particular result.  In non-strict mode, this behavior
> > becomes well-defined, and Clang uses the standard behavior for the target:
> > typically, the behavior of the target architecture's float-to-int conversion
> > instructions."
> > 
> > Ideally this would then specify that behavior for various targets.
> i think it would be difficult to determine the behavior of the target arch float-to-int conversion instructions for all supported targets. Maybe documenting this detaill should be part of adding a new target to clang/llvm. 
I think it would good to add that documentation for the existing targets, but I don't want to hold up this patch for it, so we can leave that out.


================
Comment at: clang/docs/UsersManual.rst:1203
+
+   Form fused floating point operations (e.g. FMAs).  Valid values are:
+
----------------
mibintc wrote:
> rjmccall wrote:
> > "Specify when the compiler is permitted to form fused floating-point operations, such as fused multiply-add (FMA).  Fused operations are permitted to produce more precise results than performing the same operations separately.
> > 
> > The C standard permits intermediate floating-point results within an expression to be computed with more precision than their type would normally allow.  This permits operation fusing, and Clang takes advantage of this by default.  This behavior can be controlled with the FP_CONTRACT pragma.
> > 
> > Valid values for this option are:"
> > 
> > The second paragraph should probably describe how the pragma interacts with the option.
> I made the text changes you suggested, but i think that the interaction with the pragma should be described in the pragma section.  currently there's already information about the option in the pragma section. I'll add a sentence referring the reader to the pragma documentation.
Sure, a cross-reference is fine.


================
Comment at: clang/docs/UsersManual.rst:1264
+  but can also have significantly less precision Defaults to
+  ``-fno-reciprocal-math``.
+
----------------
Missing period.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67517/new/

https://reviews.llvm.org/D67517