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/91271.diff 3 Files Affected: - (modified) clang/lib/Driver/ToolChains/Clang.cpp (+53-37) - (modified) clang/test/Driver/fp-contract.c (+14) - (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 0a2ea96de73824..b1400770f59d53 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -2751,6 +2751,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)) @@ -2792,6 +2793,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()); @@ -2893,7 +2915,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.equals("fast")) { @@ -2910,14 +2931,18 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, if (Val.equals("fast")) { FPModel = Val; applyFastMath(); + // applyFastMath sets fp-contract="fast" + LastFpContractOverrideOption = "-ffp-model=fast"; } else if (Val.equals("precise")) { FPModel = Val; FPContract = "on"; + LastFpContractOverrideOption = "-ffp-model=precise"; } else if (Val.equals("strict")) { StrictFPModel = true; FPExceptionBehavior = "strict"; FPModel = Val; FPContract = "off"; + LastFpContractOverrideOption = "-ffp-model=strict"; TrappingMath = true; RoundingFPMath = true; } else @@ -2996,8 +3021,15 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, StringRef Val = A->getValue(); if (Val.equals("fast") || Val.equals("on") || Val.equals("off") || Val.equals("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; @@ -3077,6 +3109,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: @@ -3084,14 +3117,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: @@ -3099,10 +3125,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; @@ -3114,16 +3143,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 @@ -3141,12 +3165,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; + } } } @@ -3228,21 +3257,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.equals("fast")) { - if (FPContract.equals("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 660f67fad3ccbe..efd9dd406df21f 100644 --- a/clang/test/Driver/fp-contract.c +++ b/clang/test/Driver/fp-contract.c @@ -238,3 +238,17 @@ // 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 9d12452399119b..a23bb333c398a3 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/91271 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
