[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I commit this in r350414, thank you for the patch!


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin added a comment.

Thank you!  Aaron, Could you integrate this patch, please? I do not have commit 
access yet.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

alexey.lapshin wrote:
> aaron.ballman wrote:
> > I think this error is pretty unfortunate -- it doesn't really help the user 
> > to understand what's wrong with their code. Can it be improved?
> Agreed, this does not look very informative. It surely can be improved. 
> Though it looks like not related to my fix. My fix is to add two additional 
> loop hints. That kind of diagnostic related to all loop hints "clang loop". 
> I.e. all pragmas "clang loop" can be checked for various cases and for better 
> diagnostic. It looks like separate task. But OK, I will check that case.
That's a fair point -- I'm fine with doing that work in a separate patch. I 
don't think we need to hold this patch up further for it, anyway.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

aaron.ballman wrote:
> I think this error is pretty unfortunate -- it doesn't really help the user 
> to understand what's wrong with their code. Can it be improved?
Agreed, this does not look very informative. It surely can be improved. Though 
it looks like not related to my fix. My fix is to add two additional loop 
hints. That kind of diagnostic related to all loop hints "clang loop". I.e. all 
pragmas "clang loop" can be checked for various cases and for better 
diagnostic. It looks like separate task. But OK, I will check that case.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/pragma-pipeline.cpp:3
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected 
unqualified-id}} */
+int main() {

I think this error is pretty unfortunate -- it doesn't really help the user to 
understand what's wrong with their code. Can it be improved?


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179933.
alexey.lapshin added a comment.

Thank you. I updated doc, splitted Parser test, added Sema tests. Please check 
it once more.


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp
  test/Sema/pragma-pipeline.cpp

Index: test/Sema/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Sema/pragma-pipeline.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+
+#pragma clang loop pipeline(disable) /* expected-error {{expected unqualified-id}} */
+int main() {
+  for (int i = 0; i < 10; ++i)
+;
+}
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+/* expected-error {{invalid value '-1'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(-1)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
+
Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+}
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' 

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2582
 specified for the subsequent loop. The directive allows vectorization,
-interleaving, and unrolling to be enabled or disabled. Vector width as well
-as interleave and unrolling count can be manually specified. See
-`language extensions
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining

alexey.lapshin wrote:
> aaron.ballman wrote:
> > Can you combine the new sentence with the previous one? `The directive 
> > allows vectorization, interleaving, pipelining, and unrolling to be enabled 
> > or disabled.`
> In that case it would change a meaning and become incorrect. Pipelining could 
> only be disabled. It could not be enabled.
Ah, yes, that would change the meaning then. I'd go with: `The directive allows 
pipelining to be disabled, or vectorization, interleaving, and unrolling to be 
enabled or disabled.`



Comment at: include/clang/Basic/AttrDocs.td:2663
+  could be used as hints for the software pipelining optimization. The pragma 
is
+  placed immediately before a for, while, do-while, or a C++11 range-based for
+  loop.

I'd like to see a Sema test where the pragma is put at global scope to see if 
the diagnostic is reasonable or not. e.g.,
```
#pragma clang loop pipeline(disable)

int main() {
  for (int i = 0; i < 10; ++i)
;
}
```



Comment at: include/clang/Basic/AttrDocs.td:2680-2681
+  the specified cycle count. If a schedule was not found then loop
+  remains unchanged. The initiation interval must be a positive number
+  greater than zero:
+

Please add a Sema test with a negative number as the initiation interval.



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1188
+  "vectorize_width, interleave, interleave_count, unroll, unroll_count, "
+  "pipeline, pipeline_initiation_interval or distribute">;
 

pipeline_initiation_interval or distribute -> pipeline_initiation_interval, or 
distribute

(Keeps the Oxford comma as in the original diagnostic.)



Comment at: test/Parser/pragma-pipeline.cpp:28
+/* expected-error {{invalid argument of type 'double'; expected an integer 
type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang 
loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {

This isn't really a parser test -- it should probably be in Sema instead.



Comment at: test/Parser/pragma-pipeline.cpp:35-47
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma 
clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma 
clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error 
{{incompatible directives 'pipeline(disable)' and 
'pipeline_initiation_interval(4)'}} */

These should also move to Sema.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179843.
alexey.lapshin added a comment.

Thank you. addressed all grammar things. please check it once more.


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2582
 specified for the subsequent loop. The directive allows vectorization,
-interleaving, and unrolling to be enabled or disabled. Vector width as well
-as interleave and unrolling count can be manually specified. See
-`language extensions
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining

aaron.ballman wrote:
> Can you combine the new sentence with the previous one? `The directive allows 
> vectorization, interleaving, pipelining, and unrolling to be enabled or 
> disabled.`
In that case it would change a meaning and become incorrect. Pipelining could 
only be disabled. It could not be enabled.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The docs are getting much closer -- just a few grammatical things left to fix, 
I believe.




Comment at: include/clang/Basic/AttrDocs.td:2582
 specified for the subsequent loop. The directive allows vectorization,
-interleaving, and unrolling to be enabled or disabled. Vector width as well
-as interleave and unrolling count can be manually specified. See
-`language extensions
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining

Can you combine the new sentence with the previous one? `The directive allows 
vectorization, interleaving, pipelining, and unrolling to be enabled or 
disabled.`



Comment at: include/clang/Basic/AttrDocs.td:2657
+  is the number of cycles between two iterations of an unoptimized loop in
+  newly created schedule. New optimized loop is created such that a single 
iteration
+  of the loop executes in the same number of cycles as the initiation interval.

in newly created schedule -> in the newly created schedule

New optimized loop -> A new, optimized loop



Comment at: include/clang/Basic/AttrDocs.td:2662
+  ``#pragma clang loop pipeline and #pragma loop pipeline_initiation_interval``
+  could be used as hints for Software Pipelining optimization. The pragma is
+  placed immediately before a for, while, do-while, or c++11 range-based for

for Software Pipelining optimization. -> for the software pipelining 
optimization.



Comment at: include/clang/Basic/AttrDocs.td:2663
+  could be used as hints for Software Pipelining optimization. The pragma is
+  placed immediately before a for, while, do-while, or c++11 range-based for
+  loop.

or c++11 range-based for loop. -> or a C++11 range-based for loop.



Comment at: include/clang/Basic/AttrDocs.td:2666
+
+  Using ``#pragma clang loop pipeline(disable)`` avoids software pipelining
+  optimization. The disable state can only be specified:

avoids software -> avoids the software



Comment at: include/clang/Basic/AttrDocs.td:2678
+  the software pipeliner to try the specified initiation interval.
+  If schedule was found then the resulting loop iteration would have
+  specified cycle count. If schedule was not found then loop would stay

If schedule was found -> If a schedule was found



Comment at: include/clang/Basic/AttrDocs.td:2679
+  If schedule was found then the resulting loop iteration would have
+  specified cycle count. If schedule was not found then loop would stay
+  unchanged. The initiation interval must be a positive number

would have specified cycle count. -> would have the specified cycle count.

If schedule was not -> If a schedule was not
would stay -> remains


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179459.
alexey.lapshin added a comment.

please consider changes in AttrDocs.td


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked 2 inline comments as done.
alexey.lapshin added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2655
+  dependence violated. This constant interval(in cycles) between the start
+  of iterations called initiation interval. Cycles number of one iteration
+  of newly generated loop matches with Initiation Interval. For further

aaron.ballman wrote:
> alexey.lapshin wrote:
> > aaron.ballman wrote:
> > > is called the initiation interval.
> > > 
> > > I can't quite parse the second sentence, though. Is it trying to say that 
> > > the initiation interval is the number of cycles for a single iteration of 
> > > the optimized loop?
> > Not quite right : initiation interval is not a number of cycles for a 
> > single iteration of the optimized loop. But initiation interval matches 
> > with number of cycles for a single iteration of the optimized loop. 
> > Initiation interval is the number of cycles between next and previous 
> > iteration of original loop. New loop created so that single iteration of 
> > the optimized loop has the same number cycles as initiation interval( thus 
> > every new iteration of original loop started each time when new iteration 
> > of optimized loop started - difference between iterations is initiation 
> > interval). 
> > 
> > trying to rephrase : number of cycles for a single iteration of the 
> > optimized loop matches with number of cycles of initiation interval.
> I am still a bit fuzzy on the details (optimizations are not my area of 
> expertise). Is this an accurate what to rephrase it?
> 
> "The initiation interval is the number of cycles between two iterations of an 
> unoptimized loop. These pragmas cause a new. optimized loop to be created 
> such that a single iteration of the loop executes in the same number of 
> cycles as the initiation interval."
I will take it with small modifications.



Comment at: include/clang/Basic/AttrDocs.td:2676
+  the software pipeliner to try the specified initiation interval.
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:

aaron.ballman wrote:
> alexey.lapshin wrote:
> > aaron.ballman wrote:
> > > I'm can't quite parse this sentence either. Is it trying to say that the 
> > > scheduler will try to find a loop iteration cycle count that most-closely 
> > > matches the specified initiation interval?
> > I wanted to say that pipeliner will try to find a schedule that exactly 
> > matches the specified initiation interval. And if schedule would be found 
> > then single iteration of the optimized loop would have exactly the same 
> > amount of cycles as initiation interval. trying to rephrase :
> > 
> > If schedule would be found then single iteration of the optimized loop 
> > would have exactly the same amount of cycles as initiation interval has. 
> Ah, okay! So what happens if an exact match cannot be found? Does it behave 
> as though the pragma was not specified at all (as in, the pragma is ignored)?
yes. if schedule was not found then the loop would stay unchanged.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: tonic.
aaron.ballman added a subscriber: tonic.
aaron.ballman added a comment.

Adding @tonic as a reviewer, who may have suggestions on how to improve the 
documentation wording.




Comment at: include/clang/Basic/AttrDocs.td:2655
+  dependence violated. This constant interval(in cycles) between the start
+  of iterations called initiation interval. Cycles number of one iteration
+  of newly generated loop matches with Initiation Interval. For further

alexey.lapshin wrote:
> aaron.ballman wrote:
> > is called the initiation interval.
> > 
> > I can't quite parse the second sentence, though. Is it trying to say that 
> > the initiation interval is the number of cycles for a single iteration of 
> > the optimized loop?
> Not quite right : initiation interval is not a number of cycles for a single 
> iteration of the optimized loop. But initiation interval matches with number 
> of cycles for a single iteration of the optimized loop. Initiation interval 
> is the number of cycles between next and previous iteration of original loop. 
> New loop created so that single iteration of the optimized loop has the same 
> number cycles as initiation interval( thus every new iteration of original 
> loop started each time when new iteration of optimized loop started - 
> difference between iterations is initiation interval). 
> 
> trying to rephrase : number of cycles for a single iteration of the optimized 
> loop matches with number of cycles of initiation interval.
I am still a bit fuzzy on the details (optimizations are not my area of 
expertise). Is this an accurate what to rephrase it?

"The initiation interval is the number of cycles between two iterations of an 
unoptimized loop. These pragmas cause a new. optimized loop to be created such 
that a single iteration of the loop executes in the same number of cycles as 
the initiation interval."



Comment at: include/clang/Basic/AttrDocs.td:2676
+  the software pipeliner to try the specified initiation interval.
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:

alexey.lapshin wrote:
> aaron.ballman wrote:
> > I'm can't quite parse this sentence either. Is it trying to say that the 
> > scheduler will try to find a loop iteration cycle count that most-closely 
> > matches the specified initiation interval?
> I wanted to say that pipeliner will try to find a schedule that exactly 
> matches the specified initiation interval. And if schedule would be found 
> then single iteration of the optimized loop would have exactly the same 
> amount of cycles as initiation interval. trying to rephrase :
> 
> If schedule would be found then single iteration of the optimized loop would 
> have exactly the same amount of cycles as initiation interval has. 
Ah, okay! So what happens if an exact match cannot be found? Does it behave as 
though the pragma was not specified at all (as in, the pragma is ignored)?


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked 2 inline comments as done.
alexey.lapshin added a comment.

will correct all mistakes. please check explanations for the questions.




Comment at: include/clang/Basic/AttrDocs.td:2655
+  dependence violated. This constant interval(in cycles) between the start
+  of iterations called initiation interval. Cycles number of one iteration
+  of newly generated loop matches with Initiation Interval. For further

aaron.ballman wrote:
> is called the initiation interval.
> 
> I can't quite parse the second sentence, though. Is it trying to say that the 
> initiation interval is the number of cycles for a single iteration of the 
> optimized loop?
Not quite right : initiation interval is not a number of cycles for a single 
iteration of the optimized loop. But initiation interval matches with number of 
cycles for a single iteration of the optimized loop. Initiation interval is the 
number of cycles between next and previous iteration of original loop. New loop 
created so that single iteration of the optimized loop has the same number 
cycles as initiation interval( thus every new iteration of original loop 
started each time when new iteration of optimized loop started - difference 
between iterations is initiation interval). 

trying to rephrase : number of cycles for a single iteration of the optimized 
loop matches with number of cycles of initiation interval.



Comment at: include/clang/Basic/AttrDocs.td:2676
+  the software pipeliner to try the specified initiation interval.
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:

aaron.ballman wrote:
> I'm can't quite parse this sentence either. Is it trying to say that the 
> scheduler will try to find a loop iteration cycle count that most-closely 
> matches the specified initiation interval?
I wanted to say that pipeliner will try to find a schedule that exactly matches 
the specified initiation interval. And if schedule would be found then single 
iteration of the optimized loop would have exactly the same amount of cycles as 
initiation interval. trying to rephrase :

If schedule would be found then single iteration of the optimized loop would 
have exactly the same amount of cycles as initiation interval has. 


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2650
+Software Pipelining optimization is a technique used to optimize loops by
+  utilizing instructions level parallelism. It reorders loop instructions to
+  overlap iterations. As the result next iteration started before previous

instructions level -> instruction-level



Comment at: include/clang/Basic/AttrDocs.td:2651
+  utilizing instructions level parallelism. It reorders loop instructions to
+  overlap iterations. As the result next iteration started before previous
+  have finished. The Modulo Scheduling technique creates schedule for one

As a result, the next iteration starts before the previous iteration has 
finished.



Comment at: include/clang/Basic/AttrDocs.td:2652-2654
+  have finished. The Modulo Scheduling technique creates schedule for one
+  iteration such that when repeating at regular interval no inter-iteration
+  dependence violated. This constant interval(in cycles) between the start

The module scheduling technique creates a schedule for one iteration such that 
when repeating at regular intervals, no inter-iteration dependencies are 
violated.



Comment at: include/clang/Basic/AttrDocs.td:2655
+  dependence violated. This constant interval(in cycles) between the start
+  of iterations called initiation interval. Cycles number of one iteration
+  of newly generated loop matches with Initiation Interval. For further

is called the initiation interval.

I can't quite parse the second sentence, though. Is it trying to say that the 
initiation interval is the number of cycles for a single iteration of the 
optimized loop?



Comment at: include/clang/Basic/AttrDocs.td:2676
+  the software pipeliner to try the specified initiation interval.
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:

I'm can't quite parse this sentence either. Is it trying to say that the 
scheduler will try to find a loop iteration cycle count that most-closely 
matches the specified initiation interval?



Comment at: include/clang/Basic/AttrDocs.td:2677
+  If schedule found the resulting loop iteration would have specified
+  cycle count. The positive number greater than zero can be specified:
+

The initiation interval must be a positive number greater than zero.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179248.
alexey.lapshin added a comment.

put a better link to doc


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 179145.
alexey.lapshin added a comment.

update documentation section.


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2677-2678
+`language extensions
+`_
+for further details, including limitations of the pipeline hints.
+  }];

aaron.ballman wrote:
> There is nothing in the linked documentation that describes pipeline 
> initiation intervals, so I'm still left wondering how to pick a value for the 
> attribute argument. I think the docs need a bit more detail for what that 
> value means and how it impacts the user's code.
How about this variant :

Software Pipelining optimization is a technique used to optimize loops by
utilizing instructions level parallelism. It reorders loop instructions to
overlap iterations. As the result new iteration started before previous
have finished. The Modulo Scheduling technique creates schedule for one
iteration such that when repeating at regular interval no inter-iteration
dependence violated. This constant interval(in cycles) between the start
of iterations called initiation interval. Cycles number of one iteration
of newly generated loop matches with Initiation Interval. For further
details see https://en.wikipedia.org/wiki/Software_pipelining and
"Swing Modulo Scheduling: A Lifetime-Sensitive Approach", by J. Llosa,
A. Gonzalez, E. Ayguade, and M. Valero.

``#pragma clang loop pipeline`` and `#pragma loop pipeline_initiation_interval``
could be used as hints for Software Pipelining optimization.

Using ``#pragma clang loop pipeline(disable)`` avoids software pipelining 
optimization. The disable state can only be specified:

.. code-block:: c++

  #pragma clang loop pipeline(disable)
  for (...) {
...
  }


Using ``#pragma loop pipeline_initiation_interval``
instructs the software pipeliner to check the specified initiation interval.
If schedule found the result loop iteration would have specified
cycle count:

.. code-block:: c++

 #pragma loop pipeline_initiation_interval(10)
 for (...) {
   ...
 }



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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2673
+
+Check `Software pipelining`, `Modulo scheduling` to get more details on 
optimisation.
+

alexey.lapshin wrote:
> aaron.ballman wrote:
> > "Check" -- check where? Perhaps "Search for" but I still don't think that's 
> > very satisfying for our documentation as there's no telling what users will 
> > find or how it relates (if at all) to this feature.
> > 
> > on optimisation -> on this optimization
> > 
> > I find this sentence doesn't really direct me to much of value (research 
> > presentations and class lectures, mostly), so I'd rather see the important 
> > details spelled out in our docs so that users don't have to guess as much.
> I am just unsure which references to include here ... links to wikipedia or 
> some concrete resources do not look good. 
> 
> So you are suggesting to spell things out here... Ok, I will prepare short 
> explanation...
> I am just unsure which references to include here ... links to wikipedia or 
> some concrete resources do not look good.

If there's a definitive source for the information, like a research paper or a 
Wikipedia entry that does a great job explaining it, then feel free to link to 
it. We can always update dead links when they arise.

> So you are suggesting to spell things out here... Ok, I will prepare short 
> explanation...

If there's not a good place to link to that has the information, then yes, 
spelling it out would be great. Basically, someone reading these docs should 
have enough information available (directly or through links) to help them 
understand the feature even if it doesn't make them an expert in the topic. My 
concern with the docs right now is that I have no idea what value to put there 
(Does it have to be positive? Nonzero? Is there an upper bound?) because I 
can't find an explanation about what that value actually controls.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2673
+
+Check `Software pipelining`, `Modulo scheduling` to get more details on 
optimisation.
+

aaron.ballman wrote:
> "Check" -- check where? Perhaps "Search for" but I still don't think that's 
> very satisfying for our documentation as there's no telling what users will 
> find or how it relates (if at all) to this feature.
> 
> on optimisation -> on this optimization
> 
> I find this sentence doesn't really direct me to much of value (research 
> presentations and class lectures, mostly), so I'd rather see the important 
> details spelled out in our docs so that users don't have to guess as much.
I am just unsure which references to include here ... links to wikipedia or 
some concrete resources do not look good. 

So you are suggesting to spell things out here... Ok, I will prepare short 
explanation...


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:2673
+
+Check `Software pipelining`, `Modulo scheduling` to get more details on 
optimisation.
+

"Check" -- check where? Perhaps "Search for" but I still don't think that's 
very satisfying for our documentation as there's no telling what users will 
find or how it relates (if at all) to this feature.

on optimisation -> on this optimization

I find this sentence doesn't really direct me to much of value (research 
presentations and class lectures, mostly), so I'd rather see the important 
details spelled out in our docs so that users don't have to guess as much.



Comment at: include/clang/Basic/AttrDocs.td:2677-2678
+`language extensions
+`_
+for further details, including limitations of the pipeline hints.
+  }];

There is nothing in the linked documentation that describes pipeline initiation 
intervals, so I'm still left wondering how to pick a value for the attribute 
argument. I think the docs need a bit more detail for what that value means and 
how it impacts the user's code.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 178956.
alexey.lapshin edited the summary of this revision.
alexey.lapshin added a comment.

Please consider this change:

1. addressed style issues and formatted patch with clang-format
2. renamed pragma pipeline_ii_count to pipeline_initiation_interval
3. left LoopAttributes inclass initializers refactoring for some another fix :-)
4. added more tests


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline_initiation_interval(10)
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline disable
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_initiation_interval()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_initiation_interval(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_initiation_interval 1 2
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline_initiation_interval(1
+/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0)
+/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_initiation_interval(4)
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */
+  for (int i = 0; i < Length; i++) {
+List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable)
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop 

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin added a comment.

Ok, So thank you for the suggestions. I will implement that way.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

aaron.ballman wrote:
> alexey.lapshin wrote:
> > aaron.ballman wrote:
> > > alexey.lapshin wrote:
> > > > aaron.ballman wrote:
> > > > > Missing full stops at the end of the comments. Also, having read "for 
> > > > > only 'Value' value", I'm still not certain what's happening. I think 
> > > > > this is a count of some kind, so perhaps "Tries to pipeline 'Values' 
> > > > > times." but I don't know what the verb "pipeline" means in this 
> > > > > context.
> > > > > 
> > > > > Are users going to understand what the `ii` means in the user-facing 
> > > > > name?
> > > > As to ii - yes that should be understood by users, because it is 
> > > > important property of software pipelining optimization. Software 
> > > > Pipelining optimization tries to reorder instructions of original 
> > > > loop(from different iterations) so that resulting loop body takes less 
> > > > cycles. It started from some minimal value of ii and stopped with some 
> > > > maximal value.  i.e. it tries to built schedule for min_ii, then 
> > > > min_ii+1, ... until schedule is found or until max_ii reached.  If 
> > > > resulting value of ii already known then instead of searching in range 
> > > > min_ii, max_ii - it is possible to create schedule for already known 
> > > > ii. 
> > > > 
> > > > probably following spelling would be better : 
> > > > 
> > > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > > 'Value'
> > > > because it is important property of software pipelining optimization. 
> > > 
> > > My point is that I have no idea what "ii" means and I doubt I'll be alone 
> > > -- does the "ii" differentiate this from other kinds of pipeline loop 
> > > pragmas we plan to support, or is expected that "pipeline_ii_count" be 
> > > the only pipeline count option? Can we drop the "ii" and not lose 
> > > anything?
> > > 
> > > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > > 'Value'
> > > 
> > > equals 'Value' -> equal to 'Value'
> > Do you think spelling out  ii would help ? 
> > 
> > pipeline_initiation_interval(number)
> I think it would help. I'm less concerned about the internal names of things 
> than I am for the user-facing identifiers and whether it will be 
> understandable to people other than compiler and optimizer writers.
Yes, I also think that it would help to spell this out.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

alexey.lapshin wrote:
> aaron.ballman wrote:
> > alexey.lapshin wrote:
> > > aaron.ballman wrote:
> > > > Missing full stops at the end of the comments. Also, having read "for 
> > > > only 'Value' value", I'm still not certain what's happening. I think 
> > > > this is a count of some kind, so perhaps "Tries to pipeline 'Values' 
> > > > times." but I don't know what the verb "pipeline" means in this context.
> > > > 
> > > > Are users going to understand what the `ii` means in the user-facing 
> > > > name?
> > > As to ii - yes that should be understood by users, because it is 
> > > important property of software pipelining optimization. Software 
> > > Pipelining optimization tries to reorder instructions of original 
> > > loop(from different iterations) so that resulting loop body takes less 
> > > cycles. It started from some minimal value of ii and stopped with some 
> > > maximal value.  i.e. it tries to built schedule for min_ii, then 
> > > min_ii+1, ... until schedule is found or until max_ii reached.  If 
> > > resulting value of ii already known then instead of searching in range 
> > > min_ii, max_ii - it is possible to create schedule for already known ii. 
> > > 
> > > probably following spelling would be better : 
> > > 
> > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > 'Value'
> > > because it is important property of software pipelining optimization. 
> > 
> > My point is that I have no idea what "ii" means and I doubt I'll be alone 
> > -- does the "ii" differentiate this from other kinds of pipeline loop 
> > pragmas we plan to support, or is expected that "pipeline_ii_count" be the 
> > only pipeline count option? Can we drop the "ii" and not lose anything?
> > 
> > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > 'Value'
> > 
> > equals 'Value' -> equal to 'Value'
> Do you think spelling out  ii would help ? 
> 
> pipeline_initiation_interval(number)
I think it would help. I'm less concerned about the internal names of things 
than I am for the user-facing identifiers and whether it will be understandable 
to people other than compiler and optimizer writers.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

aaron.ballman wrote:
> alexey.lapshin wrote:
> > aaron.ballman wrote:
> > > Missing full stops at the end of the comments. Also, having read "for 
> > > only 'Value' value", I'm still not certain what's happening. I think this 
> > > is a count of some kind, so perhaps "Tries to pipeline 'Values' times." 
> > > but I don't know what the verb "pipeline" means in this context.
> > > 
> > > Are users going to understand what the `ii` means in the user-facing name?
> > As to ii - yes that should be understood by users, because it is important 
> > property of software pipelining optimization. Software Pipelining 
> > optimization tries to reorder instructions of original loop(from different 
> > iterations) so that resulting loop body takes less cycles. It started from 
> > some minimal value of ii and stopped with some maximal value.  i.e. it 
> > tries to built schedule for min_ii, then min_ii+1, ... until schedule is 
> > found or until max_ii reached.  If resulting value of ii already known then 
> > instead of searching in range min_ii, max_ii - it is possible to create 
> > schedule for already known ii. 
> > 
> > probably following spelling would be better : 
> > 
> > pipeline_ii_count: create loop schedule with initiation interval equals 
> > 'Value'
> > because it is important property of software pipelining optimization. 
> 
> My point is that I have no idea what "ii" means and I doubt I'll be alone -- 
> does the "ii" differentiate this from other kinds of pipeline loop pragmas we 
> plan to support, or is expected that "pipeline_ii_count" be the only pipeline 
> count option? Can we drop the "ii" and not lose anything?
> 
> > pipeline_ii_count: create loop schedule with initiation interval equals 
> > 'Value'
> 
> equals 'Value' -> equal to 'Value'
Do you think spelling out  ii would help ? 

pipeline_initiation_interval(number)


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

alexey.lapshin wrote:
> aaron.ballman wrote:
> > Missing full stops at the end of the comments. Also, having read "for only 
> > 'Value' value", I'm still not certain what's happening. I think this is a 
> > count of some kind, so perhaps "Tries to pipeline 'Values' times." but I 
> > don't know what the verb "pipeline" means in this context.
> > 
> > Are users going to understand what the `ii` means in the user-facing name?
> As to ii - yes that should be understood by users, because it is important 
> property of software pipelining optimization. Software Pipelining 
> optimization tries to reorder instructions of original loop(from different 
> iterations) so that resulting loop body takes less cycles. It started from 
> some minimal value of ii and stopped with some maximal value.  i.e. it tries 
> to built schedule for min_ii, then min_ii+1, ... until schedule is found or 
> until max_ii reached.  If resulting value of ii already known then instead of 
> searching in range min_ii, max_ii - it is possible to create schedule for 
> already known ii. 
> 
> probably following spelling would be better : 
> 
> pipeline_ii_count: create loop schedule with initiation interval equals 
> 'Value'
> because it is important property of software pipelining optimization. 

My point is that I have no idea what "ii" means and I doubt I'll be alone -- 
does the "ii" differentiate this from other kinds of pipeline loop pragmas we 
plan to support, or is expected that "pipeline_ii_count" be the only pipeline 
count option? Can we drop the "ii" and not lose anything?

> pipeline_ii_count: create loop schedule with initiation interval equals 
> 'Value'

equals 'Value' -> equal to 'Value'



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+"invalid argument; expected 'disable'">;
 

alexey.lapshin wrote:
> aaron.ballman wrote:
> > Can you roll this into the above diagnostic with another `%select`, or does 
> > that make it too unreadable?
> yes, it makes things too complicated. Though I could do it if necessary.
Not required, but I also didn't think the duplication here and below was a good 
approach either. But yeah, I'm not certain there's a better way to do this even 
if the list were rearranged.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin marked 2 inline comments as done.
alexey.lapshin added a comment.

Ok, I will address all issues.




Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

aaron.ballman wrote:
> Missing full stops at the end of the comments. Also, having read "for only 
> 'Value' value", I'm still not certain what's happening. I think this is a 
> count of some kind, so perhaps "Tries to pipeline 'Values' times." but I 
> don't know what the verb "pipeline" means in this context.
> 
> Are users going to understand what the `ii` means in the user-facing name?
As to ii - yes that should be understood by users, because it is important 
property of software pipelining optimization. Software Pipelining optimization 
tries to reorder instructions of original loop(from different iterations) so 
that resulting loop body takes less cycles. It started from some minimal value 
of ii and stopped with some maximal value.  i.e. it tries to built schedule for 
min_ii, then min_ii+1, ... until schedule is found or until max_ii reached.  If 
resulting value of ii already known then instead of searching in range min_ii, 
max_ii - it is possible to create schedule for already known ii. 

probably following spelling would be better : 

pipeline_ii_count: create loop schedule with initiation interval equals 'Value'



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+"invalid argument; expected 'disable'">;
 

aaron.ballman wrote:
> Can you roll this into the above diagnostic with another `%select`, or does 
> that make it too unreadable?
yes, it makes things too complicated. Though I could do it if necessary.


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

https://reviews.llvm.org/D55710



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

Missing full stops at the end of the comments. Also, having read "for only 
'Value' value", I'm still not certain what's happening. I think this is a count 
of some kind, so perhaps "Tries to pipeline 'Values' times." but I don't know 
what the verb "pipeline" means in this context.

Are users going to understand what the `ii` means in the user-facing name?



Comment at: include/clang/Basic/AttrDocs.td:2583
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width as well as interleave, unrolling count and Initiation interval 
for pipelining
+can be manually specified. See `language extensions

Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining can be explicitly specified.



Comment at: include/clang/Basic/AttrDocs.td:2654
+Specifying ``#pragma clang loop pipeline(disable)`` avoids software pipelining 
optimization.
+Only `disable` state could be specified for ``#pragma clang loop pipeline``:
+

The `disable` state can only be specified for ``#pragma clang loop pipeline``.



Comment at: include/clang/Basic/AttrDocs.td:2663
+
+Specifying the ii count parameter for ``#pragma loop pipeline_ii_count`` 
instructs software
+pipeliner to use only specified initiation interval :

Spell out `ii`

instructs software -> instructs the software



Comment at: include/clang/Basic/AttrDocs.td:2664
+Specifying the ii count parameter for ``#pragma loop pipeline_ii_count`` 
instructs software
+pipeliner to use only specified initiation interval :
+

to use only specified -> to use the specified

Remove the space before the colon as well.

Having read the docs, I would have no idea how to pick a value for the 
initiation interval. I'm still not even certain what it controls or does. Can 
you expand the documentation for that (feel free to link to other sources if 
there are authoritative places we can point the user to)?



Comment at: include/clang/Basic/AttrDocs.td:2676
+`_
+for further details including limitations of the pipeline hints.
+  }];

details including -> details, including



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1178
   "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
-  "vectorize_width, interleave, interleave_count, unroll, unroll_count, or 
distribute">;
+  "vectorize_width, interleave, interleave_count, unroll, unroll_count, 
pipeline, pipeline_ii_count or distribute">;
 

80-col



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+"invalid argument; expected 'disable'">;
 

Can you roll this into the above diagnostic with another `%select`, or does 
that make it too unreadable?



Comment at: lib/CodeGen/CGLoopInfo.cpp:29
   Attrs.UnrollAndJamCount == 0 &&
+  Attrs.PipelineDisabled == false &&
+  Attrs.PipelineIICount == 0 &&

`!Attrs.PipelineDisabled`



Comment at: lib/CodeGen/CGLoopInfo.cpp:127
 
+  if(Attrs.PipelineDisabled){
+  Metadata *Vals[] = {MDString::get(Ctx, "llvm.loop.pipeline.disable"),

Formatting is incorrect here (and below) -- you should run the patch through 
clang-format.



Comment at: lib/CodeGen/CGLoopInfo.h:70
+
+  /// Value for llvm.loop.pipeline.disable metadata
+  bool PipelineDisabled;

Missing full-stop. Here and elsewhere -- can you look at all the comments and 
make sure they're properly punctuated?



Comment at: lib/CodeGen/CGLoopInfo.h:71
+  /// Value for llvm.loop.pipeline.disable metadata
+  bool PipelineDisabled;
+

One good refactoring (don't feel obligated to do this) would be to use in-class 
initializers here.



Comment at: test/Parser/pragma-pipeline.cpp:24
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang 
loop pipeline_ii_count(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_ii_count 1 2
+  for (int i = 0; i < Length; i++) {

Can you add a few tests:

`#pragma clang loop pipeline_ii_count(1` to show that we also diagnose the 
missing closing paren
`#pragma clang loop pipeline_ii_count(1.0)` to show that we diagnose a 
non-integer literal count
`#pragma clang loop pipeline enable` to show that we diagnose the missing 

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin updated this revision to Diff 178264.
alexey.lapshin added a comment.

deleted small typo in CGLoopInfo.cpp


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

https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_ii_count or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable) 
+  for (int i = 0; i < Length; i++) {
+  List[i] = Value;
+  }
+
+#pragma clang loop pipeline_ii_count(10) 
+  for (int i = 0; i < Length; i++) {
+  List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_ii_count()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_ii_count(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_ii_count 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_ii_count(4) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_ii_count(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_ii_count(4)'}} */
+  for (int i = 0; i < Length; i++) {
+  List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_ii_count or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-pipeline.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-pipeline.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple hexagon -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void pipeline_disabled(int *List, int Length, int Value) {
+  // CHECK-LABEL: define {{.*}} @_Z17pipeline_disabled 
+#pragma clang loop pipeline(disable) 
+  for (int i = 0; i < Length; i++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
+  List[i] = Value;
+  }
+}
+
+void 

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Alexey Lapshin via Phabricator via cfe-commits]

alexey.lapshin created this revision.
alexey.lapshin added reviewers: bcahoon, kparzysz, aaron.ballman, arphaman, 
rsmith, marksl, yakush.
Herald added a subscriber: zzheng.

  [PIPELINER] add two pragmas to control Software Pipelining optimisation.
  
#pragma clang loop pipeline(disable)
  
Disable SWP optimization for the next loop.
“disable” is the only possible value.
  
#pragma clang loop pipeline_ii_count(number)
  
Set value of initiation interval for SWP
optimization to specified number value for
the next loop. Number is the positive value
greater than 0.
  
These pragmas could be used for debugging or reducing
compile time purposes. It is possible to disable SWP for
concrete loops to save compilation time or to find bugs
by not doing SWP to certain loops. It is possible to set
value of initiation interval to concrete number to save
compilation time by not doing extra pipeliner passes or
to check created schedule for specific initiation interval.
  
That is clang part of the fix


https://reviews.llvm.org/D55710

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-pipeline.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-pipeline.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_ii_count or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-pipeline.cpp
===
--- /dev/null
+++ test/Parser/pragma-pipeline.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma clang loop pipeline(disable) 
+  for (int i = 0; i < Length; i++) {
+  List[i] = Value;
+  }
+
+#pragma clang loop pipeline_ii_count(10) 
+  for (int i = 0; i < Length; i++) {
+  List[i] = Value;
+  }
+
+/* expected-error {{expected ')'}} */ #pragma clang loop pipeline(disable
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(enable)
+/* expected-error {{invalid argument; expected 'disable'}} */ #pragma clang loop pipeline(error)
+/* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop pipeline_ii_count()
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang loop pipeline_ii_count(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_ii_count 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length;
+#pragma clang loop pipeline_ii_count(4) 
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length;
+
+#pragma clang loop pipeline(disable)
+#pragma clang loop pipeline_ii_count(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_ii_count(4)'}} */
+  for (int i = 0; i < Length; i++) {
+  List[i] = Value;
+  }
+
+#pragma clang loop pipeline(disable) 
+/* expected-error {{expected statement}} */ }
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count,