-
Notifications
You must be signed in to change notification settings - Fork 14.1k
[Clang][Driver] Warn on complex number range incompatibility with GCC #144468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This patch adds a warning for incompatible complex number behavior compared to GCC due to Clang's "last-flag-wins" rule. These were discussed at: https://discourse.llvm.org/t/the-priority-of-fno-fast-math-regarding-complex-number-calculations/84679
@llvm/pr-subscribers-clang-driver Author: Shunsuke Watanabe (s-watanabe314) ChangesThis patch adds a warning for incompatible complex number behavior compared to GCC due to Clang's "last-flag-wins" rule. Incompatibilities with GCC occur in the following seven cases. Clang uses the last specified option, but GCC uses an earlier specified option in these seven cases because it has a different option prioritization.
These were discussed at: Full diff: https://github.com/llvm/llvm-project/pull/144468.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 20fb47237c56f..377509cfcf2cd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -520,6 +520,9 @@ def warn_drv_math_errno_enabled_after_veclib: Warning<
"math errno enabled by '%0' after it was implicitly disabled by '%1',"
" this may limit the utilization of the vector library">,
InGroup<MathErrnoEnabledWithVecLib>;
+def warn_drv_gcc_incompatible_complex_range_override: Warning<
+ "combination of '%0' and '%1' results in complex number calculations incompatible with GCC">,
+ InGroup<GccCompat>;
def note_drv_command_failed_diag_msg : Note<
"diagnostic msg: %0">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 1d11be1d82be8..244c4c0848aa6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2915,6 +2915,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
} else {
if (GccRangeComplexOption != "-fno-cx-limited-range")
EmitComplexRangeDiag(D, GccRangeComplexOption, "-fcx-limited-range");
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules" ||
+ GccRangeComplexOption == "-fno-cx-fortran-rules")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
}
GccRangeComplexOption = "-fcx-limited-range";
LastComplexRangeOption = A->getSpelling();
@@ -2929,6 +2934,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
GccRangeComplexOption != "-fno-cx-fortran-rules")
EmitComplexRangeDiag(D, GccRangeComplexOption,
"-fno-cx-limited-range");
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
}
GccRangeComplexOption = "-fno-cx-limited-range";
LastComplexRangeOption = A->getSpelling();
@@ -3231,6 +3240,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
case options::OPT_ffast_math:
applyFastMath(true);
LastComplexRangeOption = A->getSpelling();
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules" ||
+ GccRangeComplexOption == "-fno-cx-fortran-rules")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
if (A->getOption().getID() == options::OPT_Ofast)
LastFpContractOverrideOption = "-Ofast";
else
@@ -3255,6 +3269,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
Range != LangOptions::ComplexRangeKind::CX_Full)
EmitComplexRangeDiag(D, LastComplexRangeOption, "-fno-fast-math");
Range = LangOptions::ComplexRangeKind::CX_None;
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules" ||
+ GccRangeComplexOption == "-fcx-limited-range")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
LastComplexRangeOption = "";
GccRangeComplexOption = "";
LastFpContractOverrideOption = "";
diff --git a/clang/test/Driver/range.c b/clang/test/Driver/range.c
index 30140f3c208e0..d86bec46589a1 100644
--- a/clang/test/Driver/range.c
+++ b/clang/test/Driver/range.c
@@ -255,6 +255,31 @@
// RUN: %clang -### -Werror --target=x86_64 -fno-fast-math -ffp-model=strict \
// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// Test incompatibility of complex range override with GCC 14.2.0.
+
+// RUN: %clang -### --target=x86_64 -fcx-limited-range -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT1 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -fcx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT2 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -fno-cx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT3 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -ffast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT4 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT5 %s
+
+// RUN: %clang -### --target=x86_64 -fno-cx-fortran-rules -fcx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT6 %s
+
+// RUN: %clang -### --target=x86_64 -fno-cx-fortran-rules -ffast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT7 %s
+
+
// WARN1: warning: overriding '-fcx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
// WARN2: warning: overriding '-fno-cx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
// WARN3: warning: overriding '-fcx-fortran-rules' option with '-fno-cx-limited-range' [-Woverriding-option]
@@ -281,4 +306,12 @@
// CHECK-NOT: -complex-range=improved
// RANGE-NOT: -complex-range=
+// WARN_INCOMPAT1: warning: combination of '-fcx-limited-range' and '-fno-fast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT2: warning: combination of '-fcx-fortran-rules' and '-fcx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT3: warning: combination of '-fcx-fortran-rules' and '-fno-cx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT4: warning: combination of '-fcx-fortran-rules' and '-ffast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT5: warning: combination of '-fcx-fortran-rules' and '-fno-fast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT6: warning: combination of '-fno-cx-fortran-rules' and '-fcx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT7: warning: combination of '-fno-cx-fortran-rules' and '-ffast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+
// ERR: error: unsupported argument 'foo' to option '-fcomplex-arithmetic='
|
@llvm/pr-subscribers-clang Author: Shunsuke Watanabe (s-watanabe314) ChangesThis patch adds a warning for incompatible complex number behavior compared to GCC due to Clang's "last-flag-wins" rule. Incompatibilities with GCC occur in the following seven cases. Clang uses the last specified option, but GCC uses an earlier specified option in these seven cases because it has a different option prioritization.
These were discussed at: Full diff: https://github.com/llvm/llvm-project/pull/144468.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 20fb47237c56f..377509cfcf2cd 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -520,6 +520,9 @@ def warn_drv_math_errno_enabled_after_veclib: Warning<
"math errno enabled by '%0' after it was implicitly disabled by '%1',"
" this may limit the utilization of the vector library">,
InGroup<MathErrnoEnabledWithVecLib>;
+def warn_drv_gcc_incompatible_complex_range_override: Warning<
+ "combination of '%0' and '%1' results in complex number calculations incompatible with GCC">,
+ InGroup<GccCompat>;
def note_drv_command_failed_diag_msg : Note<
"diagnostic msg: %0">;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 1d11be1d82be8..244c4c0848aa6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2915,6 +2915,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
} else {
if (GccRangeComplexOption != "-fno-cx-limited-range")
EmitComplexRangeDiag(D, GccRangeComplexOption, "-fcx-limited-range");
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules" ||
+ GccRangeComplexOption == "-fno-cx-fortran-rules")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
}
GccRangeComplexOption = "-fcx-limited-range";
LastComplexRangeOption = A->getSpelling();
@@ -2929,6 +2934,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
GccRangeComplexOption != "-fno-cx-fortran-rules")
EmitComplexRangeDiag(D, GccRangeComplexOption,
"-fno-cx-limited-range");
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
}
GccRangeComplexOption = "-fno-cx-limited-range";
LastComplexRangeOption = A->getSpelling();
@@ -3231,6 +3240,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
case options::OPT_ffast_math:
applyFastMath(true);
LastComplexRangeOption = A->getSpelling();
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules" ||
+ GccRangeComplexOption == "-fno-cx-fortran-rules")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
if (A->getOption().getID() == options::OPT_Ofast)
LastFpContractOverrideOption = "-Ofast";
else
@@ -3255,6 +3269,11 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
Range != LangOptions::ComplexRangeKind::CX_Full)
EmitComplexRangeDiag(D, LastComplexRangeOption, "-fno-fast-math");
Range = LangOptions::ComplexRangeKind::CX_None;
+ // Warn about complex range option overrides incompatible with GCC.
+ if (GccRangeComplexOption == "-fcx-fortran-rules" ||
+ GccRangeComplexOption == "-fcx-limited-range")
+ D.Diag(clang::diag::warn_drv_gcc_incompatible_complex_range_override)
+ << GccRangeComplexOption << A->getSpelling();
LastComplexRangeOption = "";
GccRangeComplexOption = "";
LastFpContractOverrideOption = "";
diff --git a/clang/test/Driver/range.c b/clang/test/Driver/range.c
index 30140f3c208e0..d86bec46589a1 100644
--- a/clang/test/Driver/range.c
+++ b/clang/test/Driver/range.c
@@ -255,6 +255,31 @@
// RUN: %clang -### -Werror --target=x86_64 -fno-fast-math -ffp-model=strict \
// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// Test incompatibility of complex range override with GCC 14.2.0.
+
+// RUN: %clang -### --target=x86_64 -fcx-limited-range -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT1 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -fcx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT2 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -fno-cx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT3 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -ffast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT4 %s
+
+// RUN: %clang -### --target=x86_64 -fcx-fortran-rules -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT5 %s
+
+// RUN: %clang -### --target=x86_64 -fno-cx-fortran-rules -fcx-limited-range \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT6 %s
+
+// RUN: %clang -### --target=x86_64 -fno-cx-fortran-rules -ffast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=WARN_INCOMPAT7 %s
+
+
// WARN1: warning: overriding '-fcx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
// WARN2: warning: overriding '-fno-cx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
// WARN3: warning: overriding '-fcx-fortran-rules' option with '-fno-cx-limited-range' [-Woverriding-option]
@@ -281,4 +306,12 @@
// CHECK-NOT: -complex-range=improved
// RANGE-NOT: -complex-range=
+// WARN_INCOMPAT1: warning: combination of '-fcx-limited-range' and '-fno-fast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT2: warning: combination of '-fcx-fortran-rules' and '-fcx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT3: warning: combination of '-fcx-fortran-rules' and '-fno-cx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT4: warning: combination of '-fcx-fortran-rules' and '-ffast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT5: warning: combination of '-fcx-fortran-rules' and '-fno-fast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT6: warning: combination of '-fno-cx-fortran-rules' and '-fcx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+// WARN_INCOMPAT7: warning: combination of '-fno-cx-fortran-rules' and '-ffast-math' results in complex number calculations incompatible with GCC [-Wgcc-compat]
+
// ERR: error: unsupported argument 'foo' to option '-fcomplex-arithmetic='
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this! Please be sure to add a release note to clang/docs/ReleaseNotes.rst
so users know about the new diagnostic.
@@ -520,6 +520,9 @@ def warn_drv_math_errno_enabled_after_veclib: Warning< | |||
"math errno enabled by '%0' after it was implicitly disabled by '%1'," | |||
" this may limit the utilization of the vector library">, | |||
InGroup<MathErrnoEnabledWithVecLib>; | |||
def warn_drv_gcc_incompatible_complex_range_override: Warning< | |||
"combination of '%0' and '%1' results in complex number calculations incompatible with GCC">, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we perhaps add note diagnostics or a %select
to the warning so we can explain how it's incompatible? I'm worried a user will get this diagnostic and say "okay, great, but... what does that mean and how do I fix that?" and being a bit stuck because they don't know what's incompatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback! I agree that a user wouldn't know how to handle this warning just by seeing it. How about adding the following note diagnostic and outputting it along with the incompatibility warning?
note: complex multiplication and division use different calculation methods than GCC. specify '%0' after '%1' for GCC compatibility
Implementing this would result in the following output:
$ clang test.cpp -fcx-fortran-rules -fcx-limited-range
clang: warning: overriding '-fcx-fortran-rules' option with '-fcx-limited-range' [-Woverriding-option]
clang: warning: combination of '-fcx-fortran-rules' and '-fcx-limited-range' results in complex number calculations incompatible with GCC [-Wgcc-compat]
clang: note: complex multiplication and division use different calculation methods than GCC. specify '-fcx-fortran-rules' after '-fcx-limited-range' for GCC compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few thoughts:
- It's unfortunate that we emit two warnings for the same thing, especially by default. We really should try to find a way to emit only one warning, I think.
- The "different calculation methods" part of the note doesn't really tell the user all that much, but the important part is the "specify '-blah' after '-yada'" bit and that's good to keep.
WDYT?
This patch adds a warning for incompatible complex number behavior compared to GCC due to Clang's "last-flag-wins" rule.
Incompatibilities with GCC occur in the following seven cases. Clang uses the last specified option, but GCC uses an earlier specified option in these seven cases because it has a different option prioritization.
These were discussed at:
https://discourse.llvm.org/t/the-priority-of-fno-fast-math-regarding-complex-number-calculations/84679