[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-08-01 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-08-01 Thread Dave Green via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC338566: [UnrollAndJam] Add unroll_and_jam pragma handling 
(authored by dmgreen, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47267?vs=158219=158537#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47267

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

Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -2748,22 +2748,27 @@
   /// interleave_count: interleaves 'Value' loop iterations.
   /// unroll: fully unroll loop if State == Enable.
   /// unroll_count: unrolls loop 'Value' times.
+  /// unroll_and_jam: attempt to unroll and jam loop if State == Enable.
+  /// unroll_and_jam_count: unroll and jams loop 'Value' times.
   /// distribute: attempt to distribute loop if State == Enable
 
   /// #pragma unroll  directive
   /// : fully unrolls loop.
   /// boolean: fully unrolls loop if State == Enable.
   /// expression: unrolls loop 'Value' times.
 
   let Spellings = [Pragma<"clang", "loop">, Pragma<"", "unroll">,
-   Pragma<"", "nounroll">];
+   Pragma<"", "nounroll">, Pragma<"", "unroll_and_jam">,
+   Pragma<"", "nounroll_and_jam">];
 
   /// State of the loop optimization specified by the spelling.
   let Args = [EnumArgument<"Option", "OptionType",
   ["vectorize", "vectorize_width", "interleave", "interleave_count",
-   "unroll", "unroll_count", "distribute"],
+   "unroll", "unroll_count", "unroll_and_jam", "unroll_and_jam_count",
+   "distribute"],
   ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount",
-   "Unroll", "UnrollCount", "Distribute"]>,
+   "Unroll", "UnrollCount", "UnrollAndJam", "UnrollAndJamCount",
+   "Distribute"]>,
   EnumArgument<"State", "LoopHintState",
["enable", "disable", "numeric", "assume_safety", "full"],
["Enable", "Disable", "Numeric", "AssumeSafety", "Full"]>,
@@ -2778,6 +2783,8 @@
 case InterleaveCount: return "interleave_count";
 case Unroll: return "unroll";
 case UnrollCount: return "unroll_count";
+case UnrollAndJam: return "unroll_and_jam";
+case UnrollAndJamCount: return "unroll_and_jam_count";
 case Distribute: return "distribute";
 }
 llvm_unreachable("Unhandled LoopHint option.");
@@ -2787,9 +2794,9 @@
 unsigned SpellingIndex = getSpellingListIndex();
 // For "#pragma unroll" and "#pragma nounroll" the string "unroll" or
 // "nounroll" is already emitted as the pragma name.
-if (SpellingIndex == Pragma_nounroll)
+if (SpellingIndex == Pragma_nounroll || SpellingIndex == Pragma_nounroll_and_jam)
   return;
-else if (SpellingIndex == Pragma_unroll) {
+else if (SpellingIndex == Pragma_unroll || SpellingIndex == Pragma_unroll_and_jam) {
   OS << ' ' << getValueString(Policy);
   return;
 }
@@ -2825,6 +2832,11 @@
   return "#pragma nounroll";
 else if (SpellingIndex == Pragma_unroll)
   return "#pragma unroll" + (option == UnrollCount ? getValueString(Policy) : "");
+else if (SpellingIndex == Pragma_nounroll_and_jam)
+  return "#pragma nounroll_and_jam";
+else if (SpellingIndex == Pragma_unroll_and_jam)
+  return "#pragma unroll_and_jam" +
+(option == UnrollAndJamCount ? getValueString(Policy) : "");
 
 assert(SpellingIndex == Pragma_clang_loop && "Unexpected spelling");
 return getOptionName(option) + getValueString(Policy);
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -185,6 +185,8 @@
   std::unique_ptr LoopHintHandler;
   std::unique_ptr UnrollHintHandler;
   std::unique_ptr NoUnrollHintHandler;
+  std::unique_ptr UnrollAndJamHintHandler;
+  std::unique_ptr NoUnrollAndJamHintHandler;
   std::unique_ptr FPHandler;
   std::unique_ptr STDCFENVHandler;
   std::unique_ptr STDCCXLIMITHandler;
Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// 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 unroll_and_jam
+  for (int i 

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-07-31 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Sema/SemaStmtAttr.cpp:183
{nullptr, nullptr}};
-
   for (const auto *I : Attrs) {

[nit] unrelated whitespace change?



Comment at: test/CodeGenCXX/pragma-unroll-and-jam.cpp:4
+void unroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z14unroll_and_jam
+#pragma unroll_and_jam

[suggestion] `CHECK-LABEL`? (applies to the function names below as well)


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-07-31 Thread Dave Green via Phabricator via cfe-commits
dmgreen updated this revision to Diff 158219.
dmgreen added a comment.

Rebase.

Michael, you happy with this part?

The pragma clang loop part is off in https://reviews.llvm.org/D47320. Let me 
know if/when I should do something with that.


https://reviews.llvm.org/D47267

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

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// 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 unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam 4
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{expected ')'}} */ #pragma unroll_and_jam(4
+/* expected-error {{missing argument; expected an integer value}} */ #pragma unroll_and_jam()
+/* expected-warning {{extra tokens at end of '#pragma unroll_and_jam'}} */ #pragma unroll_and_jam 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-warning {{extra tokens at end of '#pragma nounroll_and_jam'}} */ #pragma nounroll_and_jam 1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int j = Length;
+#pragma unroll_and_jam 4
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int k = Length;
+#pragma nounroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
+
+/* expected-error {{incompatible directives '#pragma nounroll_and_jam' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+// 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)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected statement}} */ }
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void unroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z14unroll_and_jam
+#pragma unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void unroll_and_jam_count(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z20unroll_and_jam_count
+#pragma unroll_and_jam(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_2:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z16nounroll_and_jam
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void clang_unroll_plus_nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z34clang_unroll_plus_nounroll_and_jam
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_7:.*]]
+  List[i 

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-06 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

In https://reviews.llvm.org/D47267#1123318, @hfinkel wrote:

> I have a preference for using the underscores as our primary spelling. I 
> think that it's easier to read.


I agree with it being easier to read.

> I prefer we have a different syntax that we can use consistently within the 
> 'clang loop' pragmas. How about 'unroll_and_jam disable' or similar?

The code I had for #pragma clang loop (now in https://reviews.llvm.org/D47320, 
although I may not have split all the relevant parts into there) was doing the 
same thing as the unroll code. So worked the same way, I think looking like 
"#pragma clang loop unroll_and_jam(disable)" vs enable. It sounds sensible to 
me to have these look the same way as unroll clang loop pragmas, for both the 
old syntax and the new from the RFC.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-06 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

In https://reviews.llvm.org/D47267#1123038, @Meinersbur wrote:

> In https://reviews.llvm.org/D47267#1123013, @dmgreen wrote:
>
> > I see your point about the mix of underscores. "nounroll_and_jam" also 
> > comes from the Intel docs, and theres already "nounroll" vs "unroll". The 
> > "no" becomes a qualifier on "unroll_and_jam". "no_unroll_and_jam" feels 
> > less consistent to me.
>
>
> `nounroll_and_jam` looks like it should be parsed as "(no unroll) and jam" 
> (do not unroll, but fuse) instead of "no (unroll-and-jam)" because `nounroll` 
> is one word and as you mentioned, already used as a keyword somewhere else. 
> Other variants use the underscore to append an option, e.g. `vectorize_width`.
>
> > But if you have a strong opinion, I'm happy enough to change it.
>
> I don't. Feel free to chose the name you think fits best. We might support 
> multiple spellings if necessary.


I agree that we can support multiple spellings (especially for compatibility 
with other compilers). I have a preference for using the underscores as our 
primary spelling. I think that it's easier to read. nounroll_and_jam is fine 
for compatibility if we'd like. I prefer we have a different syntax that we can 
use consistently within the 'clang loop' pragmas. How about 'unroll_and_jam 
disable' or similar?

> If we want to add more transformations, it would be nice to have an explicit 
> naming scheme. E.g. for "register tiling", "stream_unroll" (supported by 
> xlc), "index set splitting", "statement reordering", "strip mine", "overlap 
> tiling", "diamond tiling", "thread-parallelization", "task-parallelization", 
> "loop unswitching", etc.




https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-05 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In https://reviews.llvm.org/D47267#1123013, @dmgreen wrote:

> I see your point about the mix of underscores. "nounroll_and_jam" also comes 
> from the Intel docs, and theres already "nounroll" vs "unroll". The "no" 
> becomes a qualifier on "unroll_and_jam". "no_unroll_and_jam" feels less 
> consistent to me.


`nounroll_and_jam` looks like it should be parsed as "(no unroll) and jam" (do 
not unroll, but fuse) instead of "no (unroll-and-jam)" because `nounroll` is 
one word and as you mentioned, already used as a keyword somewhere else. Other 
variants use the underscore to append an option, e.g. `vectorize_width`.

> But if you have a strong opinion, I'm happy enough to change it.

I don't. Feel free to chose the name you think fits best. We might support 
multiple spellings if necessary.

If we want to add more transformations, it would be nice to have an explicit 
naming scheme. E.g. for "register tiling", "stream_unroll" (supported by xlc), 
"index set splitting", "statement reordering", "strip mine", "overlap tiling", 
"diamond tiling", "thread-parallelization", "task-parallelization", "loop 
unswitching", etc.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-05 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

I quite like the UnrollAndFuse naming. I'd not heard that the xlc compiler 
called it that. The UnrollAndJam pass was origin named that before I renamed 
for similar reasons (UnrollAndJam being more well known).

I see your point about the mix of underscores. "nounroll_and_jam" also comes 
from the Intel docs, and theres already "nounroll" vs "unroll". The "no" 
becomes a qualifier on "unroll_and_jam". "no_unroll_and_jam" feels less 
consistent to me.

But if you have a strong opinion, I'm happy enough to change it.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-05 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In https://reviews.llvm.org/D47267#1122425, @dmgreen wrote:

> I noticed in the paper that you used the name "unrollandjam", minus 
> underscores. Should I change this use that spelling here? I have no strong 
> opinion of one over the other (was just using what I had found from the Intel 
> docs).


IMHO you can keep `unroll_and_jam` (which is already supported by Intel 
syntax). When I imagined the name, I had xlc's `unrollandfuse` in mind, but 
found that "and jam" is better known than "and fuse".

We can have a discussion about how to name them in general. `nounroll_and_jam` 
seems a strange mix of write words together and separate words by underscores.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-05 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

Thanks.

I noticed in the paper that you used the name "unrollandjam", minus 
underscores. Should I change this use that spelling here? I have no strong 
opinion of one over the other (was just using what I had found from the Intel 
docs).


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-06-01 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

The RFC: https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-24 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Yes, this is what I had in mind. Thank you.

I am preparing an RFC on what I am trying to do. This should clarify our goals 
and to what extend `#pragma clang loop` conflicts with it.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-24 Thread Dave Green via Phabricator via cfe-commits
dmgreen updated this revision to Diff 148393.
dmgreen added a comment.

This splits out the pragma clang loop unroll_and_jam handling into 
https://reviews.llvm.org/D47320, for if/when we need it. Which I believe is 
what you wanted, correct me if I'm wrong.


https://reviews.llvm.org/D47267

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

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// 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 unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam 4
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{expected ')'}} */ #pragma unroll_and_jam(4
+/* expected-error {{missing argument; expected an integer value}} */ #pragma unroll_and_jam()
+/* expected-warning {{extra tokens at end of '#pragma unroll_and_jam'}} */ #pragma unroll_and_jam 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-warning {{extra tokens at end of '#pragma nounroll_and_jam'}} */ #pragma nounroll_and_jam 1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int j = Length;
+#pragma unroll_and_jam 4
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int k = Length;
+#pragma nounroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
+
+/* expected-error {{incompatible directives '#pragma nounroll_and_jam' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+// 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)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected statement}} */ }
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void unroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z14unroll_and_jam
+#pragma unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void unroll_and_jam_count(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z20unroll_and_jam_count
+#pragma unroll_and_jam(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_2:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z16nounroll_and_jam
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void clang_unroll_plus_nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z34clang_unroll_plus_nounroll_and_jam
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop 

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-24 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Just out of curiousity:

- How do you plan to implement this? Are you going to generate from the pragma 
some sort of "script" that dictates the transformation order which is going to 
be fed to the pass manager?
- About the stacking of pragmas, in your example you apply the bottom one 
first, but would a user perhaps expect the first to be applied? In other words, 
is the expected behaviour described somewhere (in a spec)?


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-23 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

In https://reviews.llvm.org/D47267#1109912, @dmgreen wrote:

> > Could we maybe disable the #pragma clang loop unroll_and_jam spelling ftm 
> > to avoid compatibility issues?
>
> Sure, I'm not against. It sounds like you have opinions on how this should 
> work. That's good. If there are multiple clang loop pragma's, what is the 
> expected behaviour there?
>
> In the llvm side of this, in the unroll and jam pass, I made it so that a 
> loop with "llvm.loop.unroll" metadata without any "llvm.loop.unroll_and_jam" 
> metadata will not do unroll and jam, it will leave the loop for the unroller. 
> On the expectation that the use really wants to unroll (and it applies to 
> llvm.loop.unroll.disable too, disabling unroll and jam as well as unroll). I 
> haven't done anything with other loop metadata though.


I'd expect that transformations "stack up". E.g.

  #pragma unroll_and_jam
  #pragma unroll(4)

which I'd expect to unroll by a factor of 4 first, then try to unroll-and-jam 
(which I am not sure https://reviews.llvm.org/D41953 can do due to then 
containing 4 loops). On the other hand

  #pragma unroll(4)
  #pragma unroll_and_jam

should unroll-and-jam, and then unroll the outer loop by a factor of 4.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-23 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

> In my experience, they are used.

Good to know, cheers.

> Could we maybe disable the #pragma clang loop unroll_and_jam spelling ftm to 
> avoid compatibility issues?

Sure, I'm not against. It sounds like you have opinions on how this should 
work. That's good. If there are multiple clang loop pragma's, what is the 
expected behaviour there?

In the llvm side of this, in the unroll and jam pass, I made it so that a loop 
with "llvm.loop.unroll" metadata without any "llvm.loop.unroll_and_jam" 
metadata will not do unroll and jam, it will leave the loop for the unroller. 
On the expectation that the use really wants to unroll (and it applies to 
llvm.loop.unroll.disable too, disabling unroll and jam as well as unroll). I 
haven't done anything with other loop metadata though.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-23 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

This is straightforward in that it clones the implementation of `#pragma 
unroll` for `unroll_and_jam`.

Hence, it also shares the same problem of clang's LoopHints, namely that the 
order of loop transformations (if there are multiple on the same loop: loop 
distribution, vectorization, etc) is defined by the order of the passes in the 
pass pipeline, which should be an implementation detail.

I am currently working on this topic. Could we maybe disable the `#pragma clang 
loop unroll_and_jam` spelling ftm to avoid compatibility issues? However, the 
problem already exists for the other loop hints, so I will have to ensure 
compatibility with those anyway.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-23 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

> I have some doubts whether this will ever be used sensibly in the real world, 
> but can
>  be useful for testing and was not difficult to put together.

I definitely support having pragmas for these kinds of loop transformations. In 
my experience, they are used.


https://reviews.llvm.org/D47267



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


[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

2018-05-23 Thread Dave Green via Phabricator via cfe-commits
dmgreen created this revision.
dmgreen added reviewers: SjoerdMeijer, hfinkel, tyler.nowicki, anemet, alexfh.
Herald added a subscriber: zzheng.

This adds support for the unroll_and_jam pragma, to go with 
https://reviews.llvm.org/D41953. The name of
the pragma is copied from the Intel compiler, and most of the code works the 
same as
for unroll.

I have some doubts whether this will ever be used sensibly in the real world, 
but can
be useful for testing and was not difficult to put together.


https://reviews.llvm.org/D47267

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

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// 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 unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam 4
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{expected ')'}} */ #pragma unroll_and_jam(4
+/* expected-error {{missing argument; expected an integer value}} */ #pragma unroll_and_jam()
+/* expected-warning {{extra tokens at end of '#pragma unroll_and_jam'}} */ #pragma unroll_and_jam 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-warning {{extra tokens at end of '#pragma nounroll_and_jam'}} */ #pragma nounroll_and_jam 1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int j = Length;
+#pragma unroll_and_jam 4
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int k = Length;
+#pragma nounroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
+
+/* expected-error {{incompatible directives 'unroll_and_jam(disable)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma clang loop unroll_and_jam(disable)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{incompatible directives 'unroll_and_jam(full)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam(4)
+#pragma clang loop unroll_and_jam(full)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* 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, unroll_and_jam, unroll_and_jam_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-unroll-and-jam.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+