[PATCH] D72675: ix -ffast-math/-ffp-contract interaction

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.
Herald added a subscriber: wuzish.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2721
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
-      !TrappingMath)
+      !TrappingMath && !FPContractDisabled)
     CmdArgs.push_back("-menable-unsafe-fp-math");
----------------
I think that you just need

  && !FPContract.equals("off")

or

  && !(FPContract.equals("off") || FPContract.equals("on"))

of which I think the latter. fp-contract=no is also not a fast-math-compatible setting in that regard, right?


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2745
   if (!FPContract.empty())
     CmdArgs.push_back(Args.MakeArgString("-ffp-contract=" + FPContract));
 
----------------
So now we pass it twice, because we also pass it here?


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2763
   if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath &&
       ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
+    if (!FPContractDisabled)
----------------
You want the check here, I think, and not below so that if parts of fast-math are disabled `__FAST_MATH__` is not set. That seems to be what the comment/code currently do, although what does GCC do in this regard?


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

https://reviews.llvm.org/D72675