[clang] [Driver] Clean up fp-contract handling in clang driver (PR #91271)
via cfe-commits
cfe-commits at lists.llvm.org
Mon May 6 14:04:22 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-driver
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
More information about the cfe-commits
mailing list