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

Hal Finkel via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 6 02:03:27 PDT 2018


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





More information about the cfe-commits mailing list