[Clang][Driver] Override complex number calculation method by -fno-fast-math #132680
Conversation
…st-math This patch fixes a bug where -fno-fast-math doesn't revert the complex number calculation method to the default. The priority of overriding options related to complex number calculations differs slightly from GCC, as discussed in: https://discourse.llvm.org/t/the-priority-of-fno-fast-math-regarding-complex-number-calculations/84679
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver Author: None (s-watanabe314) Changes…st-math This patch fixes a bug where -fno-fast-math doesn't revert the complex number calculation method to the default. The priority of overriding options related to complex number calculations differs slightly from GCC, as discussed in: I will post another patch to add a warning that the option overriding rules are incompatible with GCC. Full diff: https://github.com/llvm/llvm-project/pull/132680.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index fb3ed2db0e3c0..484ced9885883 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2997,6 +2997,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None;
std::string ComplexRangeStr = "";
std::string GccRangeComplexOption = "";
+ std::string LastComplexRangeOption = "";
auto setComplexRange = [&](LangOptions::ComplexRangeKind NewRange) {
// Warn if user expects to perform full implementation of complex
@@ -3015,7 +3016,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
if (Aggressive) {
HonorINFs = false;
HonorNaNs = false;
- setComplexRange(LangOptions::ComplexRangeKind::CX_Basic);
+ // If the last specified option related to complex range is
+ // -fno-fast-math, override 'Range' without warning.
+ if (LastComplexRangeOption == "-fno-fast-math")
+ Range = LangOptions::ComplexRangeKind::CX_Basic;
+ else
+ setComplexRange(LangOptions::ComplexRangeKind::CX_Basic);
} else {
HonorINFs = true;
HonorNaNs = true;
@@ -3080,6 +3086,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
EmitComplexRangeDiag(D, GccRangeComplexOption, "-fcx-limited-range");
}
GccRangeComplexOption = "-fcx-limited-range";
+ LastComplexRangeOption = "-fcx-limited-range";
Range = LangOptions::ComplexRangeKind::CX_Basic;
break;
case options::OPT_fno_cx_limited_range:
@@ -3093,6 +3100,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
"-fno-cx-limited-range");
}
GccRangeComplexOption = "-fno-cx-limited-range";
+ LastComplexRangeOption = "-fno-cx-limited-range";
Range = LangOptions::ComplexRangeKind::CX_Full;
break;
case options::OPT_fcx_fortran_rules:
@@ -3102,6 +3110,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
else
EmitComplexRangeDiag(D, GccRangeComplexOption, "-fcx-fortran-rules");
GccRangeComplexOption = "-fcx-fortran-rules";
+ LastComplexRangeOption = "-fcx-fortran-rules";
Range = LangOptions::ComplexRangeKind::CX_Improved;
break;
case options::OPT_fno_cx_fortran_rules:
@@ -3114,6 +3123,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
"-fno-cx-fortran-rules");
}
GccRangeComplexOption = "-fno-cx-fortran-rules";
+ LastComplexRangeOption = "-fno-cx-fortran-rules";
Range = LangOptions::ComplexRangeKind::CX_Full;
break;
case options::OPT_fcomplex_arithmetic_EQ: {
@@ -3148,6 +3158,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
ComplexArithmeticStr(RangeVal));
}
}
+ LastComplexRangeOption = "-fcomplex-arithmetic";
Range = RangeVal;
break;
}
@@ -3201,6 +3212,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
} else
D.Diag(diag::err_drv_unsupported_option_argument)
<< A->getSpelling() << Val;
+ LastComplexRangeOption = "-ffp-model";
break;
}
@@ -3386,6 +3398,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
[[fallthrough]];
case options::OPT_ffast_math:
applyFastMath(true);
+ LastComplexRangeOption = "-ffast-math";
if (A->getOption().getID() == options::OPT_Ofast)
LastFpContractOverrideOption = "-Ofast";
else
@@ -3403,6 +3416,13 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
ApproxFunc = false;
SignedZeros = true;
restoreFPContractState();
+ // If the last specified option related to complex range is -ffast-math,
+ // override 'Range' without warning.
+ if (LastComplexRangeOption == "-ffast-math")
+ Range = LangOptions::ComplexRangeKind::CX_Full;
+ else
+ setComplexRange(LangOptions::ComplexRangeKind::CX_Full);
+ LastComplexRangeOption = "-fno-fast-math";
LastFpContractOverrideOption = "";
break;
} // End switch (A->getOption().getID())
diff --git a/clang/test/Driver/range.c b/clang/test/Driver/range.c
index da5748d7c723c..315b54ea693a2 100644
--- a/clang/test/Driver/range.c
+++ b/clang/test/Driver/range.c
@@ -177,14 +177,45 @@
// RUN: %clang -### -target x86_64 -ffast-math -fcomplex-arithmetic=basic -c %s 2>&1 \
// RUN: | FileCheck --check-prefix=BASIC %s
-// BASIC: -complex-range=basic
-// FULL: -complex-range=full
-// PRMTD: -complex-range=promoted
-// BASIC-NOT: -complex-range=improved
-// CHECK-NOT: -complex-range=basic
-// IMPRVD: -complex-range=improved
-// IMPRVD-NOT: -complex-range=basic
-// CHECK-NOT: -complex-range=improved
+// RUN: %clang -### -target x86_64 -fcx-limited-range -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN21 %s
+
+// RUN: %clang -### -Werror -target x86_64 -fno-cx-limited-range -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -target x86_64 -fcx-fortran-rules -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN22 %s
+
+// RUN: %clang -### -Werror -target x86_64 -fno-cx-fortran-rules -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -Werror -target x86_64 -ffast-math -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -target x86_64 -fcomplex-arithmetic=basic -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN23 %s
+
+// RUN: %clang -### -target x86_64 -fcomplex-arithmetic=promoted -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN24 %s
+
+// RUN: %clang -### -target x86_64 -fcomplex-arithmetic=improved -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN25 %s
+
+// RUN: %clang -### -Werror -target x86_64 -fcomplex-arithmetic=full -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -target x86_64 -ffp-model=aggressive -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN23 %s
+
+// RUN: %clang -### -target x86_64 -ffp-model=fast -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN24 %s
+
+// RUN: %clang -### -Werror -target x86_64 -ffp-model=precise -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -Werror -target x86_64 -ffp-model=strict -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %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]
@@ -196,5 +227,19 @@
// WARN14: overriding '-complex-range=promoted' option with '-fcx-limited-range' [-Woverriding-option]
// WARN17: warning: overriding '-fcomplex-arithmetic=full' option with '-fcomplex-arithmetic=basic' [-Woverriding-option]
// WARN20: warning: overriding '-fcx-fortran-rules' option with '-fcx-limited-range' [-Woverriding-option]
+// WARN21: warning: overriding '-fcx-limited-range' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN22: warning: overriding '-fcx-fortran-rules' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN23: warning: overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN24: warning: overriding '-fcomplex-arithmetic=promoted' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN25: warning: overriding '-fcomplex-arithmetic=improved' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+
+// BASIC: -complex-range=basic
+// FULL: -complex-range=full
+// PRMTD: -complex-range=promoted
+// BASIC-NOT: -complex-range=improved
+// CHECK-NOT: -complex-range=basic
+// IMPRVD: -complex-range=improved
+// IMPRVD-NOT: -complex-range=basic
+// CHECK-NOT: -complex-range=improved
// ERR: error: unsupported argument 'foo' to option '-fcomplex-arithmetic='
|
|
@llvm/pr-subscribers-clang Author: None (s-watanabe314) Changes…st-math This patch fixes a bug where -fno-fast-math doesn't revert the complex number calculation method to the default. The priority of overriding options related to complex number calculations differs slightly from GCC, as discussed in: I will post another patch to add a warning that the option overriding rules are incompatible with GCC. Full diff: https://github.com/llvm/llvm-project/pull/132680.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index fb3ed2db0e3c0..484ced9885883 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2997,6 +2997,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None;
std::string ComplexRangeStr = "";
std::string GccRangeComplexOption = "";
+ std::string LastComplexRangeOption = "";
auto setComplexRange = [&](LangOptions::ComplexRangeKind NewRange) {
// Warn if user expects to perform full implementation of complex
@@ -3015,7 +3016,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
if (Aggressive) {
HonorINFs = false;
HonorNaNs = false;
- setComplexRange(LangOptions::ComplexRangeKind::CX_Basic);
+ // If the last specified option related to complex range is
+ // -fno-fast-math, override 'Range' without warning.
+ if (LastComplexRangeOption == "-fno-fast-math")
+ Range = LangOptions::ComplexRangeKind::CX_Basic;
+ else
+ setComplexRange(LangOptions::ComplexRangeKind::CX_Basic);
} else {
HonorINFs = true;
HonorNaNs = true;
@@ -3080,6 +3086,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
EmitComplexRangeDiag(D, GccRangeComplexOption, "-fcx-limited-range");
}
GccRangeComplexOption = "-fcx-limited-range";
+ LastComplexRangeOption = "-fcx-limited-range";
Range = LangOptions::ComplexRangeKind::CX_Basic;
break;
case options::OPT_fno_cx_limited_range:
@@ -3093,6 +3100,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
"-fno-cx-limited-range");
}
GccRangeComplexOption = "-fno-cx-limited-range";
+ LastComplexRangeOption = "-fno-cx-limited-range";
Range = LangOptions::ComplexRangeKind::CX_Full;
break;
case options::OPT_fcx_fortran_rules:
@@ -3102,6 +3110,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
else
EmitComplexRangeDiag(D, GccRangeComplexOption, "-fcx-fortran-rules");
GccRangeComplexOption = "-fcx-fortran-rules";
+ LastComplexRangeOption = "-fcx-fortran-rules";
Range = LangOptions::ComplexRangeKind::CX_Improved;
break;
case options::OPT_fno_cx_fortran_rules:
@@ -3114,6 +3123,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
"-fno-cx-fortran-rules");
}
GccRangeComplexOption = "-fno-cx-fortran-rules";
+ LastComplexRangeOption = "-fno-cx-fortran-rules";
Range = LangOptions::ComplexRangeKind::CX_Full;
break;
case options::OPT_fcomplex_arithmetic_EQ: {
@@ -3148,6 +3158,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
ComplexArithmeticStr(RangeVal));
}
}
+ LastComplexRangeOption = "-fcomplex-arithmetic";
Range = RangeVal;
break;
}
@@ -3201,6 +3212,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
} else
D.Diag(diag::err_drv_unsupported_option_argument)
<< A->getSpelling() << Val;
+ LastComplexRangeOption = "-ffp-model";
break;
}
@@ -3386,6 +3398,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
[[fallthrough]];
case options::OPT_ffast_math:
applyFastMath(true);
+ LastComplexRangeOption = "-ffast-math";
if (A->getOption().getID() == options::OPT_Ofast)
LastFpContractOverrideOption = "-Ofast";
else
@@ -3403,6 +3416,13 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
ApproxFunc = false;
SignedZeros = true;
restoreFPContractState();
+ // If the last specified option related to complex range is -ffast-math,
+ // override 'Range' without warning.
+ if (LastComplexRangeOption == "-ffast-math")
+ Range = LangOptions::ComplexRangeKind::CX_Full;
+ else
+ setComplexRange(LangOptions::ComplexRangeKind::CX_Full);
+ LastComplexRangeOption = "-fno-fast-math";
LastFpContractOverrideOption = "";
break;
} // End switch (A->getOption().getID())
diff --git a/clang/test/Driver/range.c b/clang/test/Driver/range.c
index da5748d7c723c..315b54ea693a2 100644
--- a/clang/test/Driver/range.c
+++ b/clang/test/Driver/range.c
@@ -177,14 +177,45 @@
// RUN: %clang -### -target x86_64 -ffast-math -fcomplex-arithmetic=basic -c %s 2>&1 \
// RUN: | FileCheck --check-prefix=BASIC %s
-// BASIC: -complex-range=basic
-// FULL: -complex-range=full
-// PRMTD: -complex-range=promoted
-// BASIC-NOT: -complex-range=improved
-// CHECK-NOT: -complex-range=basic
-// IMPRVD: -complex-range=improved
-// IMPRVD-NOT: -complex-range=basic
-// CHECK-NOT: -complex-range=improved
+// RUN: %clang -### -target x86_64 -fcx-limited-range -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN21 %s
+
+// RUN: %clang -### -Werror -target x86_64 -fno-cx-limited-range -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -target x86_64 -fcx-fortran-rules -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN22 %s
+
+// RUN: %clang -### -Werror -target x86_64 -fno-cx-fortran-rules -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -Werror -target x86_64 -ffast-math -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -target x86_64 -fcomplex-arithmetic=basic -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN23 %s
+
+// RUN: %clang -### -target x86_64 -fcomplex-arithmetic=promoted -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN24 %s
+
+// RUN: %clang -### -target x86_64 -fcomplex-arithmetic=improved -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN25 %s
+
+// RUN: %clang -### -Werror -target x86_64 -fcomplex-arithmetic=full -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -target x86_64 -ffp-model=aggressive -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN23 %s
+
+// RUN: %clang -### -target x86_64 -ffp-model=fast -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN24 %s
+
+// RUN: %clang -### -Werror -target x86_64 -ffp-model=precise -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s
+
+// RUN: %clang -### -Werror -target x86_64 -ffp-model=strict -fno-fast-math \
+// RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %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]
@@ -196,5 +227,19 @@
// WARN14: overriding '-complex-range=promoted' option with '-fcx-limited-range' [-Woverriding-option]
// WARN17: warning: overriding '-fcomplex-arithmetic=full' option with '-fcomplex-arithmetic=basic' [-Woverriding-option]
// WARN20: warning: overriding '-fcx-fortran-rules' option with '-fcx-limited-range' [-Woverriding-option]
+// WARN21: warning: overriding '-fcx-limited-range' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN22: warning: overriding '-fcx-fortran-rules' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN23: warning: overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN24: warning: overriding '-fcomplex-arithmetic=promoted' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+// WARN25: warning: overriding '-fcomplex-arithmetic=improved' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
+
+// BASIC: -complex-range=basic
+// FULL: -complex-range=full
+// PRMTD: -complex-range=promoted
+// BASIC-NOT: -complex-range=improved
+// CHECK-NOT: -complex-range=basic
+// IMPRVD: -complex-range=improved
+// IMPRVD-NOT: -complex-range=basic
+// CHECK-NOT: -complex-range=improved
// ERR: error: unsupported argument 'foo' to option '-fcomplex-arithmetic='
|
| @@ -2997,6 +2997,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D, | |||
| LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None; | |||
| std::string ComplexRangeStr = ""; | |||
| std::string GccRangeComplexOption = ""; | |||
| std::string LastComplexRangeOption = ""; | |||
There was a problem hiding this comment.
I don't think I remember during our Discourse discussion of this that there was already a GccRangeComplexOption variable here. Do we really need another variable? If you just set GccRangeComplexOption to empty for -ffast-math and -fno-fast-math, would that give us reasonable diagnostics?
There was a problem hiding this comment.
Thank you for your feedback!
If you just set GccRangeComplexOption to empty for -ffast-math and -fno-fast-math, would that give us reasonable diagnostics?
I think it's difficult. I believe diagnostics should be emitted when Range specified with options other than -ffast-math is overridden by -fno-fast-math. To distinguish between these cases, I think we need a variable that determines what options, including Clang-specific options, were specified before -fno-fast-math. Even if we empty GccRangeComplexOption when -ffast-math is spcified, it's impossible to distinguish whether the preceding option before -fno-fast-math was -ffast-math or -fcomplex-arithmetic=basic. If it's acceptable to assign "-ffp-model" or "-fcomplex-arithmetic" to GccRangeComplexOption, then a new variable might not be necessary.
There was a problem hiding this comment.
std::string LastComplexRangeOption default initialization
There was a problem hiding this comment.
Does this mean that = "" is unnecessary? I initialized it in the same way as GccRangeComplexOption, but should I remove it?
There was a problem hiding this comment.
= "" is unnecessary. The newly introduced variable can drop = "". You can also drop = "" for the two existing variables as this is minor change.
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -Werror -target x86_64 -ffp-model=precise -fno-fast-math \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s | ||
|
|
||
| // RUN: %clang -### -Werror -target x86_64 -ffp-model=strict -fno-fast-math \ |
There was a problem hiding this comment.
Can you add test cases for when -fno-fast-math is followed by other options that change the complex mode?
There was a problem hiding this comment.
I added test cases.
There was a problem hiding this comment.
I'm not comfortable with how warnings are being issued here. It's going to be tricky to get this right, and I don't think we're there. I would like to see warnings for -fno-fast-math overriding explicit complex range options, especially since GCC doesn't do that. This may contradict things I've said earlier, but it doesn't seem like we should treat the GCC options differently than the clang-specific options with regard to whether or not we issue warnings. Also, the warnings are referencing options that the user didn't explicitly specify.
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -target x86_64 -fno-fast-math -fcx-limited-range \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=BASIC,WARN26 %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -fno-fast-math -fno-cx-limited-range \ |
There was a problem hiding this comment.
| // RUN: %clang -### -target x86_64 -fno-fast-math -fno-cx-limited-range \ | |
| // RUN: %clang -### -Werror -target x86_64 -fno-fast-math -fno-cx-limited-range \ |
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -target x86_64 -fno-fast-math -fcx-fortran-rules \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=IMPRVD,WARN27 %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -fno-fast-math -fno-cx-fortran-rules \ |
There was a problem hiding this comment.
| // RUN: %clang -### -target x86_64 -fno-fast-math -fno-cx-fortran-rules \ | |
| // RUN: %clang -### -Werror -target x86_64 -fno-fast-math -fno-cx-fortran-rules \ |
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -Werror -target x86_64 -fno-fast-math -ffast-math \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=BASIC %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -fno-fast-math -fcomplex-arithmetic=basic \ |
There was a problem hiding this comment.
This should have the same behavior as the -fno-fast-math -fcx-fortran-rules case. I don't think it should warn, but if either warns both should warn.
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -Werror -target x86_64 -fcomplex-arithmetic=full -fno-fast-math \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -ffp-model=aggressive -fno-fast-math \ |
There was a problem hiding this comment.
I'm not sure this should result in a warning, and if it does the fact that the warning is about complex is likely to confuse some users. The test is checking for this:
warning: overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=full' [-Woverriding-option]
But neither of those options was explicitly used.
clang/test/Driver/range.c
Outdated
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN23 %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -ffp-model=fast -fno-fast-math \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL,WARN24 %s |
There was a problem hiding this comment.
As above, I don't think there should be a warning here.
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -Werror -target x86_64 -ffp-model=strict -fno-fast-math \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -fno-fast-math -fcx-limited-range \ |
There was a problem hiding this comment.
This warning is also debateable. The -fno-fast-math option resets to the default state, so a warning after that seems pedantic.
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -target x86_64 -fno-fast-math -fcomplex-arithmetic=basic \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=BASIC %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -fno-fast-math -fcomplex-arithmetic=promoted \ |
There was a problem hiding this comment.
Do this case and the next cause warnings? If not, I don't see why we're treating them differently than the GCC options.
clang/test/Driver/range.c
Outdated
| // RUN: %clang -### -Werror -target x86_64 -fno-fast-math -fcomplex-arithmetic=full \ | ||
| // RUN: -c %s 2>&1 | FileCheck --check-prefixes=FULL %s | ||
|
|
||
| // RUN: %clang -### -target x86_64 -fno-fast-math -ffp-model=aggressive \ |
Is "explicit complex range options" referring to the
Furthermore, if
I agree that we shouldn't treat them differently. |
|
@andykaylor I would appreciate your comments on this. |
|
@andykaylor Ping. Could you please comment on the policy regarding warning messages? |
|
@s-watanabe314 I apologize for the very long delay in responding. This fell off my radar for a while. I think your latest proposed handling of warnings looks good, but I'd like to see input from @MaskRay also. |
|
Thank you for confirming. @MaskRay, do you have any comments regarding the handling of warning messages? |
MaskRay
changed the title
Sorry for the delayed response. Does this implement the last-option-win behavior as suggested in the last few comments of https://discourse.llvm.org/t/the-priority-of-fno-fast-math-regarding-complex-number-calculations/84679 ? If so, this looks good me. Another possible behavior is the 'more-specific-option-wins' approach adopted by some options. I am not familiar with the complex number options and don't have a strong opinion. Go with the suggestion by experts like andykaylor! |
|
@MaskRay |
|
@andykaylor I considered an implementation that wouldn't add a new variable |
I've never been happy with the way we handle the warnings where aggregate flags change the complex range setting. For instance, in the case you cite, I think we just need to commit to each change making progress relative to the previous state rather than trying to fix everything. I'll give your changes a closer review tomorrow, but I think you're on the right track. |
…st-math
This patch fixes a bug where -fno-fast-math doesn't revert the complex number calculation method to the default. The priority of overriding options related to complex number calculations differs slightly from GCC, as discussed in:
https://discourse.llvm.org/t/the-priority-of-fno-fast-math-regarding-complex-number-calculations/84679
I will post another patch to add a warning that the option overriding rules are incompatible with GCC.
In that patch, I will also fix the conditions for warnings related to other complex number options.