[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 +