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

[Mahesha HS]

On Sat, Oct 20, 2012 at 4:38 PM, Mahesha HS <mahesha.llvm at gmail.com> wrote:
> 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.

I have proceeded further, and followed exactly the similar approach as
implemented in case of other pragmas like  #pragma unused, etc.
Attached is the revised patch (omp_pragma_registration_revised.patch).

In case of any further comments, please let me know. Otherwise, we
need to check-in both patch 1 (fopenmp_option_support.patch) and patch
2 (this one).

--
mahesha

>
> --
> mahesha
>
>>
>> -Eli
>
>
>
> --
> mahesha



-- 
mahesha
-------------- next part --------------
A non-text attachment was scrubbed...
Name: omp_pragma_registration_revised.patch
Type: application/octet-stream
Size: 14201 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121021/995000b1/attachment.obj>