[cfe-dev] OpenMP support in CLANG: A proposal

[Mahesha HS]

On Tue, Nov 6, 2012 at 8:45 PM, Olaf Krzikalla
<Olaf.Krzikalla at tu-dresden.de> wrote:
> Hi,
>
> I did a quick look at your code and have some comments about it:

Thanks for the review Olaf. Yes, I have re-factored the code to a
greater extent to simplify the things, and I am still working on
re-factoring. However, please find my answers to some of your
questions which are inlined.

>
> 1. I don't think it is neccessary to introduce different classes for the
> various sub-domains. One pragma handler class should do the trick.

I already re-implemented it exactly as you mentioned. The initial
thinking of introducing different classes was based on the design
decision that "Pragma handler" should call appropriate handler based
on the omp directive name like "#pragma omp parallel", "#pragma omp
for", etc. So I landed up deriving a separate class for each omp
pragma directive name, from pragma handler interface.  Now, I
re-implemented it by having only one omp pragma handler class, and
parsing the omp directive names by making use of appropriate enums and
string comparison.

> Also I
> don't think that you need extra tokens.

If we handle everything within a Handler it-self, then, tokens may not
be required. However, OpenMP syntax and semantics is quite involved.
We have to ask global Parser and Sema to do it. Following are the
reasons for it.

1. OpenMP statements are part of function body statements.
2. OpenMP statements contain expression. scope (in case of "if"
clause) and structured blocks. So, we have to piggy back Global parser
to handle scopes, parse expressions and compound statement (structured
block). Moreover,  few OpenMP constructs like "barrier" are valid only
within a structured block (parallel region) which follows omp parallel
constructs. That means,  OpenMP parsing is recursive in nature.

So we need to have tokens which represent different OpenMP directives
(note - only for directives not for clauses), and Handler will insert
those tokens to token streams depending on the type of the omp
directive. Then, global Parser and Sema will take it forward.

> The sub-domain is always an
> identifier. Parse it and schedule based on a string comparision.

I agree. I am already re-implemented  exactly as you mentioned.

>
> 2. Do not derive OpenMP directives from Stmt. Literally they are no
> statements but directives. OpenMP directives attribute the statement
> following immediately (and I use "attribute" here by full intent - always
> keep C++11 attributes in mind). If there are also other pragmas before a
> for-statement you might run in serious trouble. That said, you should try to
> use (and maybe extend) the already installed attribute framework in the AST.

I accept. I will re-think about it, and re-implement it. Probably I
will share the AST design before implementing.

>
> 3. I wonder if we need to touch Sema and Parser the way you do. Granted, the
> Parser needs some extensions (more on that in the next bullet). But the
> parsing should be done in the handler, since OpenMP is not a part of C/C++
> but rather an extension.

As I mentioned above, I do think that global Parser and Sema need to
be involved in handling of OpenMP stuffs, at least for the time being,
as per the present Clang design. However, this topic is subjected to
further discussion.

--
mahesha

>
> 4. Last and most important point for other people still reading: clang DOES
> need a way to parse expressions inside of pragmas. Admittedly this violates
> the separation of concerns since pragmas exist at the preprocessor level and
> expressions exist at the Sema level. However that's just how it is. I hacked
> my own solution in Scout but after many hours of bug fixing I won't
> advertise that solution here. Instead I would opt for a parser redesign so
> that it is possible to have more than one parser at the same time operating
> on a token stream. Then we could construct a parser in a pragma handler and
> just call ParserExpression (or what else needs to be parsed). Thus the state
> of the global parser is not touched and it will happily jump to eol.
>
>
> Best regards
> Olaf
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



-- 
mahesha