From 1b69ce1208976c71bf7ee3932aa272462c1feb1b Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat Date: Mon, 29 Aug 2022 10:18:19 -0400 Subject: [PATCH] =?utf8?q?Currently=20the=20options=20=E2=80=98ffast-math?= =?utf8?q?=E2=80=99=20and=20=E2=80=98ffp-contract=E2=80=99=20are=20connect?= =?utf8?q?ed.=20When=20=E2=80=98ffast-math=E2=80=99=20is=20set,=20ffp-cont?= =?utf8?q?ract=20is=20altered=20this=20way:=20-ffast-math/=20Ofast=20->=20?= =?utf8?q?ffp-contract=3Dfast=20-fno-fast-math=20->=20if=20ffp-contract=3D?= =?utf8?q?=20fast=20then=20ffp-contract=3Don=20else=20ffp-contract=20uncha?= =?utf8?q?nged?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This differs from gcc which doesn’t connect the two options. Connecting these two options in clang, resulted in spurious warnings when the user combines these two options -ffast-math -fno-fast-math; see issue https://github.com/llvm/llvm-project/issues/54625. The issue is that the ‘ffast-math’ option is an on/off flag, but the ‘ffp-contract’ is an on/off/fast flag. So when ‘fno-fast-math’ is used there is no obvious value for ‘ffp-contract’. What should the value of ffp-contract be for -ffp-contract=fast -fno-fast-math and -ffast-math -ffp-contract=fast -fno-fast-math? The current logic sets ffp-contract back to on in these cases. This doesn’t take into account that the value of ffp-contract is modified by an explicit ffp-contract` option. This patch is proposing a set of rules to apply when ffp-contract', ffast-math and fno-fast-math are combined. These rules would give the user the expected behavior and no diagnostic would be needed. See RFC https://discourse.llvm.org/t/rfc-making-ffast-math-option-unrelated-to-ffp-contract-option/61912 --- clang/docs/UsersManual.rst | 68 +++++++++++++++++- clang/lib/Driver/ToolChains/Clang.cpp | 19 +++--- clang/test/CodeGen/ffp-contract-option.c | 2 +- clang/test/Driver/fp-contract.c | 114 +++++++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 11 deletions(-) create mode 100644 clang/test/Driver/fp-contract.c diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index e9c1be4..bc107be 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -1359,8 +1359,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", "PreserveSign" - "denormal_fp32_math", "IEEE","IEEE", "PreserveSign" + "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" @@ -1408,6 +1408,61 @@ floating point semantic models: precise (the default), strict, and fast. * ``-ffp-contract=fast`` + Note: ``-ffast-math`` causes ``crtfastmath.o`` to be linked with code. See + :ref:`crtfastmath.o` for more details. + +.. option:: -fno-fast-math + + Disable fast-math mode. This options disables unsafe floating-point + optimizations by preventing the compiler from making any tranformations that + could affect the results. + + This option implies: + + * ``-fhonor-infinities`` + + * ``-fhonor-nans`` + + * ``-fmath-errno`` + + * ``-fno-finite-math-only`` + + * ``-fno-associative-math`` + + * ``-fno-reciprocal-math`` + + * ``-fsigned-zeros`` + + * ``-fno-trapping-math`` + + * ``-ffp-contract=on`` + + * ``-fdenormal-fp-math=ieee`` + + There is ambiguity about how ``-ffp-contract``, ``-ffast-math``, + and ``-fno-fast-math`` behave in combination. To keep the value of + ``-ffp-contract`` consistent, we define this set of rules: + + * ``-ffast-math`` sets ``ffp-contract`` to ``fast``. + + * ``-fno-fast-math`` sets ``-ffp-contract`` to ``on`` (``fast`` for CUDA and + HIP). + + * If ``-ffast-math`` and ``-ffp-contract`` are both seen, but + ``-ffast-math`` is not followed by ``-fno-fast-math``, ``ffp-contract`` + will be given the value of whichever option was last seen. + + * If ``-fno-fast-math`` is seen and ``-ffp-contract`` has been seen at least + once, the ``ffp-contract`` will get the value of the last seen value of + ``-ffp-contract``. + + * If ``-fno-fast-math`` is seen and ``-ffp-contract`` has not been seen, the + ``-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. + .. option:: -fdenormal-fp-math= Select which denormal numbers the code is permitted to require. @@ -1632,6 +1687,15 @@ Note that floating-point operations performed as part of constant initialization has no effect because the optimizer is prohibited from making unsafe transformations. +.. _crtfastmath.o: + +A note about ``crtfastmath.o`` +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +``-ffast-math`` and ``-funsafe-math-optimizations`` cause ``crtfastmath.o`` to be +automatically linked, which adds a static constructor that sets the FTZ/DAZ +bits in MXCSR, affecting not only the current compilation unit but all static +and shared libraries included in the program. + .. _FLT_EVAL_METHOD: A note about ``__FLT_EVAL_METHOD__`` diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index e215dc0..9cc0830 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -2786,6 +2786,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, // 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; + StringRef LastSeenFfpContractOption; + bool SeenFfastMathOption = false; if (!JA.isDeviceOffloading(Action::OFK_Cuda) && !JA.isOffloading(Action::OFK_HIP)) FPContract = "on"; @@ -2936,9 +2938,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, // -ffp-model=precise sets PreciseFPModel to on and Val to // "precise". FPContract is set. ; - } else if (Val.equals("fast") || Val.equals("on") || Val.equals("off")) + } else if (Val.equals("fast") || Val.equals("on") || Val.equals("off")) { FPContract = Val; - else + LastSeenFfpContractOption = Val; + } else D.Diag(diag::err_drv_unsupported_option_argument) << A->getOption().getName() << Val; break; @@ -3031,6 +3034,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, RoundingFPMath = false; // If fast-math is set then set the fp-contract mode to fast. FPContract = "fast"; + SeenFfastMathOption = true; break; case options::OPT_fno_fast_math: HonorINFs = true; @@ -3047,13 +3051,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, DenormalFPMath = DefaultDenormalFPMath; DenormalFP32Math = llvm::DenormalMode::getIEEE(); if (!JA.isDeviceOffloading(Action::OFK_Cuda) && - !JA.isOffloading(Action::OFK_HIP)) - if (FPContract == "fast") { + !JA.isOffloading(Action::OFK_HIP)) { + if (LastSeenFfpContractOption != "") { + FPContract = LastSeenFfpContractOption; + } else if (SeenFfastMathOption) FPContract = "on"; - D.Diag(clang::diag::warn_drv_overriding_flag_option) - << "-ffp-contract=fast" - << "-ffp-contract=on"; - } + } break; } if (StrictFPModel) { diff --git a/clang/test/CodeGen/ffp-contract-option.c b/clang/test/CodeGen/ffp-contract-option.c index 209f593..232403b 100644 --- a/clang/test/CodeGen/ffp-contract-option.c +++ b/clang/test/CodeGen/ffp-contract-option.c @@ -35,7 +35,7 @@ // RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-FPC-ON // RUN: %clang -Xclang -no-opaque-pointers -S -emit-llvm -ffp-contract=fast -fno-fast-math \ -// RUN: %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-FPC-ON +// RUN: %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-CONTRACTFAST // RUN: %clang -Xclang -no-opaque-pointers -S -emit-llvm -ffp-contract=on -fno-fast-math \ // RUN: %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-FPC-ON diff --git a/clang/test/Driver/fp-contract.c b/clang/test/Driver/fp-contract.c new file mode 100644 index 0000000..c4d1840 --- /dev/null +++ b/clang/test/Driver/fp-contract.c @@ -0,0 +1,114 @@ +// Test that -ffp-contract is set to the right value when combined with +// the options -ffast-math and -fno-fast-math. + +// RUN: %clang -### -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s +// CHECK-FPC-FAST: "-ffp-contract=fast" + +// RUN: %clang -### -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s +// CHECK-FPC-ON: "-ffp-contract=on" + +// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s +// CHECK-FPC-OFF: "-ffp-contract=off" + +// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s +// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s +// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=fast -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s +// RUN: %clang -### -ffp-contract=on -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s +// RUN: %clang -### -ffp-contract=off -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s + + +// RUN: %clang -### -ffast-math -ffp-contract=fast -ffp-contract=on -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s + +// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=fast \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s + +// RUN: %clang -### -ffast-math -ffp-contract=fast -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -fno-fast-math -ffp-contract=on -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -fno-fast-math -ffp-contract=off -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s + +// RUN: %clang -### -fno-fast-math -ffp-contract=fast -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=on \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=off \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s + +// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=fast \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=on \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -ffp-contract=on -ffast-math -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -ffp-contract=off -ffast-math -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s + +// RUN: %clang -### -ffp-contract=fast -ffast-math -fno-fast-math -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -fno-fast-math -ffast-math -ffp-contract=fast \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s + +// RUN: %clang -### -fno-fast-math -ffast-math -ffp-contract=on \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s + +// RUN: %clang -### -fno-fast-math -ffast-math -ffp-contract=off \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s + -- 2.7.4