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

[Mahesha HS]

On Fri, Oct 19, 2012 at 4:03 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Thu, Oct 18, 2012 at 6:34 AM, Mahesha HS <mahesha.llvm at gmail.com> wrote:
>> Sorry, in my previous mail,  I had missed to attach changes to
>> "clang/include/clang/Basic/TokenKinds.def" in the patch 2. Please
>> refer to the patch (2) attached in *this* mail, instead of the one
>> sent in the previous mail. Patch 1 is fine.
>
> +//--------------------------------
> +// OpenMP directives
> +//--------------------------------
> +// \#pragam omp parallel ...
> +TOK(pragma_omp_parallel)
> +// \#pragam omp for ...
> +TOK(pragma_omp_for)
>
> Please use ANNOTATION(pragma_omp_parallel) etc. instead.

I have taken care of it. Also, for patch 1 related test case, I
replaced -v by -###.

>
> +/// PragmaOmpHandler - "\#pragma omp ...".
> +template<class T, tok::TokenKind TKind>
> +struct PragmaOmpHandler : public PragmaHandler {
> +  PragmaOmpHandler() : PragmaHandler(T::Name) {}
> +  virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
> +                            Token &OmpTok) {
> +    PP.HandlePragmaOmp(OmpTok, TKind);
> +  }
> +};
>
> Something like following would make your patch substantially shorter:
>
> struct PragmaOmpHandler : public PragmaHandler {
>   tok::TokenKind TKind;
>   PragmaOmpHandler(tok::TokenKind Kind, StringRef Name)
>     : PragmaHandler(Name), TKind(Kind) {}
>   virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
>                             Token &OmpTok) {
>     PP.HandlePragmaOmp(OmpTok, TKind);
>   }
> };

My mis-understanding of PragmaNamespace::AddPragma() function, in
particular, mis-understanding of *assertion* statement within this
function mislead me.  I was in an impression that, adding same class
more than once to same pragma namespace will result in assertion
failure. But, I had not carefully looked into it that it should be
fine as long as the associated names are different. Anyway, I have
taken care of this too.

>
>
> Please add a test that the pragmas round-trip correctly through clang -E.

This test case is getting an assertion failure. Looks like, all the
#pragmas which requires parsing, has to be registered (to
preprocessor) from libParse.a just like how #pragma unused, etc are
handled? When -E option is added, these pragmas are handled by
Preprocessor as unknown pragmas.

Waiting for more clarification from you.

--
mahesha

>
> -Eli



-- 
mahesha