michele.scandale created this revision. michele.scandale added reviewers: zahiraam, aaron.ballman, andrew.w.kaylor, rjmccall, efriedma, fhahn. Herald added a subscriber: ormris. Herald added a project: All. michele.scandale requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
The conditions for which Clang emits the `unsafe-fp-math` function attribute has been modified as part of `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7`. In the backend code generators `"unsafe-fp-math"="true"` enable floating point contraction for the whole function. The intent of the change in `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7` was to prevent backend code generators performing contractions when that is not expected. However the change is inaccurate and incomplete because it allows `unsafe-fp-math` to be set also when only in-statement contraction is allowed. Consider the following example float foo(float a, float b, float c) { float tmp = a * b; return tmp + c; } and compile it with the command line clang -fno-math-errno -funsafe-math-optimizations -ffp-contract=on \ -O2 -mavx512f -S -o - The resulting assembly has a `vfmadd213ss` instruction which corresponds to a fused multiply-add. From the user perspective there shouldn't be any contraction because the multiplication and the addition are not in the same statement. The optimized IR is: define float @test(float noundef %a, float noundef %b, float noundef %c) #0 { %mul = fmul reassoc nsz arcp afn float %b, %a %add = fadd reassoc nsz arcp afn float %mul, %c ret float %add } attributes #0 = { [...] "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" [...] "unsafe-fp-math"="true" } The `"unsafe-fp-math"="true"` function attribute allows the backend code generator to perform `(fadd (fmul a, b), c) -> (fmadd a, b, c)`. In the current IR representation there is no way to determine the statement boundaries from the original source code. Because of this for in-statement only contraction the generated IR doesn't have instructions with the `contract` fast-math flag and `llvm.fmuladd` is being used to represent contractions opportunities that occur within a single statement. Therefore `"unsafe-fp-math"="true"` can only be emitted when contraction across statements is allowed. Moreover the change in `84a9ec2ff1ee97fd7e8ed988f5e7b197aab84a7` doesn't take into account that the floating point math function attributes can be refined during IR code generation of a function to handle the cases where the floating point math options are modified within a compound statement via pragmas (see `CGFPOptionsRAII`). For consistency `unsafe-fp-math` needs to be disabled if the contraction mode for any scope/operation is not `fast`. Similarly for consistency reason the initialization of `UnsafeFPMath` of in `TargetOptions` for the backend code generation should take into account the contraction mode as well. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D136786 Files: clang/lib/CodeGen/BackendUtil.cpp clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CodeGenFunction.cpp clang/test/CodeGen/fp-function-attrs.cpp clang/test/CodeGen/func-attr.c clang/test/CodeGenOpenCL/relaxed-fpmath.cl
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl =================================================================== --- clang/test/CodeGenOpenCL/relaxed-fpmath.cl +++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl @@ -51,7 +51,7 @@ // UNSAFE-NOT: "no-infs-fp-math" // UNSAFE-NOT: "no-nans-fp-math" // UNSAFE: "no-signed-zeros-fp-math"="true" -// UNSAFE: "unsafe-fp-math"="true" +// UNSAFE-NOT: "unsafe-fp-math"="true" // MAD: "less-precise-fpmad"="true" // MAD-NOT: "no-infs-fp-math" Index: clang/test/CodeGen/func-attr.c =================================================================== --- clang/test/CodeGen/func-attr.c +++ clang/test/CodeGen/func-attr.c @@ -1,12 +1,21 @@ // RUN: %clang -c -target x86_64 -ffast-math \ -// RUN: -emit-llvm -S -o - %s | FileCheck %s +// RUN: -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE // RUN: %clang -c -target x86_64 -funsafe-math-optimizations \ -// RUN: -emit-llvm -S -o - %s | FileCheck %s +// RUN: -fno-math-errno -ffp-contract=fast -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-UNSAFE + +// RUN: %clang -c -target x86_64 -funsafe-math-optimizations \ +// RUN: -fno-math-errno -ffp-contract=on -emit-llvm -S -o - %s | FileCheck %s --check-prefixes=CHECK,CHECK-NOUNSAFE float foo(float a, float b) { return a+b; } -// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[FAST_ATTRS:#[0-9]+]] -// CHECK: attributes [[FAST_ATTRS]] = { {{.*}} "approx-func-fp-math"="true" {{.*}} "no-signed-zeros-fp-math"="true" "no-trapping-math"="true" {{.*}} "unsafe-fp-math"="true" +// CHECK: define{{.*}} float @foo(float noundef %{{.*}}, float noundef %{{.*}}){{.*}} [[ATTRS:#[0-9]+]] +// CHECK: attributes [[ATTRS]] = { +// CHECK-SAME: "approx-func-fp-math"="true" +// CHECK-SAME: "no-signed-zeros-fp-math"="true" +// CHECK-SAME: "no-trapping-math"="true" +// CHECK-UNSAFE-SAME: "unsafe-fp-math"="true" +// CHECK-NOUNSAFE-NOT: "unsafe-fp-math"="true" +// CHECK-SAME: } Index: clang/test/CodeGen/fp-function-attrs.cpp =================================================================== --- clang/test/CodeGen/fp-function-attrs.cpp +++ clang/test/CodeGen/fp-function-attrs.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffp-contract=fast-honor-pragmas -emit-llvm -o - %s | FileCheck %s float test_default(float a, float b, float c) { float tmp = a; @@ -35,10 +36,23 @@ return tmp; } -// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NOREASSOC_ATTRS:#[0-9]+]] +// CHECK: define{{.*}} float @_Z27test_reassociate_off_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]] // CHECK: fadd nnan ninf nsz arcp contract afn float {{%.+}}, {{%.+}} // CHECK: fadd fast float {{%.+}}, {{%.+}} +float test_contract_on_pragma(float a, float b, float c) { + float tmp = a * b; + { + #pragma clang fp contract(on) + tmp += c; + } + return tmp; +} + +// CHECK: define{{.*}} float @_Z23test_contract_on_pragmafff(float noundef %a, float noundef %b, float noundef %c) [[NO_UNSAFE_ATTRS:#[0-9]+]] +// CHECK: fmul fast float {{%.+}}, {{%.+}} +// CHECK: fadd reassoc nnan ninf nsz arcp afn float {{%.+}}, {{%.+}} + // CHECK: attributes [[FAST_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="true"{{.*}} } // CHECK: attributes [[PRECISE_ATTRS]] = { {{.*}}"no-infs-fp-math"="false" {{.*}}"no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" {{.*}}"unsafe-fp-math"="false"{{.*}} } -// CHECK: attributes [[NOREASSOC_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} } +// CHECK: attributes [[NO_UNSAFE_ATTRS]] = { {{.*}}"no-infs-fp-math"="true" {{.*}}"no-nans-fp-math"="true" "no-signed-zeros-fp-math"="true" {{.*}}"unsafe-fp-math"="false"{{.*}} } Index: clang/lib/CodeGen/CodeGenFunction.cpp =================================================================== --- clang/lib/CodeGen/CodeGenFunction.cpp +++ clang/lib/CodeGen/CodeGenFunction.cpp @@ -173,10 +173,11 @@ mergeFnAttrValue("no-infs-fp-math", FPFeatures.getNoHonorInfs()); mergeFnAttrValue("no-nans-fp-math", FPFeatures.getNoHonorNaNs()); mergeFnAttrValue("no-signed-zeros-fp-math", FPFeatures.getNoSignedZero()); - mergeFnAttrValue("unsafe-fp-math", FPFeatures.getAllowFPReassociate() && - FPFeatures.getAllowReciprocal() && - FPFeatures.getAllowApproxFunc() && - FPFeatures.getNoSignedZero()); + mergeFnAttrValue( + "unsafe-fp-math", + FPFeatures.getAllowFPReassociate() && FPFeatures.getAllowReciprocal() && + FPFeatures.getAllowApproxFunc() && FPFeatures.getNoSignedZero() && + FPFeatures.allowFPContractAcrossStatement()); } CodeGenFunction::CGFPOptionsRAII::~CGFPOptionsRAII() { Index: clang/lib/CodeGen/CGCall.cpp =================================================================== --- clang/lib/CodeGen/CGCall.cpp +++ clang/lib/CodeGen/CGCall.cpp @@ -1862,11 +1862,11 @@ FuncAttrs.addAttribute("no-nans-fp-math", "true"); if (LangOpts.ApproxFunc) FuncAttrs.addAttribute("approx-func-fp-math", "true"); - if ((LangOpts.FastMath || - (!LangOpts.FastMath && LangOpts.AllowFPReassoc && - LangOpts.AllowRecip && !LangOpts.FiniteMathOnly && - LangOpts.NoSignedZero && LangOpts.ApproxFunc)) && - LangOpts.getDefaultFPContractMode() != LangOptions::FPModeKind::FPM_Off) + if (LangOpts.UnsafeFPMath && + (LangOpts.getDefaultFPContractMode() == + LangOptions::FPModeKind::FPM_Fast || + LangOpts.getDefaultFPContractMode() == + LangOptions::FPModeKind::FPM_FastHonorPragmas)) FuncAttrs.addAttribute("unsafe-fp-math", "true"); if (CodeGenOpts.SoftFloat) FuncAttrs.addAttribute("use-soft-float", "true"); Index: clang/lib/CodeGen/BackendUtil.cpp =================================================================== --- clang/lib/CodeGen/BackendUtil.cpp +++ clang/lib/CodeGen/BackendUtil.cpp @@ -410,7 +410,11 @@ Options.NoInfsFPMath = LangOpts.NoHonorInfs; Options.NoNaNsFPMath = LangOpts.NoHonorNaNs; Options.NoZerosInBSS = CodeGenOpts.NoZeroInitializedInBSS; - Options.UnsafeFPMath = LangOpts.UnsafeFPMath; + Options.UnsafeFPMath = LangOpts.UnsafeFPMath && + (LangOpts.getDefaultFPContractMode() == + LangOptions::FPModeKind::FPM_Fast || + LangOpts.getDefaultFPContractMode() == + LangOptions::FPModeKind::FPM_FastHonorPragmas); Options.ApproxFuncFPMath = LangOpts.ApproxFunc; Options.BBSections =
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits