[PATCH] D80440: [OpenCL] Prevent fused mul and add by default

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: rjmccall, arsenm.
Herald added subscribers: ebevhan, yaxunl, wdng.

Currently, clang will generate fused operation `fmuladd` for expressions like 
`a*b+c`. However, I can't find anything in the spec that explains this 
behavior. But clang was doing this for almost 10 years

I looked at:
**s6.13.2 **where it details fp conract pragma:

  // on-off-switch is one of ON, OFF, or DEFAULT.
  // The DEFAULT value is ON.
  #pragma OPENCL FP_CONTRACT on-off-switch

The default behavior here refers to the default value of the pragma when used 
in the source code.

**s7.4** table 38

  x * y + z
  Implemented either as a correctly rounded fma or as a multiply and an add 
both of which are correctly rounded.

This table described behavior when `-cl-unsafe-math-optimizations` flag is 
passed.

I can't find anything else that would justify allowing fused operations, 
therefore I assume this is not safe to do and can cause issues on some 
applications/targets


https://reviews.llvm.org/D80440

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl
  clang/test/CodeGenOpenCL/single-precision-constant.cl


Index: clang/test/CodeGenOpenCL/single-precision-constant.cl
===
--- clang/test/CodeGenOpenCL/single-precision-constant.cl
+++ clang/test/CodeGenOpenCL/single-precision-constant.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -cl-single-precision-constant -emit-llvm -o - | 
FileCheck %s
 
 float fn(float f) {
-  // CHECK: tail call float @llvm.fmuladd.f32(float %f, float 2.00e+00, 
float 1.00e+00)
+  // CHECK: fmul float %f, 2.00e+00
+  // CHECK: fadd float %mul, 1.00e+00
   return f*2. + 1.;
 }
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -17,7 +17,8 @@
 }
 
 float fused_mad(float a, float b, float c) {
-  // NORMAL: tail call float @llvm.fmuladd.f32(float %a, float %b, float %c)
+  // NORMAL: fmul float
+  // NORMAL: fadd float
   // FAST: fmul fast float
   // FAST: fadd fast float
   // MAD: fmul contract float
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2322,7 +2322,6 @@
 Opts.AltiVec = 0;
 Opts.ZVector = 0;
 Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::None);
-Opts.setDefaultFPContractMode(LangOptions::FPM_On);
 Opts.NativeHalfType = 1;
 Opts.NativeHalfArgsAndReturns = 1;
 Opts.OpenCLCPlusPlus = Opts.CPlusPlus;


Index: clang/test/CodeGenOpenCL/single-precision-constant.cl
===
--- clang/test/CodeGenOpenCL/single-precision-constant.cl
+++ clang/test/CodeGenOpenCL/single-precision-constant.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -cl-single-precision-constant -emit-llvm -o - | FileCheck %s
 
 float fn(float f) {
-  // CHECK: tail call float @llvm.fmuladd.f32(float %f, float 2.00e+00, float 1.00e+00)
+  // CHECK: fmul float %f, 2.00e+00
+  // CHECK: fadd float %mul, 1.00e+00
   return f*2. + 1.;
 }
Index: clang/test/CodeGenOpenCL/relaxed-fpmath.cl
===
--- clang/test/CodeGenOpenCL/relaxed-fpmath.cl
+++ clang/test/CodeGenOpenCL/relaxed-fpmath.cl
@@ -17,7 +17,8 @@
 }
 
 float fused_mad(float a, float b, float c) {
-  // NORMAL: tail call float @llvm.fmuladd.f32(float %a, float %b, float %c)
+  // NORMAL: fmul float
+  // NORMAL: fadd float
   // FAST: fmul fast float
   // FAST: fadd fast float
   // MAD: fmul contract float
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2322,7 +2322,6 @@
 Opts.AltiVec = 0;
 Opts.ZVector = 0;
 Opts.setLaxVectorConversions(LangOptions::LaxVectorConversionKind::None);
-Opts.setDefaultFPContractMode(LangOptions::FPM_On);
 Opts.NativeHalfType = 1;
 Opts.NativeHalfArgsAndReturns = 1;
 Opts.OpenCLCPlusPlus = Opts.CPlusPlus;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80440: [OpenCL] Prevent fused mul and add by default

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think "the default value is on" is meant to imply that contraction is allowed 
by default, not that you can write the pragma without an `on-off-switch` and it 
means "on".

FWIW, that would be consistent with C, which also enables contraction by 
default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80440/new/

https://reviews.llvm.org/D80440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80440: [OpenCL] Prevent fused mul and add by default

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

Contraction is just an optimization technique. Unless a user explicitly 
requests strict FP semantics, contraction does not break C++ semantics.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80440/new/

https://reviews.llvm.org/D80440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80440: [OpenCL] Prevent fused mul and add by default

[Matt Arsenault via Phabricator via cfe-commits]

arsenm requested changes to this revision.
arsenm added a comment.
This revision now requires changes to proceed.

I think the current handling is correct. As you said, the specified default is 
FP_CONTRACT ON. The description of the mad function in the table is unrelated, 
since that's the definition and won't change based on the pragma.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80440/new/

https://reviews.llvm.org/D80440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits