Fix -fno-unsafe-math-optimizations behavior

[andykaylor]

This changes the handling of -fno-unsafe-fp-math to stop having that
option imply -ftrapping-math. In gcc, -fno-unsafe-math-optimizations sets
-ftrapping-math, but that dependency is based on the fact the
-ftrapping-math is enabled by default in gcc. Because clang does not
enable -ftrapping-math by default, there is no reason for
-fno-unsafe-math-optimizations to set it.

On the other hand, -funsafe-math-optimizations continues to imply
-fno-trapping-math because this option necessarily disables strict
exception semantics.

This fixes #87523

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Andy Kaylor (andykaylor)

Changes

This changes the handling of -fno-unsafe-fp-math to stop having that
option imply -ftrapping-math. In gcc, -fno-unsafe-math-optimizations sets
-ftrapping-math, but that dependency is based on the fact the
-ftrapping-math is enabled by default in gcc. Because clang does not
enabled -ftrapping-math by default, there is no reason for
-fno-unsafe-math-optimizations to set it.

On the other hand, -funsafe-math-optimizations continues to imply
-fno-trapping-math because this option necessarily disables strict
exception semantics.

This fixes #87523

---

Full diff: https://github.com/llvm/llvm-project/pull/89473.diff

3 Files Affected:

• (modified) clang/docs/UsersManual.rst (-1)
• (modified) clang/lib/Driver/ToolChains/Clang.cpp (-2)
• (modified) clang/test/Driver/fast-math.c (+13-11)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index c464bc3a69adc5..2b4155d4b65a48 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1690,7 +1690,6 @@ floating point semantic models: precise (the default), strict, and fast.
    * ``-fno-associative-math``
    * ``-fno-reciprocal-math``
    * ``-fsigned-zeros``
-   * ``-ftrapping-math``
    * ``-ffp-contract=on``
    * ``-fdenormal-fp-math=ieee``
 
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 456ea74caadb00..0776d095327219 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3137,8 +3137,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       ReciprocalMath = false;
       SignedZeros = true;
       ApproxFunc = false;
-      TrappingMath = true;
-      FPExceptionBehavior = "strict";
 
       // The target may have opted to flush by default, so force IEEE.
       DenormalFPMath = llvm::DenormalMode::getIEEE();
diff --git a/clang/test/Driver/fast-math.c b/clang/test/Driver/fast-math.c
index 882e81fd14d34a..ef23f88dd817ea 100644
--- a/clang/test/Driver/fast-math.c
+++ b/clang/test/Driver/fast-math.c
@@ -271,11 +271,11 @@
 // RUN:   2>&1 | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 
 // RUN: %clang -### -funsafe-math-optimizations -fno-reciprocal-math -c %s \
-// RUN:   2>&1 | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN:   2>&1 | FileCheck --check-prefix=CHECK-REASSOC-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -fsigned-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -ftrapping-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN:   | FileCheck --check-prefix=CHECK-TRAPPING-NO-UNSAFE-MATH %s
 // RUN: %clang -### -funsafe-math-optimizations -fno-unsafe-math-optimizations \
 // RUN:     -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
@@ -283,18 +283,20 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 
 // RUN: %clang -### -ffast-math -fno-reciprocal-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN:   | FileCheck --check-prefix=CHECK-REASSOC-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fsigned-zeros -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -ftrapping-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
+// RUN:   | FileCheck --check-prefix=CHECK-TRAPPING-NO-UNSAFE-MATH %s
 // RUN: %clang -### -ffast-math -fno-unsafe-math-optimizations -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 
 // CHECK-NO-UNSAFE-MATH: "-cc1"
 // CHECK-NO-UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
-// CHECK-NO_UNSAFE-MATH-NOT: "-mreassociate"
+// CHECK-NO-UNSAFE-MATH-NOT: "-mreassociate"
 // CHECK-NO-UNSAFE-MATH: "-o"
+// CHECK-NO-UNSAFE-MATH-NOT: "-ffp-exception-behavior=strict"
+// CHECK-TRAPPING-NO-UNSAFE-MATH: "-ffp-exception-behavior=strict"
 
 
 // Reassociate is allowed because it does not require reciprocal-math.
@@ -304,8 +306,8 @@
 // RUN:   | FileCheck --check-prefix=CHECK-REASSOC-NO-UNSAFE-MATH %s
 
 // CHECK-REASSOC-NO-UNSAFE-MATH: "-cc1"
-// CHECK-REASSOC-NO_UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
-// CHECK-REASSOC-NO_UNSAFE-MATH: "-mreassociate"
+// CHECK-REASSOC-NO-UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
+// CHECK-REASSOC-NO-UNSAFE-MATH: "-mreassociate"
 // CHECK-REASSOC-NO-UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
 // CHECK-REASSOC-NO-UNSAFE-MATH: "-o"
 
@@ -318,12 +320,12 @@
 
 // RUN: %clang -### -fassociative-math -freciprocal-math -fno-signed-zeros \
 // RUN:     -fno-trapping-math -ftrapping-math -c %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-NO-REASSOC-NO-UNSAFE-MATH %s
+// RUN:   | FileCheck --check-prefix=CHECK-NO-UNSAFE-MATH %s
 
 // CHECK-NO-REASSOC-NO-UNSAFE-MATH: "-cc1"
-// CHECK-NO-REASSOC-NO_UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
-// CHECK-NO-REASSOC-NO_UNSAFE-MATH-NOT: "-mreassociate"
-// CHECK-NO-REASSOC-NO_UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
+// CHECK-NO-REASSOC-NO-UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
+// CHECK-NO-REASSOC-NO-UNSAFE-MATH-NOT: "-mreassociate"
+// CHECK-NO-REASSOC-NO-UNSAFE-MATH-NOT: "-funsafe-math-optimizations"
 // CHECK-NO-REASSOC-NO-UNSAFE-MATH: "-o"
 
 

[arsenm]

So this test was full of broken test checks?

[andykaylor]

At least this one was broken, which I think was indicated by the underscore in the check line. Many of the others are at least insufficient.

[zahiraam]

It looks like the lines with _ were just ignored! That's bad.

[arsenm]

I don't think this testing technique works. All of these flags would be printed on the same line as cc1. You would need to use CHECK-SAME-NOT, but I'm not sure you can actually compose -SAME and -NOT checks

-NOT checks are hazardous this way, can we positively check for the flags that are emitted.

[andykaylor]

I'd be happy to make that change. I was a little lazy with this test. I just wanted something that tested the change I was making, and then the rest of what I did was just what seemed necessary to make the test pass while still testing the things it meant to test. The whole thing seemed a bit of a mess. I can clean it up some with a bit more effort.

[andykaylor]

I'm not sure what this case was originally trying to do, but it seems that I can make CHECK, CHECK-NOT, and CHECK-SAME in a limited way. Specifically, CHECK-NOT doesn't reset the line used for CHECK-SAME. So if I have checks like this:

CHECK: foo
CHECK-NOT: bar
CHECK-SAME: fubar
CHECK-NOT: bar

The string "foo fubar" will pass, the string "foo bar fubar" will fail the first CHECK-NOT test, and "foo fubar bar" will fail the second CHECK-NOT test.

I think I can update this test such that it continues to use "-cc1" as the starting check for the options produced, and uses CHECK-SAME for the FP-related options it expects to be produced, and any options that should not be produced can be verified with a CHECK-NOT before and after the CHECK-SAME.

[zahiraam]

Yes, I think that should work!

[andykaylor]

I've made some minor changes to clean up the LIT test checks for this PR. I started to clean up the entire test, but it quickly became obvious that I should put that into a separate PR.

I will submit a new PR to clean up the entire fast-math.c test. I have an idea that I think will work, hard-coding the order of the math-related flags and using a separate prefix for each one plus negative checks. Then the test can just use "--check-prefixes=..." to put together the combinations that are expected. I think that will be more maintainable and more readable than trying to group checks in the more-or-less arbitrary way the test currently does.

[andykaylor]

I've made extensive changes to the fast-math.c test here: #89687

That update isn't intended to change any existing behavior. A few things worked differently than I expected, but I left the checks consistent with the current behavior and added comments where I thought the current behavior was incorrect.

[andykaylor]

ping for review

[zahiraam]

LGTM! Thanks.