llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) <details> <summary>Changes</summary> This change refactors the fp-contract handling in RenderFloatingPointOptions in the clang driver code and fixes some inconsistencies in the way warnings are reported for changes in the fp-contract behavior. --- Full diff: https://github.com/llvm/llvm-project/pull/99723.diff 3 Files Affected: - (modified) clang/lib/Driver/ToolChains/Clang.cpp (+53-37) - (modified) clang/test/Driver/fp-contract.c (+35-21) - (modified) clang/test/Driver/fp-model.c (+14) ``````````diff diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index f7b987bf810c1..b6838f42f1331 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -2812,6 +2812,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, // If one wasn't given by the user, don't pass it here. StringRef FPContract; StringRef LastSeenFfpContractOption; + StringRef LastFpContractOverrideOption; bool SeenUnsafeMathModeOption = false; if (!JA.isDeviceOffloading(Action::OFK_Cuda) && !JA.isOffloading(Action::OFK_HIP)) @@ -2853,6 +2854,27 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, SeenUnsafeMathModeOption = true; }; + // Lambda to consolidate common handling for fp-contract + auto restoreFPContractState = [&]() { + // CUDA and HIP don't rely on the frontend to pass an ffp-contract option. + // For other targets, if the state has been changed by one of the + // unsafe-math umbrella options a subsequent -fno-fast-math or + // -fno-unsafe-math-optimizations option reverts to the last value seen for + // the -ffp-contract option or "on" if we have not seen the -ffp-contract + // option. If we have not seen an unsafe-math option or -ffp-contract, + // we leave the FPContract state unchanged. + if (!JA.isDeviceOffloading(Action::OFK_Cuda) && + !JA.isOffloading(Action::OFK_HIP)) { + if (LastSeenFfpContractOption != "") + FPContract = LastSeenFfpContractOption; + else if (SeenUnsafeMathModeOption) + FPContract = "on"; + } + // In this case, we're reverting to the last explicit fp-contract option + // or the platform default + LastFpContractOverrideOption = ""; + }; + if (const Arg *A = Args.getLastArg(options::OPT_flimited_precision_EQ)) { CmdArgs.push_back("-mlimit-float-precision"); CmdArgs.push_back(A->getValue()); @@ -2954,7 +2976,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, AssociativeMath = false; ReciprocalMath = false; SignedZeros = true; - FPContract = "on"; StringRef Val = A->getValue(); if (OFastEnabled && Val != "fast") { @@ -2971,14 +2992,18 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, if (Val == "fast") { FPModel = Val; applyFastMath(); + // applyFastMath sets fp-contract="fast" + LastFpContractOverrideOption = "-ffp-model=fast"; } else if (Val == "precise") { FPModel = Val; FPContract = "on"; + LastFpContractOverrideOption = "-ffp-model=precise"; } else if (Val == "strict") { StrictFPModel = true; FPExceptionBehavior = "strict"; FPModel = Val; FPContract = "off"; + LastFpContractOverrideOption = "-ffp-model=strict"; TrappingMath = true; RoundingFPMath = true; } else @@ -3057,8 +3082,15 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, StringRef Val = A->getValue(); if (Val == "fast" || Val == "on" || Val == "off" || Val == "fast-honor-pragmas") { + if (Val != FPContract && LastFpContractOverrideOption != "") { + D.Diag(clang::diag::warn_drv_overriding_option) + << LastFpContractOverrideOption + << Args.MakeArgString("-ffp-contract=" + Val); + } + FPContract = Val; LastSeenFfpContractOption = Val; + LastFpContractOverrideOption = ""; } else D.Diag(diag::err_drv_unsupported_option_argument) << A->getSpelling() << Val; @@ -3137,6 +3169,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, TrappingMath = false; FPExceptionBehavior = ""; FPContract = "fast"; + LastFpContractOverrideOption = "-funsafe-math-optimizations"; SeenUnsafeMathModeOption = true; break; case options::OPT_fno_unsafe_math_optimizations: @@ -3144,14 +3177,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, ReciprocalMath = false; SignedZeros = true; ApproxFunc = false; - - if (!JA.isDeviceOffloading(Action::OFK_Cuda) && - !JA.isOffloading(Action::OFK_HIP)) { - if (LastSeenFfpContractOption != "") { - FPContract = LastSeenFfpContractOption; - } else if (SeenUnsafeMathModeOption) - FPContract = "on"; - } + restoreFPContractState(); break; case options::OPT_Ofast: @@ -3159,10 +3185,13 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, if (!OFastEnabled) continue; [[fallthrough]]; - case options::OPT_ffast_math: { + case options::OPT_ffast_math: applyFastMath(); + if (A->getOption().getID() == options::OPT_Ofast) + LastFpContractOverrideOption = "-Ofast"; + else + LastFpContractOverrideOption = "-ffast-math"; break; - } case options::OPT_fno_fast_math: HonorINFs = true; HonorNaNs = true; @@ -3174,16 +3203,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, ReciprocalMath = false; ApproxFunc = false; SignedZeros = true; - // -fno_fast_math restores default fpcontract handling - if (!JA.isDeviceOffloading(Action::OFK_Cuda) && - !JA.isOffloading(Action::OFK_HIP)) { - if (LastSeenFfpContractOption != "") { - FPContract = LastSeenFfpContractOption; - } else if (SeenUnsafeMathModeOption) - FPContract = "on"; - } + restoreFPContractState(); + LastFpContractOverrideOption = ""; break; - } + } // End switch (A->getOption().getID()) + // The StrictFPModel local variable is needed to report warnings // in the way we intend. If -ffp-model=strict has been used, we // want to report a warning for the next option encountered that @@ -3201,12 +3225,17 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, else { StrictFPModel = false; FPModel = ""; + // The warning for -ffp-contract would have been reported by the + // OPT_ffp_contract_EQ handler above. A special check here is needed + // to avoid duplicating the warning. auto RHS = (A->getNumValues() == 0) ? A->getSpelling() : Args.MakeArgString(A->getSpelling() + A->getValue()); - if (RHS != "-ffp-model=strict") - D.Diag(clang::diag::warn_drv_overriding_option) - << "-ffp-model=strict" << RHS; + if (A->getSpelling() != "-ffp-contract=") { + if (RHS != "-ffp-model=strict") + D.Diag(clang::diag::warn_drv_overriding_option) + << "-ffp-model=strict" << RHS; + } } } @@ -3288,21 +3317,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, // individual features enabled by -ffast-math instead of the option itself as // that's consistent with gcc's behaviour. if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath && ApproxFunc && - ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) { + ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) CmdArgs.push_back("-ffast-math"); - if (FPModel == "fast") { - if (FPContract == "fast") - // All set, do nothing. - ; - else if (FPContract.empty()) - // Enable -ffp-contract=fast - CmdArgs.push_back(Args.MakeArgString("-ffp-contract=fast")); - else - D.Diag(clang::diag::warn_drv_overriding_option) - << "-ffp-model=fast" - << Args.MakeArgString("-ffp-contract=" + FPContract); - } - } // Handle __FINITE_MATH_ONLY__ similarly. if (!HonorINFs && !HonorNaNs) diff --git a/clang/test/Driver/fp-contract.c b/clang/test/Driver/fp-contract.c index e2691dc211cc3..978bf80345165 100644 --- a/clang/test/Driver/fp-contract.c +++ b/clang/test/Driver/fp-contract.c @@ -10,18 +10,18 @@ // RUN: %clang -### -fno-fast-math -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=on -c %s 2>&1 \ +// 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 -### -Werror -ffast-math -ffp-contract=off -c %s 2>&1 \ +// 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 -### -Werror -ffast-math -ffp-contract=fast -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=fast-honor-pragmas -c %s 2>&1 \ +// RUN: %clang -### -ffast-math -ffp-contract=fast-honor-pragmas -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-FAST-HONOR %s // CHECK-FPC-FAST-HONOR: "-ffp-contract=fast-honor-pragmas" @@ -43,22 +43,22 @@ // RUN: %clang -### -Werror -ffast-math -ffp-contract=fast -ffp-contract=on -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \ +// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \ +// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \ +// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=off -ffp-contract=fast \ +// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=fast \ // RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \ +// RUN: %clang -### -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \ +// RUN: %clang -### -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s // RUN: %clang -### -Werror -ffast-math -ffp-contract=fast -fno-fast-math -c %s 2>&1 \ @@ -112,10 +112,10 @@ // RUN: %clang -### -Werror -fno-fast-math -ffast-math -ffp-contract=fast \ // RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -fno-fast-math -ffast-math -ffp-contract=on \ +// RUN: %clang -### -fno-fast-math -ffast-math -ffp-contract=on \ // RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -fno-fast-math -ffast-math -ffp-contract=off \ +// RUN: %clang -### -fno-fast-math -ffast-math -ffp-contract=off \ // RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s // funsafe-math-optimizations, fno-unsafe-math-optimizations @@ -125,10 +125,10 @@ // RUN: %clang -### -fno-unsafe-math-optimizations -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=on -c %s 2>&1 \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=on -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=off -c %s 2>&1 \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=off -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s // RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=fast -c %s 2>&1 \ @@ -151,25 +151,25 @@ // RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=fast \ // RUN: -ffp-contract=on -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=on \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=on \ // RUN: -ffp-contract=off -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=on \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=on \ // RUN: -ffp-contract=fast -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=off \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=off \ // RUN: -ffp-contract=on -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=off \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=off \ // RUN: -ffp-contract=fast \ // RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=on \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=on \ // RUN: -fno-unsafe-math-optimizations -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -funsafe-math-optimizations -ffp-contract=off \ +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=off \ // RUN: -fno-unsafe-math-optimizations -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-FPC-OFF %s @@ -229,9 +229,23 @@ // RUN: -ffp-contract=fast \ // RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s -// RUN: %clang -### -Werror -fno-unsafe-math-optimizations -funsafe-math-optimizations \ +// RUN: %clang -### -fno-unsafe-math-optimizations -funsafe-math-optimizations \ // RUN: -ffp-contract=on -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s -// RUN: %clang -### -Werror -fno-unsafe-math-optimizations -funsafe-math-optimizations \ +// RUN: %clang -### -fno-unsafe-math-optimizations -funsafe-math-optimizations \ // RUN: -ffp-contract=off -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s +// RUN: %clang -### -funsafe-math-optimizations -ffp-contract=off -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=WARN %s +// WARN: warning: overriding '-funsafe-math-optimizations' option with '-ffp-contract=off' + +// This case should not warn +// RUN: %clang -### -Werror -funsafe-math-optimizations \ +// RUN: -fno-unsafe-math-optimizations -ffp-contract=off -c %s + +// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=WARN2 %s +// WARN2: warning: overriding '-ffast-math' option with '-ffp-contract=off' + +// This case should not warn +// RUN: %clang -### -Werror -ffast-math -fno-fast-math -ffp-contract=off -c %s diff --git a/clang/test/Driver/fp-model.c b/clang/test/Driver/fp-model.c index 644523394d6b1..8451885669512 100644 --- a/clang/test/Driver/fp-model.c +++ b/clang/test/Driver/fp-model.c @@ -81,6 +81,20 @@ // RUN: | FileCheck --check-prefix=WARN13 %s // WARN13: warning: overriding '-ffp-model=strict' option with '-fapprox-func' [-Woverriding-option] +// RUN: %clang -### -ffp-model=precise -ffp-contract=off -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=WARN14 %s +// WARN14: warning: overriding '-ffp-model=precise' option with '-ffp-contract=off' [-Woverriding-option] + +// RUN: %clang -### -ffp-model=precise -ffp-contract=fast -c %s 2>&1 \ +// RUN: | FileCheck --check-prefix=WARN15 %s +// WARN15: warning: overriding '-ffp-model=precise' option with '-ffp-contract=fast' [-Woverriding-option] + +// RUN: %clang -### -ffp-model=strict -fassociative-math -ffp-contract=on \ +// RUN: -c %s 2>&1 | FileCheck --check-prefix=WARN16 %s +// WARN16: warning: overriding '-ffp-model=strict' option with '-fassociative-math' [-Woverriding-option] +// WARN16: warning: overriding '-ffp-model=strict' option with '-ffp-contract=on' [-Woverriding-option] + + // RUN: %clang -### -c %s 2>&1 \ // RUN: | FileCheck --check-prefix=CHECK-NOROUND %s // CHECK-NOROUND: "-cc1" `````````` </details> https://github.com/llvm/llvm-project/pull/99723 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
