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

[Mahesha HS]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fopenmp_option_support.tar.gz
Type: application/x-gzip
Size: 1550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20121013/9cbf59f1/attachment.bin>