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

Warren Ristow via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 27 14:52:23 PST 2020


wristow updated this revision to Diff 240689.
wristow added a comment.

Used the clearer '!off && !on' (rather than '!(off || on)') in the checks.

Added more tests that note the current situation that `-ffast-math` enables FMA. overriding an earlier switch that had disabled it (included a "TODO" comment in these tests, since it isn't clear if that behavior is what we will ultimately stick with).


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

https://reviews.llvm.org/D72675

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fast-math.c


Index: clang/test/Driver/fast-math.c
===================================================================
--- clang/test/Driver/fast-math.c
+++ clang/test/Driver/fast-math.c
@@ -180,6 +180,40 @@
 // CHECK-FAST-MATH: "-ffast-math"
 // CHECK-FAST-MATH: "-ffinite-math-only"
 //
+// -ffp-contract=off and -ffp-contract=on must disable the fast-math umbrella,
+// and the unsafe-fp-math umbrella (-ffp-contract=fast leaves them enabled).
+// Note that there is some uncertainty as to whether we want the fast-math
+// umbrella to enable FMA, but it is clear that irrespective of whether FMA
+// is enabled by fast-math, it should be disabled with a later switch.
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH %s
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNSAFE-MATH %s
+//
+// The current behavior is that the umbrella aspect of -ffast-math enables FMA.
+// As noted above, there is some uncertainty on whether this is the desired
+// behavior.  For now, we verify that the umbrella aspect does kick in to
+// enable -ffp-contract=fast mode, overriding an earlier setting that had
+// disabled FMA.
+// TODO: Decide on whether -ffast-math should enable FMA.
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FP-CONTRACT-FAST %s
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FP-CONTRACT-FAST %s
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FP-CONTRACT-FAST %s
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FP-CONTRACT-FAST %s
+// CHECK-FP-CONTRACT-FAST: "-ffp-contract=fast"
+//
 // RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-FAST-MATH %s
 // RUN: %clang -### -ffast-math -fno-finite-math-only -c %s 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2785,8 +2785,11 @@
   if (MathErrno)
     CmdArgs.push_back("-fmath-errno");
 
+  // If -ffp-contract=off/on has been specified on the command line, then we
+  // must suppress the emission of -ffast-math and -menable-unsafe-fp-math to
+  // cc1.
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
-      !TrappingMath)
+      !TrappingMath && !FPContract.equals("off") && !FPContract.equals("on"))
     CmdArgs.push_back("-menable-unsafe-fp-math");
 
   if (!SignedZeros)
@@ -2831,12 +2834,17 @@
 
   ParseMRecip(D, Args, CmdArgs);
 
-  // -ffast-math enables the __FAST_MATH__ preprocessor macro, but check for the
-  // individual features enabled by -ffast-math instead of the option itself as
-  // that's consistent with gcc's behaviour.
+  // -ffast-math acts as an "umbrella", enabling a variety of individual
+  // floating-point transformations.  We check if the appropriate set of those
+  // transformations are enabled, and if they are, we pass that umbrella switch
+  // to cc1.  This also interacts with another "umbrella" switch, -ffp-model.
+  // We handle -ffp-contract somewhat specially here, to produce a warning in
+  // situations where -ffp-model=fast is overridden by the setting of
+  // -ffp-contract.
   if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath &&
       ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
-    CmdArgs.push_back("-ffast-math");
+    if (!FPContract.equals("off") && !FPContract.equals("on"))
+      CmdArgs.push_back("-ffast-math");
     if (FPModel.equals("fast")) {
       if (FPContract.equals("fast"))
         // All set, do nothing.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72675.240689.patch
Type: text/x-patch
Size: 4391 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200127/3a61fac7/attachment.bin>


More information about the llvm-commits mailing list