[PATCH] D72675: [Clang][Driver] Fix -ffast-math/-ffp-contract interaction

[Warren Ristow via Phabricator via llvm-commits]

wristow added a comment.

Revisiting this patch.  I think that before fixing the `-ffp-contract=off` problem I originally raised here, there are two questions that have come up in this discussion that we should first resolve.  Specifically:

(1)  Do we want to enable FMA transformations by default (at appropriate optimization levels), like GCC does?  That is, should FMA be done for targets that support it with a command as simple as the following?

  clang -O2 -c test.c

FTR, currently we don't do FMA with simply `-O2`.  We do enable FMA if `-ffast-math` is added, or (of course) if FMA is explicitly enabled via `-ffp-contract=fast`.  Also note that with GCC, FMA is completely orthogonal to `-ffast-math`.  For example:

  gcc -O2 -ffp-contract=off -ffast-math test.c  # '-ffast-math' does not turn FMA "back on"

(2)  Under what conditions do we want to predefine the `__FAST_MATH__` macro?  That is, should we continue with our current policy (essentially, define it when "all the fast-math-flags are enabled"), or do we want to do precisely what GCC does (define it when some particular subset of the fast-math-flags are enabled)?

My vote for (1) is yes.  To add to this, I would make the `-ffp-contract` setting independent of `-ffast-math`, as GCC does.  But to be explicit, enabling FMA at `-O2` is a change in behavior that may be unexpected for some users -- so I can imagine some users objecting to this change.

My vote for (2) is to keep our current behavior.  This approach seems more sensible to me than the GCC behavior.  And we're not boxing ourselves into anything by keeping the current behavior (that is, we can just as easily change this in the future if we find the GCC behavior is better for some reason).  Among other things, this means that whether we define `__FAST_MATH__` will continue to not be impacted by the `-ffp-contract` setting -- that makes sense to me, especially if we make FMA orthogonal to `-ffast-math`, as discussed in (1).

One related point.  Our documentation (UsersManual.rst) for `-ffp-model=precise` says:

//Disables optimizations that are not value-safe on floating-point data, although FP contraction (FMA) is enabled (``-ffp-contract=fast``).  This is the default behavior.//

So this explicitly says that FMA is enabled by default (consistent with GCC in this regard).  However, that doesn't match our implementation, as we have FMA disabled by default, as noted in this discussion.  Concretely, the following two commands are not equivalent:

  clang -c -O2 test.c                         # FMA is disabled
  clang -c -O2 -ffp-model=precise test.c      # FMA is enabled

If we decide to enable FMA by default (that is, if we agree on "yes" for (1)), then the current `-ffp-model=precise` description will become correct.  But if we decide to continue to disable FMA by default, then we'll need to change the above description (possibly add another value to `-ffp-model=<value>` that will //really // be the default).


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

https://reviews.llvm.org/D72675