Skip to content

[Driver] Clean up denormal handling with fast-math-related options #89477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions clang/docs/UsersManual.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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", "IEEE"
"denormal_fp32_math", "IEEE","IEEE", "IEEE"
"support_math_errno", "on", "on", "off"
"no_honor_nans", "off", "off", "on"
"no_honor_infinities", "off", "off", "on"
Expand All @@ -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 ``-ffp-model`` option does not modify the ``fdenormal-fp-math``
setting, 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
Expand Down Expand Up @@ -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
Expand All @@ -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>
Expand Down Expand Up @@ -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``
Expand Down
13 changes: 9 additions & 4 deletions clang/lib/Driver/ToolChain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1316,14 +1316,19 @@ 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);
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)
Default = false;
if (A && A->getOption().getID() == options::OPT_ffp_model_EQ) {
StringRef Model = A->getValue();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: A->getValue() != "fast"

inline used-once variable and prefer == != for StringRef, which are much more common than equals.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope to add -ffp-model=aggressive soon, with will require a second use of Model here.

if (Model != "fast")
Default = false;
}
}

// Whatever decision came as a result of the above implicit settings, either
Expand Down
20 changes: 3 additions & 17 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")) {
Expand Down Expand Up @@ -3122,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 != "") {
Expand Down Expand Up @@ -3154,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 != "") {
Expand All @@ -3171,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
;
Expand Down
9 changes: 5 additions & 4 deletions clang/test/Driver/fp-model.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@

// RUN: %clang -### -ffp-model=strict -fdenormal-fp-math=preserve-sign,preserve-sign -c %s 2>&1 \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use -### -Werror to ensure that there is no warning.

(In the absence of a --target=, you probably want to check a few different targets that the warning is printed regardless of the default target triple.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't intend to change the logic of this test. The behavior here should be target-independent now.

// RUN: | FileCheck --check-prefix=WARN10 %s
// WARN10: warning: overriding '-ffp-model=strict' option with '-fdenormal-fp-math=preserve-sign,preserve-sign' [-Woverriding-option]
// WARN10: "-cc1"
// 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
Expand All @@ -73,9 +74,8 @@

// RUN: %clang -### -Ofast -ffp-model=strict -c %s 2>&1 | FileCheck \
// RUN: --check-prefix=WARN12 %s
// RUN: %clang -### -ffast-math -ffp-model=strict -c %s 2>&1 | FileCheck \
// RUN: --check-prefix=WARN12 %s
// WARN12-NOT: warning: overriding '-ffp-model=strict' option with '-ffp-model=strict' [-Woverriding-option]
// RUN: %clang -### -Werror -ffast-math -ffp-model=strict -c %s
// WARN12: warning: overriding '-ffp-model=strict' option with '-Ofast'

// RUN: %clang -### -ffp-model=strict -fapprox-func -c %s 2>&1 \
// RUN: | FileCheck --check-prefix=WARN13 %s
Expand Down Expand Up @@ -129,6 +129,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 \
Expand Down
3 changes: 3 additions & 0 deletions clang/test/Driver/linux-ld.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions clang/test/Driver/solaris-ld.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down