[PATCH] D78827: Add support for #pragma clang fp reassoc(on|off) -- floating point control of associative math transformations

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3159
+controlled with this pragma.
+``#pragma clang fp allow_reassociation`` allows control over the reassociation
+of floating point expressions. When enabled, this pragma allows the expression

sepavloff wrote:
> I would say the previous name, `reassoc`, was more consistent. We do not use 
> `allow_contraction`.
"contract" isn't a shortening, though, it's a verb.  The idea is that a pragma 
is a directive to the compiler.  So arguably the more consistent spelling would 
be something like `#pragma clang fp reassociate`.



Comment at: clang/docs/LanguageExtensions.rst:3177
+Both floating point reassociation and floating point contraction can be
+controlled with this pragma.
+``#pragma clang fp reassoc`` allows control over the reassociation

Let's go ahead and word this as if arbitrary things will be controllable in the 
future.  So:

> Currently, the following things can be controlled by this pragma:



Comment at: clang/docs/LanguageExtensions.rst:3191
 option is identical to using ``#pragma STDC FP_CONTRACT(ON)`` and it allows
 fusion as specified the language standard.  The ``fast`` option allows fusiong
 in cases when the language standard does not make this possible (e.g. across

Please fix this typo while you're here.



Comment at: clang/include/clang/Basic/LangOptions.h:186
+FPM_Fast
   };
 

I'm not sure I think this fusion was an improvement; the net effect was to 
remove a few lines from this header and make a bunch of switches unnecessarily 
non-exhaustive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827



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

[PATCH] D78827: Add support for #pragma clang fp reassoc(on|off) -- floating point control of associative math transformations

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 261036.
mibintc added a comment.

Thanks for your review @sepavloff and @erichkeane, I have responded to your 
feedback and reverted the name back to reassoc which is what Serge prefers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/fp-reassoc-pragma.cpp
  clang/test/Parser/pragma-fp-contract.c

Index: clang/test/Parser/pragma-fp-contract.c
===
--- clang/test/Parser/pragma-fp-contract.c
+++ clang/test/Parser/pragma-fp-contract.c
@@ -23,3 +23,18 @@
 // expected-error@+1 {{this pragma cannot appear in union declaration}}
 #pragma STDC FP_CONTRACT ON
 };
+
+float fp_reassoc_fail(float a, float b) {
+  // CHECK-LABEL: fp_reassoc_fail
+  // expected-error@+2{{'#pragma clang fp' can only appear at file scope or at the start of a compound statement}}
+  float c = a + b;
+#pragma clang fp reassoc(on)
+  return c - b;
+}
+
+float fp_reassoc_no_fast(float a, float b) {
+// CHECK-LABEL: fp_reassoc_no_fast
+// expected-error@+1{{unexpected argument 'fast' to '#pragma clang fp reassoc'; expected 'on' or 'off'}}
+#pragma clang fp reassoc(fast)
+  return a - b;
+}
Index: clang/test/CodeGen/fp-reassoc-pragma.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+// Simple case
+float fp_reassoc_simple(float a, float b, float c) {
+// CHECK: _Z17fp_reassoc_simplefff
+// CHECK: %[[M:.+]] = fmul reassoc float %a, %b
+// CHECK-NEXT: fadd reassoc float %[[M]], %c
+#pragma clang fp reassoc(on)
+  return a * b + c;
+}
+
+// Reassoc pragma should only apply to its scope
+float fp_reassoc_scoped(float a, float b, float c) {
+  // CHECK: _Z17fp_reassoc_scopedfff
+  // CHECK: %[[M:.+]] = fmul float %a, %b
+  // CHECK-NEXT: fadd float %[[M]], %c
+  {
+#pragma clang fp reassoc(on)
+  }
+  return a * b + c;
+}
+
+// Reassoc pragma should apply to templates as well
+class Foo {};
+Foo operator+(Foo, Foo);
+template 
+T template_reassoc(T a, T b, T c) {
+#pragma clang fp reassoc(on)
+  return ((a + b) - c) + c;
+}
+
+float fp_reassoc_template(float a, float b, float c) {
+  // CHECK: _Z19fp_reassoc_templatefff
+  // CHECK: %[[A1:.+]] = fadd reassoc float %a, %b
+  // CHECK-NEXT: %[[A2:.+]] = fsub reassoc float %[[A1]], %c
+  // CHECK-NEXT: fadd reassoc float %[[A2]], %c
+  return template_reassoc(a, b, c);
+}
+
+// File Scoping should work across functions
+#pragma clang fp reassoc(on)
+float fp_file_scope_on(float a, float b, float c) {
+  // CHECK: _Z16fp_file_scope_onfff
+  // CHECK: %[[M1:.+]] = fmul reassoc float %a, %c
+  // CHECK-NEXT: %[[M2:.+]] = fmul reassoc float %b, %c
+  // CHECK-NEXT: fadd reassoc float %[[M1]], %[[M2]]
+  return (a * c) + (b * c);
+}
+
+// Inner pragma has precedence
+float fp_file_scope_stop(float a, float b, float c) {
+  // CHECK: _Z18fp_file_scope_stopfff
+  // CHECK: %[[A:.+]] = fadd reassoc float %a, %a
+  // CHECK: %[[M1:.+]] = fmul float %[[A]], %c
+  // CHECK-NEXT: %[[M2:.+]] = fmul float %b, %c
+  // CHECK-NEXT: fsub float %[[M1]], %[[M2]]
+  a = a + a;
+  {
+#pragma clang fp reassoc(off)
+return (a * c) - (b * c);
+  }
+}
+
+#pragma clang fp reassoc(off)
+float fp_reassoc_off(float a, float b, float c) {
+  // CHECK: _Z14fp_reassoc_of
+  // CHECK: %[[D1:.+]] = fdiv float %a, %c
+  // CHECK-NEXT: %[[D2:.+]] = fdiv float %b, %c
+  // CHECK-NEXT: fadd float %[[D1]], %[[D2]]
+  return (a / c) + (b / c);
+}
+
+// Takes latest flag
+float fp_reassoc_many(float a, float b, float c) {
+// CHECK: _Z15fp_reassoc_manyfff
+// CHECK: %[[D1:.+]] = fdiv reassoc float %a, %c
+// CHECK-NEXT: %[[D2:.+]] = fdiv reassoc float %b, %c
+// CHECK-NEXT: fadd reassoc float %[[D1]], %[[D2]]
+#pragma clang fp reassoc(off) reassoc(on)
+  return (a / c) + (b / c);
+}
+
+// Pragma does not propagate through called functions
+float helper_func(float a, float b, float c) { return a + b + c; }
+float fp_reassoc_call_helper(float a, float b, float c) {
+// CHECK: _Z22fp_reassoc_call_helperfff
+// CHECK: %[[S1:.+]] = fadd float %a, %b
+// CHECK-NEXT: fadd float %[[S1]], %c
+#pragma clang fp reassoc(on)
+  return helper_func(a, b, c);
+}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -985,20 +985,33 @@
   }
 }
 
-void 

[PATCH] D78827: Add support for #pragma clang fp reassoc(on|off) -- floating point control of associative math transformations

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked an inline comment as done.
mibintc added a comment.

added an inline comment




Comment at: clang/include/clang/AST/Stmt.h:618
 // Only meaningful for floating point types.
-unsigned FPFeatures : 8;
+unsigned FPFeatures : 14;
   };

This correction belongs in the parent revision, i will move it there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78827



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

[PATCH] D78827: Add support for #pragma clang fp reassoc(on|off) -- floating point control of associative math transformations

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: rjmccall, sepavloff, andrew.w.kaylor.
Herald added a project: clang.
mibintc marked an inline comment as done.
mibintc added a comment.

added an inline comment




Comment at: clang/include/clang/AST/Stmt.h:618
 // Only meaningful for floating point types.
-unsigned FPFeatures : 8;
+unsigned FPFeatures : 14;
   };

This correction belongs in the parent revision, i will move it there.


The folks at Intel who program FPGA would like to be able to control the 
FastMathFlag that governs reassociating operations at the source level using 
pragma's e.g.
#pragma clang fp reassoc(on) // allow reassociation of operations

This patch builds on reviews.llvm.org/D72841 

I just realized that I need to update the user manual to describe the syntax, 
so I owe you an update.  Hoping you will take a look


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78827

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/fp-reassoc-pragma-fails.cpp
  clang/test/CodeGen/fp-reassoc-pragma.cpp

Index: clang/test/CodeGen/fp-reassoc-pragma.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
+// Simple case
+float fp_reassoc_simple(float a, float b, float c) {
+// CHECK: _Z17fp_reassoc_simplefff
+// CHECK: %[[M:.+]] = fmul reassoc float %a, %b
+// CHECK-NEXT: fadd reassoc float %[[M]], %c
+#pragma clang fp reassoc(on)
+  return a * b + c;
+}
+
+// Reassoc pragma should only apply to its scope
+float fp_reassoc_scoped(float a, float b, float c) {
+  // CHECK: _Z17fp_reassoc_scopedfff
+  // CHECK: %[[M:.+]] = fmul float %a, %b
+  // CHECK-NEXT: fadd float %[[M]], %c
+  {
+#pragma clang fp reassoc(on)
+  }
+  return a * b + c;
+}
+
+// Reassoc pragma should apply to templates as well
+class Foo {};
+Foo operator+(Foo, Foo);
+template 
+T template_reassoc(T a, T b, T c) {
+#pragma clang fp reassoc(on)
+  return ((a + b) - c) + c;
+}
+
+float fp_reassoc_template(float a, float b, float c) {
+  // CHECK: _Z19fp_reassoc_templatefff
+  // CHECK: %[[A1:.+]] = fadd reassoc float %a, %b
+  // CHECK-NEXT: %[[A2:.+]] = fsub reassoc float %[[A1]], %c
+  // CHECK-NEXT: fadd reassoc float %[[A2]], %c
+  return template_reassoc(a, b, c);
+}
+
+// File Scoping should work across functions
+#pragma clang fp reassoc(on)
+float fp_file_scope_on(float a, float b, float c) {
+  // CHECK: _Z16fp_file_scope_onfff
+  // CHECK: %[[M1:.+]] = fmul reassoc float %a, %c
+  // CHECK-NEXT: %[[M2:.+]] = fmul reassoc float %b, %c
+  // CHECK-NEXT: fadd reassoc float %[[M1]], %[[M2]]
+  return (a * c) + (b * c);
+}
+
+// Inner pragma has precedence
+float fp_file_scope_stop(float a, float b, float c) {
+  // CHECK: _Z18fp_file_scope_stopfff
+  // CHECK: %[[A:.+]] = fadd reassoc float %a, %a
+  // CHECK: %[[M1:.+]] = fmul float %[[A]], %c
+  // CHECK-NEXT: %[[M2:.+]] = fmul float %b, %c
+  // CHECK-NEXT: fsub float %[[M1]], %[[M2]]
+  a = a + a;
+  {
+#pragma clang fp reassoc(off)
+return (a * c) - (b * c);
+  }
+}
+
+#pragma clang fp reassoc(off)
+float fp_reassoc_off(float a, float b, float c) {
+  // CHECK: _Z14fp_reassoc_of
+  // CHECK: %[[D1:.+]] = fdiv float %a, %c
+  // CHECK-NEXT: %[[D2:.+]] = fdiv float %b, %c
+  // CHECK-NEXT: fadd float %[[D1]], %[[D2]]
+  return (a / c) + (b / c);
+}
+
+// Takes latest flag
+float fp_reassoc_many(float a, float b, float c) {
+// CHECK: _Z15fp_reassoc_manyfff
+// CHECK: %[[D1:.+]] = fdiv reassoc float %a, %c
+// CHECK-NEXT: %[[D2:.+]] = fdiv reassoc float %b, %c
+// CHECK-NEXT: fadd reassoc float %[[D1]], %[[D2]]
+#pragma clang fp reassoc(off) reassoc(on)
+  return (a / c) + (b / c);
+}
+
+// Pragma does not propagate through called functions
+float helper_func(float a, float b, float c) { return a + b + c; }
+float fp_reassoc_call_helper(float a, float b, float c) {
+// CHECK: _Z22fp_reassoc_call_helperfff
+// CHECK: %[[S1:.+]] = fadd float %a, %b
+// CHECK-NEXT: fadd float %[[S1]], %c
+#pragma clang fp reassoc(on)
+  return helper_func(a, b, c);
+}
Index: clang/test/CodeGen/fp-reassoc-pragma-fails.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma-fails.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+float fp_reassoc_fail(float a, float b) {
+  // CHECK-LABEL: fp_reassoc_fail
+  // expected-error@+2{{'#pragma clang fp' can only appear at file scope or at the start of a compound