[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations
scanon added a comment. TS18661-5 is quite vague on what the intended semantics for the pragma are. These pragmas are intended to be bindings of clause 10.4 of IEEE 754, which is also pretty wishy-washy on the whole, but it's worth noting that clause 10 is titled *expression evaluation* specifically. The relevant text here is: > A language standard should also define, and require implementations to > provide, attributes that allow and disallow value-changing optimizations, > separately or collectively, for a block. These optimizations might include, > but are not limited to: > ― Applying the associative or distributive laws. > ― Synthesis of a fusedMultiplyAdd operation from a multiplication and an > addition. > ― Synthesis of a formatOf operation from an operation and a conversion of > the result of the 40 operation. > ― Use of wider intermediate results in expression evaluation. So IEEE-754 appears to view this as being "just like" contraction. (Note that this is all under a "should", so #yolo). 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 reassociate(on|off) -- floating point control of associative math transformations
mibintc added a comment. The ISO C proposal is here http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2407.pdf but the details are in the IEEE standards documents. 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 reassociate(on|off) -- floating point control of associative math transformations
rjmccall added a comment. That's actually really interesting. Is there a paper describing the desired model in more detail? I think the natural interpretation of this pragma is to say that the specific operations written within the pragma are considered to be associative and are therefore allowed to be re-associated with other associative operators. However, that might not be the committee's formal interpretation, given how contraction works. 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 reassociate(on|off) -- floating point control of associative math transformations
mibintc added a comment. BTW there is a proposal http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2421.pdf at the ISO C meeting to support some new floating point pragmas including #pragma STDC FENV_ALLOW_ASSOCIATIVE_LAW on-off-switch The committee wants to see an implementation(s) to ensure that it's viable. 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 reassociate(on|off) -- floating point control of associative math transformations
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc355bec749e9: Add support for #pragma clang fp
reassociate(on|off) (authored by mibintc).
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
clang/test/Parser/pragma-fp.cpp
Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
// RUN: %clang_cc1 -std=c++11 -verify %s
void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected contract}} */
+/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
#pragma clang fp
for (int i = 0; i < Length; i++) {
List[i] = i;
}
}
void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
#pragma clang fp blah
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -24,7 +24,7 @@
}
void test_4(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(while)
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -32,7 +32,7 @@
}
void test_5(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(maybe)
for (int i = 0; i < Length; i++) {
List[i] = i;
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 reassociate(off)
+ 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 reassociate'; expected 'on' or 'off'}}
+#pragma clang fp reassociate(fast)
+ return a - b;
+}
Index: clang/test/CodeGen/fp-reassoc-pragma.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma.cpp
@@ -0,0 +1,92 @@
+// 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: %[[A:.+]] = fadd reassoc float %b, %c
+// CHECK: %[[M:.+]] = fmul reassoc float %[[A]], %b
+// CHECK-NEXT: fadd reassoc float %[[M]], %c
+#pragma clang fp reassociate(on)
+ a = b + c;
+ 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 reassociate(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 reassociate(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 reassociate(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);
+}
+
+//
[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations
rjmccall accepted this revision. rjmccall added a comment. LGTM, too. Thanks! 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 reassociate(on|off) -- floating point control of associative math transformations
scanon accepted this revision. scanon added a comment. This revision is now accepted and ready to land. My concerns have been addressed. Thanks for bearing with me, Melanie! 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 reassociate(on|off) -- floating point control of associative math transformations
scanon added a comment. (Please get one additional sign off before committing; I'm mainly signing off on the numerics model aspect). 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 reassociate(on|off) -- floating point control of associative math transformations
mibintc updated this revision to Diff 262217.
mibintc retitled this revision from "Add support for #pragma clang fp
reassociate(fast|off) -- floating point control of associative math
transformations" to "Add support for #pragma clang fp reassociate(on|off) --
floating point control of associative math transformations".
mibintc edited the summary of this revision.
mibintc added a comment.
Here it is again with the on/off spelling
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
clang/test/Parser/pragma-fp.cpp
Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
// RUN: %clang_cc1 -std=c++11 -verify %s
void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected contract}} */
+/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
#pragma clang fp
for (int i = 0; i < Length; i++) {
List[i] = i;
}
}
void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
#pragma clang fp blah
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -24,7 +24,7 @@
}
void test_4(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(while)
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -32,7 +32,7 @@
}
void test_5(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(maybe)
for (int i = 0; i < Length; i++) {
List[i] = i;
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 reassociate(off)
+ 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 reassociate'; expected 'on' or 'off'}}
+#pragma clang fp reassociate(fast)
+ return a - b;
+}
Index: clang/test/CodeGen/fp-reassoc-pragma.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma.cpp
@@ -0,0 +1,92 @@
+// 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: %[[A:.+]] = fadd reassoc float %b, %c
+// CHECK: %[[M:.+]] = fmul reassoc float %[[A]], %b
+// CHECK-NEXT: fadd reassoc float %[[M]], %c
+#pragma clang fp reassociate(on)
+ a = b + c;
+ 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 reassociate(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 reassociate(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 reassociate(on)
+float fp_file_scope_on(float a,
[PATCH] D78827: Add support for #pragma clang fp reassociate(fast|off) -- floating point control of associative math transformations
rjmccall added a comment. That makes sense, thanks. I think I'm comfortable using on/off for this, but it's definitely good to stop and consider, and you're absolutely right that it needs to be documented. 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 reassociate(fast|off) -- floating point control of associative math transformations
scanon added a comment. > I don't think the C standard is likely to ever bless reassociative FP math > with an expression-local restriction. Steve, do you actually think that would > be a useful optimization mode? I think it's pretty unlikely that C would do this, as well. It is plausibly a useful semantic mode, but I don't know that we need to reserve the name for it. I only want to raise the question, and be sure that we're aware that we're making a decision here (also, either way we need to document it clearly). 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 reassociate(fast|off) -- floating point control of associative math transformations
rjmccall added a comment. The reason we call FP contraction "fast" instead of "on" when it's cross-statement is because "on" is supposed to be consistent with the C language rules, whereas "fast" is stronger. There's no analogous situation with reassociation; I don't think the C standard is likely to ever bless reassociative FP math with an expression-local restriction. Steve, do you actually think that would be a useful optimization mode? 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 reassociate(fast|off) -- floating point control of associative math transformations
mibintc added a comment. I checked with FPGA folks and confirm what @scanon says is correct, the reassoc fast math flag enables reassociation across multiple statements, so i changed the syntax to use 'fast' and 'off', and changed the documentation 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 reassociate(fast|off) -- floating point control of associative math transformations
mibintc updated this revision to Diff 262108.
mibintc retitled this revision from "Add support for #pragma clang fp
reassociate(on|off) -- floating point control of associative math
transformations" to "Add support for #pragma clang fp reassociate(fast|off) --
floating point control of associative math transformations".
mibintc edited the summary of this revision.
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
clang/test/Parser/pragma-fp.cpp
Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
// RUN: %clang_cc1 -std=c++11 -verify %s
void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected contract}} */
+/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
#pragma clang fp
for (int i = 0; i < Length; i++) {
List[i] = i;
}
}
void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
#pragma clang fp blah
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -24,7 +24,7 @@
}
void test_4(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(while)
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -32,7 +32,7 @@
}
void test_5(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(maybe)
for (int i = 0; i < Length; i++) {
List[i] = i;
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 reassociate(fast)
+ return c - b;
+}
+
+float fp_reassoc_no_fast(float a, float b) {
+// CHECK-LABEL: fp_reassoc_no_on
+// expected-error@+1{{unexpected argument 'on' to '#pragma clang fp reassociate'; expected 'fast' or 'off'}}
+#pragma clang fp reassociate(on)
+ return a - b;
+}
Index: clang/test/CodeGen/fp-reassoc-pragma.cpp
===
--- /dev/null
+++ clang/test/CodeGen/fp-reassoc-pragma.cpp
@@ -0,0 +1,92 @@
+// 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: %[[A:.+]] = fadd reassoc float %b, %c
+// CHECK: %[[M:.+]] = fmul reassoc float %[[A]], %b
+// CHECK-NEXT: fadd reassoc float %[[M]], %c
+#pragma clang fp reassociate(fast)
+ a = b + c;
+ 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 reassociate(fast)
+ }
+ 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 reassociate(fast)
+ 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 reassociate(fast)
+float fp_file_scope_on(float a, float b, float c) {
+ // CHECK: _Z16fp_file_scope_onfff
+ //
[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations
mibintc updated this revision to Diff 261921.
mibintc added a comment.
I fixed the issues that @rjmccall mentioned. I don't yet have an answer for
@scanon, need to get back to you about that.
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
clang/test/Parser/pragma-fp.cpp
Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
// RUN: %clang_cc1 -std=c++11 -verify %s
void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected contract}} */
+/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
#pragma clang fp
for (int i = 0; i < Length; i++) {
List[i] = i;
}
}
void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
#pragma clang fp blah
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -24,7 +24,7 @@
}
void test_4(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(while)
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -32,7 +32,7 @@
}
void test_5(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(maybe)
for (int i = 0; i < Length; i++) {
List[i] = i;
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 reassociate(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 reassociate'; expected 'on' or 'off'}}
+#pragma clang fp reassociate(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 reassociate(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 reassociate(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 reassociate(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 reassociate(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
[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations
scanon added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3182 +enabled for the translation unit with the ``-fassociative-math`` flag. +The pragma can take two values: ``on`` and ``off``. + mibintc wrote: > scanon wrote: > > rjmccall wrote: > > > Do you want to add an example here? > > Is the intention that this allows reassociation only within statements > > (like fp contract on)? Based on the ir in the tests, I think the answer is > > "no". > > > > If so, should "on" be called "fast" instead, since its semantics match the > > "fast" semantics for contract, and reserve "on" for reassociation within a > > statement (that seems like it would also be useful, potentially)? > > > > Otherwise, please add some tests with multiple statements. > > > > I agree with John that `pragma clang fp reassociate` is a reasonable > > spelling. > The intention is that the pragma can be placed at either file scope or at the > start of a compound statement, if at file scope it affects the functions > following the pragma. If at compound statement it is effective for the > compound statement, i can modify the test to have multiple statements. I > disagree with the suggestion of "fast" instead of on/off. at the command > line you can use -f[no-]associative-math; of course that command line option > isn't specific to floating point, but the point is it's on/off ; i can't > speak to whether "fast" would be a useful setting. Thanks for your review I don't think you understood my comment, so I'll try to explain more clearly: I am not asking if it applies to multiple statements or a single statement; I am asking within what scope reassociation is allowed. An example: `fp contract on` allows: ``` double r = a*b + c; ``` to be contracted to an fma, but does not allow: ``` double t = a*b; double r = t + c; ``` to be contracted. `fp contract fast` allows both to be contracted. Your reassociate pragma, if I am understanding correctly, allows: ``` double t = a + b; double r = t + c; ``` to be reassociated; this matches the behavior of `fp contract fast`. I am asking if, for the sake of understandability and orthogonality of the floating-point model, it would make more sense for this option to be named `fast`, reserving `on` for a mode that only allows reassociation within expressions. 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 reassociate(on|off) -- floating point control of associative math transformations
mibintc marked an inline comment as done. mibintc added a comment. A reply to @scanon Comment at: clang/docs/LanguageExtensions.rst:3182 +enabled for the translation unit with the ``-fassociative-math`` flag. +The pragma can take two values: ``on`` and ``off``. + scanon wrote: > rjmccall wrote: > > Do you want to add an example here? > Is the intention that this allows reassociation only within statements (like > fp contract on)? Based on the ir in the tests, I think the answer is "no". > > If so, should "on" be called "fast" instead, since its semantics match the > "fast" semantics for contract, and reserve "on" for reassociation within a > statement (that seems like it would also be useful, potentially)? > > Otherwise, please add some tests with multiple statements. > > I agree with John that `pragma clang fp reassociate` is a reasonable spelling. The intention is that the pragma can be placed at either file scope or at the start of a compound statement, if at file scope it affects the functions following the pragma. If at compound statement it is effective for the compound statement, i can modify the test to have multiple statements. I disagree with the suggestion of "fast" instead of on/off. at the command line you can use -f[no-]associative-math; of course that command line option isn't specific to floating point, but the point is it's on/off ; i can't speak to whether "fast" would be a useful setting. Thanks for your review 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 reassociate(on|off) -- floating point control of associative math transformations
rjmccall added a comment. Just minor requests now. 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 rjmccall wrote: > 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: Thanks. Please put the first sentence in its own paragraph and end it with a colon so that it logically leads into both of the following blocks. Comment at: clang/docs/LanguageExtensions.rst:3182 +enabled for the translation unit with the ``-fassociative-math`` flag. +The pragma can take two values: ``on`` and ``off``. + Do you want to add an example here? Comment at: clang/docs/LanguageExtensions.rst:3192 in cases when the language standard does not make this possible (e.g. across statements in C) Oh, and there's a missing period here. Comment at: clang/include/clang/Basic/LangOptions.h:186 +FPM_Fast }; mibintc wrote: > rjmccall wrote: > > 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. > I dropped the on/off enumeration and just using boolean, where needed, to > show the setting, do you like this better? Yeah, thanks. Comment at: clang/include/clang/Sema/Sema.h:9624 + /// \#pragma clang fp reassociate + void ActOnPragmaFPAllowReassociation(bool IsEnabled); Please name this the same as the language feature. Comment at: clang/lib/Parse/ParsePragma.cpp:2900 return; } PP.Lex(Tok); Please make this a single block by making the condition something like `if (!FlagValue || (FlagKind == TokFPAnnotValue::Reassociate && FlagValue == TokFPAnnotValue::Fast))`. 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 reassociate(on|off) -- floating point control of associative math transformations
scanon added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3182 +enabled for the translation unit with the ``-fassociative-math`` flag. +The pragma can take two values: ``on`` and ``off``. + rjmccall wrote: > Do you want to add an example here? Is the intention that this allows reassociation only within statements (like fp contract on)? Based on the ir in the tests, I think the answer is "no". If so, should "on" be called "fast" instead, since its semantics match the "fast" semantics for contract, and reserve "on" for reassociation within a statement (that seems like it would also be useful, potentially)? Otherwise, please add some tests with multiple statements. I agree with John that `pragma clang fp reassociate` is a reasonable spelling. 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 reassociate(on|off) -- floating point control of associative math transformations
mibintc updated this revision to Diff 261875.
mibintc retitled this revision from "Add support for #pragma clang fp
reassoc(on|off) -- floating point control of associative math transformations"
to "Add support for #pragma clang fp reassociate(on|off) -- floating point
control of associative math transformations".
mibintc added a comment.
Responded to @rjmccall 's review.
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
clang/test/Parser/pragma-fp.cpp
Index: clang/test/Parser/pragma-fp.cpp
===
--- clang/test/Parser/pragma-fp.cpp
+++ clang/test/Parser/pragma-fp.cpp
@@ -1,14 +1,14 @@
// RUN: %clang_cc1 -std=c++11 -verify %s
void test_0(int *List, int Length) {
-/* expected-error@+1 {{missing option; expected contract}} */
+/* expected-error@+1 {{missing option; expected 'contract' or 'reassociate'}} */
#pragma clang fp
for (int i = 0; i < Length; i++) {
List[i] = i;
}
}
void test_1(int *List, int Length) {
-/* expected-error@+1 {{invalid option 'blah'; expected contract}} */
+/* expected-error@+1 {{invalid option 'blah'; expected 'contract' or 'reassociate'}} */
#pragma clang fp blah
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -24,7 +24,7 @@
}
void test_4(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'while' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(while)
for (int i = 0; i < Length; i++) {
List[i] = i;
@@ -32,7 +32,7 @@
}
void test_5(int *List, int Length) {
-/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'on', 'fast' or 'off'}} */
+/* expected-error@+1 {{unexpected argument 'maybe' to '#pragma clang fp contract'; expected 'fast' or 'on' or 'off'}} */
#pragma clang fp contract(maybe)
for (int i = 0; i < Length; i++) {
List[i] = i;
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 reassociate(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 reassociate'; expected 'on' or 'off'}}
+#pragma clang fp reassociate(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 reassociate(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 reassociate(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 reassociate(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 reassociate(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:
[PATCH] D78827: Add support for #pragma clang fp reassociate(on|off) -- floating point control of associative math transformations
mibintc marked 4 inline comments as done. mibintc added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:186 +FPM_Fast }; rjmccall wrote: > 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. I dropped the on/off enumeration and just using boolean, where needed, to show the setting, do you like this better? 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
