[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

In D107994#3131268 <https://reviews.llvm.org/D107994#3131268>, @zahiraam wrote:

> In D107994#3130494 <https://reviews.llvm.org/D107994#3130494>, @wristow wrote:
>
>> The Release Note change here says:
>>
>>   Floating Point Support in Clang
>>   -------------------------------
>>   - The -ffp-model=precise now implies -ffp-contract=on rather than
>>     -ffp-contract=fast, and the documentation of these features has been
>>     clarified. Previously, the documentation claimed that -ffp-model=precise was
>>     the default, but this was incorrect because the precise model implied
>>     -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
>>     -ffp-model=precise is now exactly the default mode of the compiler.
>>
>> Unless I'm missing something, there is a related change here that I think should be more overtly noted (given the discussions in this review, I //think// this additional change is understood/expected, but I'm surprised it's not pointed out explicitly -- so maybe I'm misunderstanding).
>>
>> Specifically, this commit explicitly sets `-ffp-contract=on` in the default mode (which is what the documentation said, and continues to say).  But previously, there wasn't //any// explicit setting of `-ffp-contract` by default (and I think that lack of an explicit setting, was equivalent to `-ffp-contract=off`).
>> ...
>
> @wristow Are you suggesting a change of wording in the ReleaseNotes?

Yes @zahiraam, either a modification to what was written, or an additional separate point.  The critical user-visible change from this commit (AFAICS) is that previously the default behavior was `-ffp-contract=off`, whereas now the default behavior is `-ffp-contract=on`.  The ReleaseNote as written implies that the FMA situation //was// `-ffp-contract=fast`, and it has been changed to `-ffp-contract=on`, and either of those modes would result in FMA being done for cases like:

  float foo(float a, float b, float c) {
    return a * b + c;
  }

In short, the current ReleaseNote implies that FMA would be used by default, for cases like the above (because `-ffp-contract=on` was the default) -- but this isn't true.  (The current ReleaseNote also clarifies that if `-ffp-model=precise` were explicitly specified, then it previously would set `-ffp-contract=fast` (which was not what the documentation said) and now it sets `-ffp-contract=on` (which //is// what the documentation says) -- this aspect is correct.)

I think the ReleaseNote should clarify that the result of this change is that previously, the default behavior was equivalent to `-ffp-contract=off` (and so no FMA would be done by default), but this commit makes the default behavior `-ffp-contract=on` (so FMA is enabled by default, even at `-O0`).  The documentation indicates that the default is `-ffp-contract=on`, so this change makes the compiler behavior consistent with the documentation, with respect to the `-ffp-contact` switch.  It also makes it consistent with the documentation for the `-ffp-model` switch.

A possible new wording is below (maybe this is too verbose, so making it more concise is fine too, from my POV):

  Floating Point Support in Clang
  -------------------------------
  - The default setting of FP contraction (FMA) is now -ffp-contract=on (for
    languages other than CUDA/HIP).  This is consistent with the documentation.
    Previously, the default behavior was equivalent to -ffp-contract=off, so
    the old behavior was that FMA would not be used by default for code like:
        float foo(float a, float b, float c) {
          return a * b + c;
        }
    Whereas now, FMA will be used in the above code, even when optimization
    is off.
  
    Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
    rather than -ffp-contract=fast, and the documentation of these features has
    been clarified.  Previously, the documentation claimed that
    -ffp-model=precise was the default, but this was incorrect because the
    precise model implied -ffp-contract=fast, whereas the (now corrected)
    default behavior is -ffp-contract=on.
    -ffp-model=precise is now exactly the default mode of the compiler.


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

https://reviews.llvm.org/D107994