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

[Mahesha HS]

Hi Eli,

Attached is the next patch in the line. This patch
(class_pragma_omp_handler_support.patch) contains the implementation
of the class "class PragmaOmpHandler".  I also attached the test case
(openmp_syntax_test.c).  This test case is actually to test the
syntactically legal simple OpenMP constructs. However, we can *really*
test it only after submitting the next two patches - one related to
PragmaOmpHandler object construction and the another related to OpenMP
parsing.

Please, let me know, if you need to know any additional information.

If you think that some other mechanism is required to speed-up the
review process, I will really welcome it.


--
mahesha


On Sat, Oct 13, 2012 at 9:36 PM, Mahesha HS <mahesha.llvm at gmail.com> wrote:
> Hi Eli,
>
> Attached zipped file, named, "fopenmp_option_support.tar.gz" contains
> the first patch, along with relevant *test case*. This patch is to
> support the option "-fopenmp" option in Clang.
>
> Following files are changed in this patch. Please start going through
> this patch, and let me know comments. Meanwhile, I will prepare next
> patch.
>
> =========================================
> #. clang/include/clang/Driver/Options.td
> #. clang/include/clang/Basic/LangOptions.def
> #. clang/include/clang/Basic/DiagnosticLexKinds.td
>  #. clang/lib/Driver/Tools.cpp
>  #. clang/lib/Frontend/CompilerInvocation.cpp
> =========================================
>
> --
> mahesha
>
>
>
> On Sat, Oct 13, 2012 at 3:39 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Sat, Oct 13, 2012 at 2:33 AM, Mahesha HS <mahesha.llvm at gmail.com> wrote:
>>> On Sat, Oct 13, 2012 at 1:31 PM, Tobias Grosser <tobias at grosser.es> wrote:
>>>> On 10/13/2012 04:38 AM, Mahesha HS wrote:
>>>>>
>>>>> On Sat, Oct 13, 2012 at 5:14 AM, Eli Friedman <eli.friedman at gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Wed, Oct 10, 2012 at 6:15 AM, Mahesha HS <mahesha.llvm at gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Eli and Others
>>>>>>>
>>>>>>> In response to your feedback, I have taken care of all your review
>>>>>>> comments
>>>>>>> - I removed clangOMP.a
>>>>>>> and moved the implementation of "class OmpPragmaHandler" to
>>>>>>> "clangLex.a".
>>>>>>>
>>>>>>> The attached zipped file - namely - 'OpenMP_support_in_Clang.tar.gz'
>>>>>>> contains all the implemented
>>>>>>> "patches" along with *newly* added source files.
>>>>>>>
>>>>>>> Another attached text file - namely -
>>>>>>> 'OpenMP_support_in_Clang_Read_Me.txt'
>>>>>>> briefly describes the
>>>>>>> implementation.
>>>>>>>
>>>>>>> You can also refer to the initial proposal document - namely -
>>>>>>> 'OpenMP_Support_in_Clang.pdf'.
>>>>>>>
>>>>>>> Looking forward for your review comments.
>>>>>>
>>>>>>
>>>>>> Can you please split this patch somehow?  This is way too big to
>>>>>> easily review.  I'd like to see separate patches for the command-line
>>>>>> option, the pragma handler itself, parsing each pragma, each new AST
>>>>>> node, and semantic analysis for each kind of pragma, with testcases
>>>>>> for every patch.  And the more you can split them, the better to get
>>>>>> through reviews faster.
>>>>>>
>>>>>> Please include new files in patches using "svn add"; don't send new
>>>>>> files separately.
>>>>>>
>>>>>> Please look over patches before submitting to make sure you don't
>>>>>> include unnecessary changes to whitespace etc.
>>>>>
>>>>>
>>>>> Thanks for the reply. Sure, I will again send updated patches which
>>>>> take care of all the above comments.This is the first time, I am
>>>>> sending the patch to CLANG/LLVM community in particular, and to any
>>>>> open source community, in general. Also, I was in a hurry to send the
>>>>> patch based on your first reply. Will be sending the updated patches
>>>>> soon.
>>>>
>>>>
>>>> Hey Mahesh,
>>>>
>>>> thanks for working on this. As a small hint from my side, I have the feeling
>>>> there is currently no need to submit the entire OpenMP patch for review. You
>>>> could e.g. start with the patch that adds the (command
>>>> line option, but just ignores it. As a next step, you could submit a patch
>>>> that makes  >>include "omp.h" << work (without yet supporting
>>>> any definitions, ...)
>>>>
>>>> I think at the beginning it is helpful to start with smaller patches and to
>>>> get used to the review policies. This will prepare you for the
>>>> larger discussions with Elli and Co. :-)
>>>
>>> Hi Tobias,
>>>
>>> Incremental reviews (and check-ins) make sense actually.  I think, it
>>> helps both me and Eli (and others) in several ways.
>>>
>>> @ Eli,
>>>
>>> Is it fine with you too?
>>
>> Yes, please do.
>>
>> -Eli
>
>
>
> --
> mahesha



-- 
mahesha
-------------- next part --------------
A non-text attachment was scrubbed...
Name: class_pragma_omp_handler_support.patch
Type: application/octet-stream
Size: 44887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121016/08269b3b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openmp_syntax_test.c
Type: text/x-csrc
Size: 14145 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20121016/08269b3b/attachment.c>