[Driver] Clean up fp-contract handling in clang driver #91271
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver Author: Andy Kaylor (andykaylor) ChangesThis change refactors the fp-contract handling in Full diff: https://github.com/llvm/llvm-project/pull/91271.diff 3 Files Affected:
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"
|
|
@llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis change refactors the fp-contract handling in Full diff: https://github.com/llvm/llvm-project/pull/91271.diff 3 Files Affected:
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"
|
zahiraam
reviewed
May 7, 2024
| @@ -2893,7 +2915,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, | |||
| AssociativeMath = false; | |||
| ReciprocalMath = false; | |||
| SignedZeros = true; | |||
| FPContract = "on"; | |||
There was a problem hiding this comment.
Not quite sure why this is removed from here?
There was a problem hiding this comment.
It was unnecessary. Only one of the three fp-models sets FPContract to "on" and that's happening explicitly below. The code here is setting everything to what it would be for fp-model=precise and then resetting it immediately if we're handling -ffp-model=fast. Except for the FPContract state, these are right for fp-model strict as well.
MaskRay
reviewed
May 10, 2024
| @@ -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 \ | |||
There was a problem hiding this comment.
L161 has similar -funsafe-math-optimizations -ffp-contract=off checks. The test can be folded there.
There was a problem hiding this comment.
I just went ahead and added a bunch of -Werror to previous lines in 514d80b
There was a problem hiding this comment.
That introduces a merge conflict for this patch. Is there a way I can solve that without having to do a rebase and force push for this, which kills the ability to see what changed between updates? I guess with a small-ish patch like this that's not a big problem, but it's something I try to avoid in general.
There was a problem hiding this comment.
Some folks prefer git merge origin/main while some just use git rebase origin/main and force push.
I am fine with either style, but someone really dislike force push (perhaps not for such a trivial change).
MaskRay
reviewed
May 10, 2024
| @@ -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; | |||
There was a problem hiding this comment.
Overridden seems more appropriate, but the variable name will become too long.
I wonder whether Last can be removed. It's clear from the type that this can only hold one option name.
MaskRay
added a commit
that referenced
this pull request
May 10, 2024
Test that these combinations do not lead to a warning. -funsafe-math-optimizations -ffp-contract=off and -ffast-math -ffp-contract=off may issue a warning after #91271.
|
Can this be closed then? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.