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

[Warren Ristow via Phabricator via llvm-commits]

wristow added a comment.

About:

>> This is a bit of an oddity in our handling.
> 
> Yes it is!
> 
> This is certainly getting more complicated than I had originally expected. I feel there isn't a clear spec on what we want in terms of whether FMA should be enabled "automatically" at (for example) '-O2', and/or whether it should be enabled by -ffast-math. I'm very willing to make whatever change is needed here, to meet whatever spec we all ultimately agree on.
> 
> So I think this patch should be put on hold until we decide on these wider aspects.

Thinking about this a bit more, one thing that I believe there //is// a clear spec on, is that when `-ffp-contract=off` is "the last switch related to FMA", then FMA ought to be disabled.  So for example, `-ffast-math -ffp-contract=off` should result in FMA being disabled, //irrrspective// of how we decide on the "wider aspects" of the spec mentioned above.  (FTR, this inability to enable FastMath in general, but explicitly disable FMA, was a problem one of our customers reported, which is what initially motivated this patch.)

Note that the points about the "oddity" of "-ffp-contract=fast" being implied by the two long sets of switches in this comment <https://reviews.llvm.org/D72675#1839950> are unrelated to the change proposed here.  That is, that oddity is the current behavior of Clang.

So one approach we could take is to first fix the problem of when `-ffp-contract=off` is the "the last switch related to FMA", it isn't disabling FMA in some cases.  And then as a followup, address the oddities that have been noted here.  If we decide to take this two-step approach, I'm happy to take on the followup work (once we decide what the spec should be).


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

https://reviews.llvm.org/D72675