[clang] Clean up denormal handling with -ffp-model, -ffast-math, etc. (PR #89477)

Andy Kaylor via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 25 15:17:55 PDT 2024


https://github.com/andykaylor updated https://github.com/llvm/llvm-project/pull/89477

>From 8ab931c4506f08685758a58f4cf7974c5254c3fa Mon Sep 17 00:00:00 2001
From: Andy Kaylor <andrew.kaylor at intel.com>
Date: Fri, 19 Apr 2024 17:53:52 -0700
Subject: [PATCH 1/5] Clean up denormal handling with -ffp-model, -ffast-math,
 etc.

This change cleans up the clang driver handling of umbrella options like
-ffast-math, -funsafe-math-optimizations, and -ffp-model, and aligns the
behavior of -ffp-model=fast with -ffast-math with regard to the linking of
crtfastmath.o.

We agreed in a previous review that the fast-math options should not
attempt to change the -fdenormal-fp-math option, which is inherently
target-specific.

The clang user's manual claims that -ffp-model=fast "behaves identically
to specifying both -ffast-math and -ffp-contract=fast." Since -ffast-math
causes crtfastmath.o to be linked if it is available, that should also
happen with -ffp-model=fast.

I am going to be proposing further changes to -ffp-model=fast,
decoupling it from -ffast-math and introducing a new
-ffp-model=aggressive that matches the current behavior, but I wanted
to solidfy the current behavior before I do that.
---
 clang/docs/UsersManual.rst            | 4 ++--
 clang/lib/Driver/ToolChain.cpp        | 8 +++++++-
 clang/lib/Driver/ToolChains/Clang.cpp | 4 ++++
 clang/test/Driver/linux-ld.c          | 3 +++
 clang/test/Driver/solaris-ld.c        | 3 +++
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index d0326f01d251e0..bd6b6966ef409f 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1452,8 +1452,8 @@ floating point semantic models: precise (the default), strict, and fast.
   "fenv_access", "off", "on", "off"
   "rounding_mode", "tonearest", "dynamic", "tonearest"
   "contract", "on", "off", "fast"
-  "denormal_fp_math", "IEEE", "IEEE", "IEEE"
-  "denormal_fp32_math", "IEEE","IEEE", "IEEE"
+  "denormal_fp_math", "IEEE", "IEEE", "target-dependent"
+  "denormal_fp32_math", "IEEE","IEEE", "target-dependent"
   "support_math_errno", "on", "on", "off"
   "no_honor_nans", "off", "off", "on"
   "no_honor_infinities", "off", "off", "on"
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 341d6202a9ca3c..0476ba87d07b66 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1319,11 +1319,17 @@ bool ToolChain::isFastMathRuntimeAvailable(const ArgList &Args,
     Arg *A =
       Args.getLastArg(options::OPT_ffast_math, options::OPT_fno_fast_math,
                       options::OPT_funsafe_math_optimizations,
-                      options::OPT_fno_unsafe_math_optimizations);
+                      options::OPT_fno_unsafe_math_optimizations,
+                      options::OPT_ffp_model_EQ);
 
     if (!A || A->getOption().getID() == options::OPT_fno_fast_math ||
         A->getOption().getID() == options::OPT_fno_unsafe_math_optimizations)
       Default = false;
+    if (A && A->getOption().getID() == options::OPT_ffp_model_EQ) {
+      StringRef Model = A->getValue();
+      if (Model != "fast")
+        Default = false;
+    }
   }
 
   // Whatever decision came as a result of the above implicit settings, either
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 651a2b5aac368b..1b1b8d6ee74801 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2924,6 +2924,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       if (Val.equals("fast")) {
         FPModel = Val;
         applyFastMath();
+        // The target-specific getDefaultDenormalModeForType handler should
+        // account for -ffp-model=fast and choose its behavior
+        DenormalFPMath = DefaultDenormalFPMath;
+        DenormalFP32Math = DefaultDenormalFP32Math;
       } else if (Val.equals("precise")) {
         optID = options::OPT_ffp_contract;
         FPModel = Val;
diff --git a/clang/test/Driver/linux-ld.c b/clang/test/Driver/linux-ld.c
index 958e682b6c3c11..db4abbbf874eae 100644
--- a/clang/test/Driver/linux-ld.c
+++ b/clang/test/Driver/linux-ld.c
@@ -1417,6 +1417,9 @@
 // RUN: %clang --target=x86_64-unknown-linux -no-pie -### %s -funsafe-math-optimizations\
 // RUN:        --sysroot=%S/Inputs/basic_linux_tree 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-CRTFASTMATH %s
+// RUN: %clang --target=x86_64-unknown-linux -no-pie -### %s -ffp-model=fast \
+// RUN:        --sysroot=%S/Inputs/basic_linux_tree 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-CRTFASTMATH %s
 // RUN: %clang --target=x86_64-unknown-linux -no-pie -### %s -Ofast\
 // RUN:        --sysroot=%S/Inputs/basic_linux_tree 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-CRTFASTMATH %s
diff --git a/clang/test/Driver/solaris-ld.c b/clang/test/Driver/solaris-ld.c
index 6d74389e89222c..ce0728d392bf23 100644
--- a/clang/test/Driver/solaris-ld.c
+++ b/clang/test/Driver/solaris-ld.c
@@ -193,6 +193,9 @@
 // RUN: %clang --target=sparc-sun-solaris2.11 -### %s -ffast-math \
 // RUN:        --sysroot=%S/Inputs/solaris_sparc_tree 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-CRTFASTMATH-SPARC32 %s
+// RUN: %clang --target=sparc-sun-solaris2.11 -### %s -ffp-model=fast \
+// RUN:        --sysroot=%S/Inputs/solaris_sparc_tree 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-CRTFASTMATH-SPARC32 %s
 // CHECK-CRTFASTMATH-SPARC32: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-CRTFASTMATH-SPARC32: "[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2{{/|\\\\}}crtfastmath.o"
 // CHECK-NOCRTFASTMATH-SPARC32-NOT: crtfastmath.o

>From bf1e0133ac1be2427bf23d4abee208ff96de879e Mon Sep 17 00:00:00 2001
From: Andy Kaylor <andrew.kaylor at intel.com>
Date: Wed, 24 Apr 2024 16:32:11 -0700
Subject: [PATCH 2/5] Update FP option rendering to leave denormal-fp-math
 alone

---
 clang/lib/Driver/ToolChains/Clang.cpp | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 1b1b8d6ee74801..250dc078edf242 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2743,13 +2743,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   StringRef FPExceptionBehavior = "";
   // -ffp-eval-method options: double, extended, source
   StringRef FPEvalMethod = "";
-  const llvm::DenormalMode DefaultDenormalFPMath =
+  llvm::DenormalMode DenormalFPMath =
       TC.getDefaultDenormalModeForType(Args, JA);
-  const llvm::DenormalMode DefaultDenormalFP32Math =
+  llvm::DenormalMode DenormalFP32Math =
       TC.getDefaultDenormalModeForType(Args, JA, &llvm::APFloat::IEEEsingle());
 
-  llvm::DenormalMode DenormalFPMath = DefaultDenormalFPMath;
-  llvm::DenormalMode DenormalFP32Math = DefaultDenormalFP32Math;
   // CUDA and HIP don't rely on the frontend to pass an ffp-contract option.
   // If one wasn't given by the user, don't pass it here.
   StringRef FPContract;
@@ -2899,11 +2897,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       SignedZeros = true;
       // -fno_fast_math restores default denormal and fpcontract handling
       FPContract = "on";
-      DenormalFPMath = llvm::DenormalMode::getIEEE();
-
-      // FIXME: The target may have picked a non-IEEE default mode here based on
-      // -cl-denorms-are-zero. Should the target consider -fp-model interaction?
-      DenormalFP32Math = llvm::DenormalMode::getIEEE();
 
       StringRef Val = A->getValue();
       if (OFastEnabled && !Val.equals("fast")) {
@@ -2924,10 +2917,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       if (Val.equals("fast")) {
         FPModel = Val;
         applyFastMath();
-        // The target-specific getDefaultDenormalModeForType handler should
-        // account for -ffp-model=fast and choose its behavior
-        DenormalFPMath = DefaultDenormalFPMath;
-        DenormalFP32Math = DefaultDenormalFP32Math;
       } else if (Val.equals("precise")) {
         optID = options::OPT_ffp_contract;
         FPModel = Val;
@@ -3126,9 +3115,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       TrappingMath = true;
       FPExceptionBehavior = "strict";
 
-      // The target may have opted to flush by default, so force IEEE.
-      DenormalFPMath = llvm::DenormalMode::getIEEE();
-      DenormalFP32Math = llvm::DenormalMode::getIEEE();
       if (!JA.isDeviceOffloading(Action::OFK_Cuda) &&
           !JA.isOffloading(Action::OFK_HIP)) {
         if (LastSeenFfpContractOption != "") {
@@ -3158,9 +3144,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       ReciprocalMath = false;
       ApproxFunc = false;
       SignedZeros = true;
-      // -fno_fast_math restores default denormal and fpcontract handling
-      DenormalFPMath = DefaultDenormalFPMath;
-      DenormalFP32Math = llvm::DenormalMode::getIEEE();
+      // -fno_fast_math restores default fpcontract handling
       if (!JA.isDeviceOffloading(Action::OFK_Cuda) &&
           !JA.isOffloading(Action::OFK_HIP)) {
         if (LastSeenFfpContractOption != "") {
@@ -3175,8 +3159,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       // subsequent options conflict then emit warning diagnostic.
       if (HonorINFs && HonorNaNs && !AssociativeMath && !ReciprocalMath &&
           SignedZeros && TrappingMath && RoundingFPMath && !ApproxFunc &&
-          DenormalFPMath == llvm::DenormalMode::getIEEE() &&
-          DenormalFP32Math == llvm::DenormalMode::getIEEE() &&
           FPContract.equals("off"))
         // OK: Current Arg doesn't conflict with -ffp-model=strict
         ;

>From f1f643c02b9dd2f7cac8a3207c2d14883bbb628d Mon Sep 17 00:00:00 2001
From: Andy Kaylor <andrew.kaylor at intel.com>
Date: Wed, 24 Apr 2024 16:45:26 -0700
Subject: [PATCH 3/5] User manual updates

---
 clang/docs/UsersManual.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index bd6b6966ef409f..9c7a2bb5f650c6 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1452,8 +1452,6 @@ floating point semantic models: precise (the default), strict, and fast.
   "fenv_access", "off", "on", "off"
   "rounding_mode", "tonearest", "dynamic", "tonearest"
   "contract", "on", "off", "fast"
-  "denormal_fp_math", "IEEE", "IEEE", "target-dependent"
-  "denormal_fp32_math", "IEEE","IEEE", "target-dependent"
   "support_math_errno", "on", "on", "off"
   "no_honor_nans", "off", "off", "on"
   "no_honor_infinities", "off", "off", "on"
@@ -1462,6 +1460,14 @@ floating point semantic models: precise (the default), strict, and fast.
   "allow_approximate_fns", "off", "off", "on"
   "allow_reassociation", "off", "off", "on"
 
+   The ``-fp-model`` option does not modify the "fdenormal-fp-math" or
+   "fdenormal-fp-math-f32" settings, but it does have an impact on whether
+   "crtfastmath.o" is linked. Because linking "crtfastmath.o" has a global
+   effect on the program, and because the global denormal handling can be
+   changed in other ways, the state of "fdenormal-fp-math" handling cannot
+   be assumed in any function based on fp-model. See :ref:`crtfastmath.o`
+   for more details.
+
 .. option:: -ffast-math
 
    Enable fast-math mode.  This option lets the
@@ -1537,7 +1543,6 @@ floating point semantic models: precise (the default), strict, and fast.
    Also, this option resets following options to their target-dependent defaults.
 
    * ``-f[no-]math-errno``
-   * ``-fdenormal-fp-math=<value>``
 
    There is ambiguity about how ``-ffp-contract``, ``-ffast-math``,
    and ``-fno-fast-math`` behave when combined. To keep the value of
@@ -1560,8 +1565,7 @@ floating point semantic models: precise (the default), strict, and fast.
      ``-ffp-contract`` setting is determined by the default value of
      ``-ffp-contract``.
 
-   Note: ``-fno-fast-math`` implies ``-fdenormal-fp-math=ieee``.
-   ``-fno-fast-math`` causes ``crtfastmath.o`` to not be linked with code
+   Note: ``-fno-fast-math`` causes ``crtfastmath.o`` to not be linked with code
    unless ``-mdaz-ftz`` is present.
 
 .. option:: -fdenormal-fp-math=<value>
@@ -1694,7 +1698,6 @@ floating point semantic models: precise (the default), strict, and fast.
    * ``-fsigned-zeros``
    * ``-ftrapping-math``
    * ``-ffp-contract=on``
-   * ``-fdenormal-fp-math=ieee``
 
    There is ambiguity about how ``-ffp-contract``,
    ``-funsafe-math-optimizations``, and ``-fno-unsafe-math-optimizations``

>From 061485033bd50edbc35c82283c39732d0be8f673 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <andrew.kaylor at intel.com>
Date: Thu, 25 Apr 2024 12:09:53 -0700
Subject: [PATCH 4/5] Formatting fixes

---
 clang/lib/Driver/ToolChain.cpp | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 0476ba87d07b66..0e86bc07e0ea2c 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1316,11 +1316,10 @@ bool ToolChain::isFastMathRuntimeAvailable(const ArgList &Args,
   // (to keep the linker options consistent with gcc and clang itself).
   if (Default && !isOptimizationLevelFast(Args)) {
     // Check if -ffast-math or -funsafe-math.
-    Arg *A =
-      Args.getLastArg(options::OPT_ffast_math, options::OPT_fno_fast_math,
-                      options::OPT_funsafe_math_optimizations,
-                      options::OPT_fno_unsafe_math_optimizations,
-                      options::OPT_ffp_model_EQ);
+    Arg *A = Args.getLastArg(
+        options::OPT_ffast_math, options::OPT_fno_fast_math,
+        options::OPT_funsafe_math_optimizations,
+        options::OPT_fno_unsafe_math_optimizations, options::OPT_ffp_model_EQ);
 
     if (!A || A->getOption().getID() == options::OPT_fno_fast_math ||
         A->getOption().getID() == options::OPT_fno_unsafe_math_optimizations)

>From 4fc3057dbbdd1bec181d637f2aeb0fdb6e1a42ba Mon Sep 17 00:00:00 2001
From: Andy Kaylor <andrew.kaylor at intel.com>
Date: Thu, 25 Apr 2024 15:17:04 -0700
Subject: [PATCH 5/5] Fix fp-model.c test and UsersManual.rst

---
 clang/docs/UsersManual.rst   | 14 +++++++-------
 clang/test/Driver/fp-model.c |  5 ++++-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 9c7a2bb5f650c6..34679a6589982b 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1460,13 +1460,13 @@ floating point semantic models: precise (the default), strict, and fast.
   "allow_approximate_fns", "off", "off", "on"
   "allow_reassociation", "off", "off", "on"
 
-   The ``-fp-model`` option does not modify the "fdenormal-fp-math" or
-   "fdenormal-fp-math-f32" settings, but it does have an impact on whether
-   "crtfastmath.o" is linked. Because linking "crtfastmath.o" has a global
-   effect on the program, and because the global denormal handling can be
-   changed in other ways, the state of "fdenormal-fp-math" handling cannot
-   be assumed in any function based on fp-model. See :ref:`crtfastmath.o`
-   for more details.
+The ``-fp-model`` option does not modify the "fdenormal-fp-math" or
+"fdenormal-fp-math-f32" settings, but it does have an impact on whether
+"crtfastmath.o" is linked. Because linking "crtfastmath.o" has a global
+effect on the program, and because the global denormal handling can be
+changed in other ways, the state of "fdenormal-fp-math" handling cannot
+be assumed in any function based on fp-model. See :ref:`crtfastmath.o`
+for more details.
 
 .. option:: -ffast-math
 
diff --git a/clang/test/Driver/fp-model.c b/clang/test/Driver/fp-model.c
index 74b7de7a275a76..e4e10dc174912c 100644
--- a/clang/test/Driver/fp-model.c
+++ b/clang/test/Driver/fp-model.c
@@ -64,7 +64,8 @@
 
 // RUN: %clang -### -ffp-model=strict -fdenormal-fp-math=preserve-sign,preserve-sign -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN10 %s
-// WARN10: warning: overriding '-ffp-model=strict' option with '-fdenormal-fp-math=preserve-sign,preserve-sign' [-Woverriding-option]
+// WARN10: clang
+// WARN10-NOT: warning: overriding '-ffp-model=strict' option with '-fdenormal-fp-math=preserve-sign,preserve-sign' [-Woverriding-option]
 
 // RUN: %clang -### -ffp-model=fast -ffp-model=strict -c %s 2>&1 | FileCheck \
 // RUN:   --check-prefix=WARN11 %s
@@ -75,6 +76,7 @@
 // RUN:   --check-prefix=WARN12 %s
 // RUN: %clang -### -ffast-math -ffp-model=strict -c %s 2>&1 | FileCheck \
 // RUN:   --check-prefix=WARN12 %s
+// WARN12: clang
 // WARN12-NOT: warning: overriding '-ffp-model=strict' option with '-ffp-model=strict' [-Woverriding-option]
 
 // RUN: %clang -### -ffp-model=strict -fapprox-func -c %s 2>&1 \
@@ -129,6 +131,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s
 // RUN: %clang -### -nostdinc -ffp-model=strict -Ofast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NO-EXCEPT %s
+// CHECK-NO-EXCEPT: "-cc1"
 // CHECK-NO-EXCEPT-NOT: "-ffp-exception-behavior=strict"
 
 // RUN: %clang -### -nostdinc -ffp-exception-behavior=strict -c %s 2>&1 \



More information about the cfe-commits mailing list