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

Reply via email to