[clang] [Driver] Clean up fp-contract handling in clang driver (PR #99723)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 19 17:05:09 PDT 2024
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
More information about the cfe-commits
mailing list