OpenMP support in CLANG: A proposal

Project Infrastructure LLVM Dev List Archives
#21

Hi Olaf,

Here is my further update.

1. As I already mentioned in my previous reply, extra redundant
Handler classes are removed, and we can do magic with just one Handler
class.

2. Later, after shooting my reply to your mail, I started to think
that I can even get rid of extra annotated omp tokens, as you
inidicated. I can achieve similar functionality by keeping *only one*
generic omp token, and *without* lexing the omp directive *name* in
the Handler, instead allowing it to take care by Parser.

3. Regarding your suggestion of *not* disturbing Global Parser and
Sema, I really doubt, if I can avoid it. I agree that, Global Parser
and Sema should not be disturbed to handle the stuffs which is suppose
to be handled by Preprocessor. But, as you already guessed, re-design
for Clang is required to achieve this, and I do not think if it can be
achieved very quickly though it may not be an impossible task.

4. I quickly went through the following link to understand the concept
of C++11 attributes -
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2761.pdf. In
this doc, I see some proposal to represent OpenMP directives as C++11
attributes, though I do not know if any other compilers have already
implemented OpenMP by parsing C++11 attribute based OpenMP
representation. I am bit confused here about your suggestion of not
using ASTs to represent OpenMP statements. As per my understanding,
GCC and other compilers create ASTs for OpenMP constructs - and make
use of these ASTs for further *outlining* of parallel OpenMP regions,
and sub-sequent *lowering*. I do have an opinion that ASTs are very
much required to process classic OpenMP *pragma* constructs. However,
I appreciate, if you can share your suggestion with more *detail*

#22

Hi,

a general note at first: I noticed the line

PP.EnterTokenStream(Toks, 1, /*DisableMacroExpansion=*/true,
                         /*OwnsTokens=*/false);

in your code. IMHO DisableMacroExpansion should be false, since all text after a #pragma omp is subject to macro expansion (OpenMP 3.1, sec.2.1).
Also I dont understand what you mean with scope in the case of "if" clauses.

I see, what you want to accomplish by integrating OpenMP directly in the parser. If you encounter a omp parallel directive you instruct the parser to parse a structured block. And if that fails it is an error.

I would go another route. I would stll see OpenMP directives as attributes to statements. And by the moment these attributes are attached to a statement the appropriate checks can be made.
Try to attach an "omp parallel" directive to a class declaration -> error
Try to attach an "omp parallel for" directive to a non-conforming for-Statement -> error.

You see by the second example, that with this approach you aren't longer limited by the means of the parser.

Of course you are right by writing:
> So, we have to piggy back Global parser
> to handle scopes, parse expressions and compound statement (structured
> block).
Thats what I meant in point 4 of my previous writing. We have to shout it out as often as possible to the right people (I don't belong to them) that the Parser needs a serious refactoring in order to support expressions inside pragmas and (later) C++0x-attributes.
To say it with Douglas Gregors words from a another posting: while OpenMP is just a standard, using it is an industry trend. (actually I have wondered that the clang people could ignore it successfully for such a long time).

So my suggestion remains: try to handle OpenMP directives as attributes. Extend the attribute framework in a way, that made it possible to user-check attributes at the time they are attached to the appropriate AST entity. And refactor the parser framework so that it is possible to parse code snippets.

Best Olaf

#23

Ok, give me some time to understand your proposal. Regarding your
question about *scope*, I meant following example. In OpenMP
annotated C++ source, following code snippet is valid. In this
particular example, I meant about handling the scope of variable
"VarA" .

#24

This is not backed up by the OpenMP standard. The Grammar mentions only

if (expression)

And the C++ standard has its own grammar rules for expressions in conditions in order to make declarations possible.
Of course if the above extension is consensus among all other compilers then we have to adopt to it. However I doubt it. It begs for trouble if you e.g. compile with OpenMP disabled. Thus, if this is just an gcc extension I'd just leave it out.

Best Olaf

#25

I was puzzled by this too and I quickly browse again the OpenMP 3.1
standard.

I think we should get back to the "OpenMP spirit", but of course it is
very subjective and I had some passionate debates on this (hi Antoniu
Pop! :slight_smile: ).

For me, the first example violates the property that if a compiler
ignores the #pragma omp, the program should remain a syntax-correct
program here. If we have an example where VarA is not defined before but
used inside the parallel block, it would not even compile.

The second example is that we have a side effect on VarA = 2.
This makes me uncomfortable since I try usually to write OpenMP programs
with the same semantics as the sequential program resulting of ignoring
the #pragma. It is where we can argument on what is the "OpenMP spirit",
since I do not remember having seen this stated as an axiom in the norm.

The only thing I can relate to it in the "OpenMP Application Program
Interface Version 3.1 July 2011" norm:

p. 35: Restrictions
lines 24-25:

    A program must not depend on any ordering of the evaluations of the
    clauses of the parallel directive, or on any side effects of the
    evaluations of the clauses.

p. 36: 2.4.1 Determining the Number of Threads for a parallel Region
lines 16-19:

    The if clause expression and the num_threads clause expression are
    evaluated in the context outside of the parallel construct, and no
    ordering of those evaluations is specified. It is also unspecified
    whether, in what order, or how many times any side-effects of the
    evaluation of the num_threads or if clause expressions occur.

So these not preclude the use of side effect (well, I had not imagined
to be able to put side effects here. Thank you for this Halloween
suggestion :slight_smile: ). I just keep thinking side effects in the OpenMP pragma
is against the OpenMP spirit and depends too much on implementation
details.

So clearly
#pragma omp parallel num_threads(++i) if(++i)
is forbidden :slight_smile:

If we go back to:

#pragma omp parallel if (VarA = 2)
{
    // parallel region
}
this is not forbidden by the norm but I choke on the fact that if we
have an OpenMP compiler, the value of VarA is 2 in the continuation of the program
after the #pragma, but if we compile without an OpenMP compiler, the
value of VarA is unchanged neither even accessed.

So to summarize: I vote for Olaf Krzikalla on this point. :slight_smile:

#26

Sometimes, the *dry* definitions of standard (I do not hesitate to use
the word *dry* here) makes the implementation job very messy. And, I
am sure, it is true in all standards including C/C++ standard.

GCC ( g++ ) happily reports *un-declared* error for the variable VarA
when I do not pass -fopenmp option, and abort the compilation. Yes,
OpenMP spirit is clearly broken here. Though, I did not test the
other compilers, I guess that they follow GCC for whatever the
reasons.

And, now, if Clang starts fighting against these clearly violated
rules, it is very probable that users will start their voice against
it.

#27

Hi Mahesha,

Sometimes, the *dry* definitions of standard (I do not hesitate to use
the word *dry* here) makes the implementation job very messy. And, I
am sure, it is true in all standards including C/C++ standard.

I think it would actually be easier to just handle an expression
inside the if() clause.

GCC ( g++ ) happily reports *un-declared* error for the variable VarA
when I do not pass -fopenmp option, and abort the compilation. Yes,
OpenMP spirit is clearly broken here. Though, I did not test the
other compilers, I guess that they follow GCC for whatever the
reasons.

And, now, if Clang starts fighting against these clearly violated
rules, it is very probable that users will start their voice against
it.

While it makes sense to be compatible with other compilers, being
bug-for-bug compatible is clearly the wrong way to go. This
"extension" violates the OMP spirit, has vague semantics and thus, in
my opinion, has no practical use.

Dmitri

#28

Ok. Shall I assume the following basic thumb rule for our OpenMP
support in Clang?

"The OpenMP annotated C/C++ source when compiled with OpenMP
*disabled* should be (both syntactically and semantically) equal to
its sequential counterpart. Any OpenMP (non-standard) extension which
violates the above rule will not be supported even if it will going to
be *not* compatible with existing OpenMP compilers".

#29

Hi,

#30

While I don't think it is possible to guarantee that, I would agree
that this is the spirit of OpenMP. But I would not make a statement
as strong as that -- every extension is different and should be
considered on a case by case basis. If we find an extension that
*lots* of code depends on, then we might consider implementing that.
But in my opinion that particular if(int var = ...) extension does not
meet this bar.

Dmitri

#31

I understand. I was bit aggressive with my previous *generalized*
statement. Let us discuss such topics case by case, when we encounter
their implementation in near future.

As of now, I am really trying to understand the Olaf's suggestion of
treating OpenMP *pragmas* as *attributes* and implement them
accordingly. This is actually the core of the implementation in my
opinion, and I appreciate any further discussions in this line. I
would request you guys to go through his proposal, and pour some
lights upon it.

#32

GCC ( g++ ) happily reports *un-declared* error for the variable VarA
when I do not pass -fopenmp option, and abort the compilation. Yes,
OpenMP spirit is clearly broken here. Though, I did not test the
other compilers, I guess that they follow GCC for whatever the
reasons.

MSVC rejects the code.

And, now, if Clang starts fighting against these clearly violated
rules, it is very probable that users will start their voice against
it.

I don't think so. I haven't seen something like this anywhere in production code. It is just way over the top (I believe it is even a non-intentionally extension of gcc introduced by an unified handling of condition-expressions).
Regarding side effects in expressions, while these might be considered evil, they are actually allowed by the OpenMP standard. But they don't impose any problems to the implementation.

A program shall compile and run with and without -fopenmp. It is not required that both variants yield the same output.

Best Olaf

#33

GCC ( g++ ) happily reports *un-declared* error for the variable VarA
when I do not pass -fopenmp option, and abort the compilation. Yes,
OpenMP spirit is clearly broken here. Though, I did not test the
other compilers, I guess that they follow GCC for whatever the
reasons.

MSVC rejects the code.

(I believe it is even a non-intentionally
extension of gcc introduced by an unified handling of
condition-expressions).

Possible.

Regarding side effects in expressions, while these might be considered evil,
they are actually allowed by the OpenMP standard. But they don't impose any
problems to the implementation.

A program shall compile and run with and without -fopenmp. It is not
required that both variants yield the same output.

Yes. I agree. We can probably re-discuss about it when we encounter
the implementation in a near future, if required.

Now, I am going through the Clang code base which is related to
*attributes* implementation. Once, I get hold of this code base, I
guess, some discussion is required around how to implement OpenMP
*pragmas* as *attributes*. Meanwhile, it would be great if your share
your thoughts too in this line, I mean from the perspective of
implementation strategy. I do think that common code base should serve
the need of both the syntax - classic pragma syntax and new C++11
attribute based syntax.

#34

Hi,

I spent some time going through the code base related to *attributes*
support in Clang. As per the definition of the class "class
AttributeList" in clang/include/clang/Sema/AttributeList.h, the
implementation aims at supporting following four kinds of attributes
syntax as of today.

1. Classis GNU syntax
2. C++11X syntax
3. Microsoft declspec syntax
4. Microsoft typespec syntax

If OpenMP standards body accept the new C++11X based OpenMP syntax
like "for [[omp::for(clause, clause), … ]] (loop-head) loop-body",
then its implementation belongs to above syntax category 2 (C++11X
syntax).

However, as you are suggesting to treat OpenMP classic *pragma*
syntax too as *attribute* syntax, one way of doing it is by including
pragma syntax as another attribute syntax item to above list as 5th
one. But, I do not know - at this point - how well we can fit the
OpenMP pragma syntax as attribute syntax. I would ask other community
members to comment on this approach.

On the other hand, internal mapping of OpenMP pragma syntax to new
C++11X based OpenMP syntax will become *messy* in many ways starting
from preserving source location information till error reporting. So,
I am personally *against* this approach unless somebody proves that
this approach will be neat and it will not be error prone, and it will
not be maintenance head ache. That said, door is open for others to
discuss about this approach.

#35

Hello All,
We're interested in Mahesha's efforts on OpenMP support in Clang. We're
going to collaborate with Mahesha and take a part in this work.
Alexey Bataev
Software Engineer
Intel Compiler Team
Intel Corp.

#36

Hi all,

As Alexey Bataev mentioned in his mail, we are collaborating on
implementing OpenMP in Clang. If you guys are now free to review my
*initial* patches, I will start sending them.

#37

Alexey, Mahesha,

This is all very good news! Please send the patches; I'll try to review them next week.

Alexey, it would be helpful if we had code reviews of these patches from Intel as well.

Thanks again,
Hal

#38

Hello Hal,
I'm working on it. If I'll find anything, I'll report as soon as possible.
Best,
Alexey