Author: Zahira Ammarguellat Date: 2022-04-05T04:58:19-07:00 New Revision: 4d165ad7d9b3395a59c287ef60542b4de3a4d95a
URL: https://github.com/llvm/llvm-project/commit/4d165ad7d9b3395a59c287ef60542b4de3a4d95a DIFF: https://github.com/llvm/llvm-project/commit/4d165ad7d9b3395a59c287ef60542b4de3a4d95a.diff LOG: In fast-math mode, when unsafe math optimizations are enabled, the compiler is allowed to use optimizations that allow reassociation and transformations that don’t guaranty accuracy. For example (x+y)+z is transformed into x+(y+z) . Although mathematically equivalent, these two expressions may not lead to the same final result due to errors of summation. Or x/x is transformed into 1.0 but x could be 0.0, INF or NaN. And so this transformation also may not lead to the same final result. Setting the eval method 'ffp-eval-method' or via '#pragma clang fp eval_method' in this mode, doesn’t have any effect. This patch adds code to warn the user of this. Differential Revision: https://reviews.llvm.org/D122155 Added: clang/test/Driver/eval-method-with-unsafe-math.c clang/test/Sema/eval-method-with-unsafe-math.c Modified: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Sema/SemaAttr.cpp clang/test/CodeGen/X86/32bit-behavior.c Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index eacb7e4de0ead..afc166b5d087a 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -48,6 +48,10 @@ def warn_fe_backend_unsupported_fp_exceptions : Warning< "overriding currently unsupported use of floating point exceptions " "on this target">, InGroup<UnsupportedFPOpt>; +def err_incompatible_fp_eval_method_options : Error< + "option 'ffp-eval-method' cannot be used with option " + "%select{'fapprox-func'|'mreassociate'|'freciprocal'}0">; + def remark_fe_backend_optimization_remark : Remark<"%0">, BackendInfo, InGroup<BackendOptimizationRemark>; def remark_fe_backend_optimization_remark_missed : Remark<"%0">, BackendInfo, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index aec172c39ed9a..c5171359e7e4c 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6491,6 +6491,10 @@ def warn_floatingpoint_eq : Warning< "comparing floating point with == or != is unsafe">, InGroup<DiagGroup<"float-equal">>, DefaultIgnore; +def err_setting_eval_method_used_in_unsafe_context : Error < + "%select{'#pragma clang fp eval_method'|option 'ffp-eval-method'}0 cannot be used with " + "%select{option 'fapprox-func'|option 'mreassociate'|option 'freciprocal'|option 'ffp-eval-method'|'#pragma clang fp reassociate'}1">; + def warn_remainder_division_by_zero : Warning< "%select{remainder|division}0 by zero is undefined">, InGroup<DivZero>; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index f8fd3a2900237..91adacdee3ad7 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -503,6 +503,22 @@ static bool FixupInvocation(CompilerInvocation &Invocation, Diags.Report(diag::warn_ignored_hip_only_option) << Args.getLastArg(OPT_gpu_max_threads_per_block_EQ)->getAsString(Args); + // When these options are used, the compiler is allowed to apply + // optimizations that may affect the final result. For example + // (x+y)+z is transformed to x+(y+z) but may not give the same + // final result; it's not value safe. + // Another example can be to simplify x/x to 1.0 but x could be 0.0, INF + // or NaN. Final result may then diff er. An error is issued when the eval + // method is set with one of these options. + if (Args.hasArg(OPT_ffp_eval_method_EQ)) { + if (LangOpts.ApproxFunc) + Diags.Report(diag::err_incompatible_fp_eval_method_options) << 0; + if (LangOpts.AllowFPReassoc) + Diags.Report(diag::err_incompatible_fp_eval_method_options) << 1; + if (LangOpts.AllowRecip) + Diags.Report(diag::err_incompatible_fp_eval_method_options) << 2; + } + // -cl-strict-aliasing needs to emit diagnostic in the case where CL > 1.0. // This option should be deprecated for CL > 1.0 because // this option was added for compatibility with OpenCL 1.0. diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 9e94136ce76d0..113b3ac89f463 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -486,6 +486,12 @@ void Sema::ActOnPragmaFPEvalMethod(SourceLocation Loc, NewFPFeatures.setFPEvalMethodOverride(LangOptions::FEM_Extended); break; } + if (getLangOpts().ApproxFunc) + Diag(Loc, diag::err_setting_eval_method_used_in_unsafe_context) << 0 << 0; + if (getLangOpts().AllowFPReassoc) + Diag(Loc, diag::err_setting_eval_method_used_in_unsafe_context) << 0 << 1; + if (getLangOpts().AllowRecip) + Diag(Loc, diag::err_setting_eval_method_used_in_unsafe_context) << 0 << 2; FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures); CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts()); PP.setCurrentFPEvalMethod(Loc, Value); @@ -1153,6 +1159,23 @@ void Sema::ActOnPragmaFPContract(SourceLocation Loc, } void Sema::ActOnPragmaFPReassociate(SourceLocation Loc, bool IsEnabled) { + if (IsEnabled) { + // For value unsafe context, combining this pragma with eval method + // setting is not recommended. See comment in function FixupInvocation#506. + int Reason = -1; + if (getLangOpts().getFPEvalMethod() != LangOptions::FEM_UnsetOnCommandLine) + // Eval method set using the option 'ffp-eval-method'. + Reason = 1; + if (PP.getLastFPEvalPragmaLocation().isValid()) + // Eval method set using the '#pragma clang fp eval_method'. + // We could have both an option and a pragma used to the set the eval + // method. The pragma overrides the option in the command line. The Reason + // of the diagnostic is overriden too. + Reason = 0; + if (Reason != -1) + Diag(Loc, diag::err_setting_eval_method_used_in_unsafe_context) + << Reason << 4; + } FPOptionsOverride NewFPFeatures = CurFPFeatureOverrides(); NewFPFeatures.setAllowFPReassociateOverride(IsEnabled); FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures); diff --git a/clang/test/CodeGen/X86/32bit-behavior.c b/clang/test/CodeGen/X86/32bit-behavior.c index a7e0f008c9f35..67a4cc86ac15b 100644 --- a/clang/test/CodeGen/X86/32bit-behavior.c +++ b/clang/test/CodeGen/X86/32bit-behavior.c @@ -14,22 +14,6 @@ // RUN: -emit-llvm -o - %s -ffp-eval-method=extended \ // RUN: | FileCheck -check-prefix=CHECK-DBL %s -// SSE Fast Math -// RUN: %clang_cc1 -fexperimental-strict-floating-point \ -// RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature +sse \ -// RUN: -emit-llvm -o - %s -ffp-eval-method=source \ -// RUN: -ffast-math | FileCheck -check-prefix=CHECK-FM-SRC %s - -// RUN: %clang_cc1 -fexperimental-strict-floating-point \ -// RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature +sse \ -// RUN: -emit-llvm -o - %s -ffp-eval-method=double \ -// RUN: -ffast-math | FileCheck -check-prefix=CHECK-FM %s - -// RUN: %clang_cc1 -fexperimental-strict-floating-point \ -// RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature +sse \ -// RUN: -emit-llvm -o - %s -ffp-eval-method=extended \ -// RUN: -ffast-math | FileCheck -check-prefix=CHECK-FM %s - // NO SSE // RUN: %clang_cc1 -fexperimental-strict-floating-point \ // RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature -sse \ @@ -46,22 +30,6 @@ // RUN: -emit-llvm -o - %s -ffp-eval-method=extended \ // RUN: | FileCheck -check-prefix=CHECK-DBL %s -// NO SSE Fast Math -// RUN: %clang_cc1 -fexperimental-strict-floating-point \ -// RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature -sse \ -// RUN: -emit-llvm -o - %s -ffp-eval-method=source \ -// RUN: -ffast-math | FileCheck -check-prefix=CHECK %s - -// RUN: %clang_cc1 -fexperimental-strict-floating-point \ -// RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature -sse \ -// RUN: -emit-llvm -o - %s -ffp-eval-method=double \ -// RUN: -ffast-math | FileCheck -check-prefix=CHECK-DBL-FM %s - -// RUN: %clang_cc1 -fexperimental-strict-floating-point \ -// RUN: -triple i386-pc-windows -target-cpu pentium4 -target-feature -sse \ -// RUN: -emit-llvm -o - %s -ffp-eval-method=extended \ -// RUN: -ffast-math | FileCheck -check-prefix=CHECK-DBL-FM %s - float addit(float a, float b, float c) { // CHECK-SRC: load float, float* // CHECK-SRC: load float, float* @@ -69,21 +37,6 @@ float addit(float a, float b, float c) { // CHECK-SRC: load float, float* // CHECK-SRC: fadd float - // CHECK-FM-SRC: load float, float* - // CHECK-FM-SRC: load float, float* - // CHECK-FM-SRC: fadd reassoc nnan ninf nsz arcp afn float - // CHECK-FM-SRC: load float, float* - // CHECK-FM-SRC: fadd reassoc nnan ninf nsz arcp afn float - - // CHECK-FM: load float, float* - // CHECK-FM: fpext float {{.*}} to double - // CHECK-FM: load float, float* - // CHECK-FM: fpext float {{.*}} to double - // CHECK-FM: fadd reassoc nnan ninf nsz arcp afn double - // CHECK-FM: load float, float* - // CHECK-FM: fadd reassoc nnan ninf nsz arcp afn double - // CHECK-FM: fptrunc double {{.*}} to float - // CHECK-DBL: load float, float* // CHECK-DBL: fpext float {{.*}} to double // CHECK-DBL: load float, float* @@ -94,16 +47,6 @@ float addit(float a, float b, float c) { // CHECK-DBL: fadd double // CHECK-DBL: fptrunc double {{.*}} to float - // CHECK-DBL-FM: load float, float* - // CHECK-DBL-FM: fpext float {{.*}} to double - // CHECK-DBL-FM: load float, float* - // CHECK-DBL-FM: fpext float {{.*}} to double - // CHECK-DBL-FM: fadd reassoc nnan ninf nsz arcp afn double - // CHECK-DBL-FM: load float, float* - // CHECK-DBL-FM: fpext float {{.*}} to double - // CHECK-DBL-FM: fadd reassoc nnan ninf nsz arcp afn double - // CHECK-DBL-FM: fptrunc double {{.*}} to float - // CHECK: ret float return a + b + c; } diff --git a/clang/test/Driver/eval-method-with-unsafe-math.c b/clang/test/Driver/eval-method-with-unsafe-math.c new file mode 100644 index 0000000000000..fb3c1da127dc3 --- /dev/null +++ b/clang/test/Driver/eval-method-with-unsafe-math.c @@ -0,0 +1,29 @@ +// RUN: not %clang -Xclang -fexperimental-strict-floating-point \ +// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -fapprox-func \ +// RUN: -Xclang -verify -ffp-eval-method=source %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-FUNC + +// RUN: not %clang -Xclang -fexperimental-strict-floating-point \ +// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -mreassociate \ +// RUN: -ffp-eval-method=source -Xclang -verify %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-ASSOC + +// RUN: not %clang -Xclang -fexperimental-strict-floating-point \ +// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -freciprocal-math \ +// RUN: -ffp-eval-method=source -Xclang -verify %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-RECPR + +// RUN: not %clang -Xclang -fexperimental-strict-floating-point \ +// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -freciprocal-math \ +// RUN: -Xclang -mreassociate -ffp-eval-method=source -Xclang -verify %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR + +// RUN: not %clang -Xclang -fexperimental-strict-floating-point \ +// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -freciprocal-math \ +// RUN: -Xclang -mreassociate -fapprox-func -ffp-eval-method=source \ +// RUN: -Xclang -verify %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR,CHECK-FUNC + +// CHECK-FUNC: (frontend): option 'ffp-eval-method' cannot be used with option 'fapprox-func' +// CHECK-ASSOC: (frontend): option 'ffp-eval-method' cannot be used with option 'mreassociate' +// CHECK-RECPR: (frontend): option 'ffp-eval-method' cannot be used with option 'freciprocal' diff --git a/clang/test/Sema/eval-method-with-unsafe-math.c b/clang/test/Sema/eval-method-with-unsafe-math.c new file mode 100644 index 0000000000000..b68dc6e7505d3 --- /dev/null +++ b/clang/test/Sema/eval-method-with-unsafe-math.c @@ -0,0 +1,56 @@ +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu \ +// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-PRGM + +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu -freciprocal-math \ +// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-RECPR,CHECK-PRGM + +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu -mreassociate \ +// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-PRGM + +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu -fapprox-func \ +// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FUNC,CHECK-PRGM + +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -verify \ +// RUN: %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR,CHECK-PRGM + +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -fapprox-func \ +// RUN: -verify %s 2>&1 \ +// RUN: | FileCheck %s --check-prefixes=CHECK-FUNC,CHECK-ASSOC,CHECK-RECPR,CHECK-PRGM + +// RUN: not %clang_cc1 -fexperimental-strict-floating-point \ +// RUN: -triple x86_64-linux-gnu -ffp-eval-method=source \ +// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FFP-OPT,CHECK-PRGM + +// expected-no-diagnostics + +float f1(float a, float b, float c) { + a = b + c; + return a * b + c; +} + +float f2(float a, float b, float c) { + // CHECK-FFP-OPT: option 'ffp-eval-method' cannot be used with '#pragma clang fp reassociate' +#pragma clang fp reassociate(on) + return (a + b) + c; +} + +float f3(float a, float b, float c) { +#pragma clang fp reassociate(off) + return (a - b) - c; +} + +float f4(float a, float b, float c) { +#pragma clang fp eval_method(double) + // CHECK-FUNC: '#pragma clang fp eval_method' cannot be used with option 'fapprox-func' + // CHECK-ASSOC: '#pragma clang fp eval_method' cannot be used with option 'mreassociate' + // CHECK-RECPR: '#pragma clang fp eval_method' cannot be used with option 'freciprocal' + // CHECK-PRGM: '#pragma clang fp eval_method' cannot be used with '#pragma clang fp reassociate' +#pragma clang fp reassociate(on) + return (a * c) - (b * c); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits