[PATCH] [OPENMP] Added option -fopenmp=libiomp5

[Alp Toker]

On 28/02/2014 06:13, Alexey Bataev wrote:
>> Fair enough, but this was non-obvious to me until I looked at 
>> lib/CodeGen in the GitHub project and saw the runtime support focused 
>> on kmp. It's worth mentioning that in the patch submission because 
>> reviewers aren't always going to have a chance to look at the 
>> external project.
> Alp, yes, you're right. I should mention the fact that all the changes 
> are for native libiomp5 interface. Though I've sent a patch already 
> with codegen which targets libiomp5, though it seems nobody looked at 
> it (http://llvm-reviews.chandlerc.com/D2883).
>
>> In that case the cc1 frontend option should probably be something 
>> like -fopenmp-runtime=(kmp/iomp5/gomp) instead of passing through the 
>> driver option verbatim.
> I tried to reduce number of options and decided to use the same option 
> for frontend as for driver. Besides, -fopenmp=libiomp5 does exactly 
> what you says - commands frontend to generate IR which uses kmp* 
> runtime functions (native libiomp5 interface).

OK, I got the impression it may be preferable to add a -fopenmp-runtime= 
option while keeping the -fopenmp and /openmp driver flags identical to 
gcc/MSVC but it's your call.

Alp.

>
> Best regards,
> Alexey Bataev
> =============
> Software Engineer
> Intel Compiler Team
> Intel Corp.
>
> 28 Февраль 2014 г. 9:58:02, Alp Toker писал:
>>
>> On 28/02/2014 05:33, Alexey Bataev wrote:
>>> Alp, generated IR cannot be identical, because interfaces of gomp and
>>> iomp5 are different. My patch with initial codegen support for OpenMP
>>> constructs targets iomp5 interface.
>>
>> I see, so this will use the kmp interface rather than the gomp
>> compatibility entry points that both gomp and iomp5 support?
>>
>> Fair enough, but this was non-obvious to me until I looked at
>> lib/CodeGen in the GitHub project and saw the runtime support focused
>> on kmp. It's worth mentioning that in the patch submission because
>> reviewers aren't always going to have a chance to look at the external
>> project.
>>
>>
>>> Besides, it is incomplete and I don't want someone run into the
>>> troubles with incomplete codegen for OpenMP constructs. When it will
>>> be completed we'll remove -fopenmp=libiomp5 option or modify it 
>>> somehow.
>>
>> I see what you're trying to do now.
>>
>> In that case the cc1 frontend option should probably be something like
>> -fopenmp-runtime=(kmp/iomp5/gomp) instead of passing through the
>> driver option verbatim.
>>
>> Alp.
>>
>>
>>>
>>> Best regards,
>>> Alexey Bataev
>>> =============
>>> Software Engineer
>>> Intel Compiler Team
>>> Intel Corp.
>>>
>>> 27 Февраль 2014 г. 16:32:01, Alp Toker писал:
>>>>
>>>> On 27/02/2014 11:58, Alexey Bataev wrote:
>>>>>
>>>>> ================
>>>>> Comment at: lib/Driver/Tools.cpp:5237-5240
>>>>> @@ -5236,4 +5236,6 @@
>>>>> -  if (Args.hasArg(options::OPT_fopenmp))
>>>>> +  if (Args.hasArg(options::OPT_fopenmp)) {
>>>>>       // This is more complicated in gcc...
>>>>>       CmdArgs.push_back("-lgomp");
>>>>> +  } else if (const Arg *A =
>>>>> Args.getLastArg(options::OPT_fopenmp_EQ)) {
>>>>> +    if (StringRef(A->getValue()) == "libiomp5")
>>>>> ----------------
>>>>> Dmitri Gribenko wrote:
>>>>>> What happens if the user passes -fopenmp -fopenmp=libiomp5?
>>>>> Then only -fopenmp will work and the warning for -fopenmp=libiomp5
>>>>> will be emitted. I thought that we have to be conservative with
>>>>> -fopenmp option.
>>>>>
>>>>> ================
>>>>> Comment at: test/OpenMP/parallel_ast_print.cpp:3
>>>>> @@ -2,3 +2,3 @@
>>>>>   // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -emit-pch -o %t %s
>>>>> -// RUN: %clang_cc1 -fopenmp -std=c++11 -include-pch %t -fsyntax-only
>>>>> -verify %s -ast-print | FileCheck %s
>>>>> +// RUN: %clang_cc1 -fopenmp=libiomp5 -std=c++11 -include-pch %t
>>>>> -fsyntax-only -verify %s -ast-print | FileCheck %s
>>>>>   // expected-no-diagnostics
>>>>> ----------------
>>>>> Dmitri Gribenko wrote:
>>>>>> If this flag only changes the linker option, then there is no need
>>>>>> to update AST tests with it.
>>>>> No, it also turns on OpenMP support in front end. You can consider
>>>>> -fopenmp=libiomp5 option as an extension for -fopenmp option. My idea
>>>>> was not only link libiomp5, but also generate the IR for libiomp5
>>>>> runtime only if -fopenmp=libiomp5 is specified. If -fopenmp is
>>>>> specified, the IR won't be generated (for compatibility).
>>>>
>>>> I'd expect the IR generated for the two runtimes to be identical given
>>>> that the target is a stable ABI.
>>>>
>>>> How about just passing -lomp from the driver to the linker and leaving
>>>> it up to distributors to symlink the implementation (GNU or Intel) to
>>>> the generic name, the same way -pthread works?
>>>>
>>>> (The -stdlib= optional argument was necessary only due to the need to
>>>> modify header search paths and select the correct ABI between the two
>>>> C++ standard libraries simultaneously. Neither of these needs ought to
>>>> exist with OpenMP runtimes.)
>>>>
>>>> Alp.
>>>>
>>>>>
>>>>>
>>>>> http://llvm-reviews.chandlerc.com/D2841
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>

-- 
http://www.nuanti.com
the browser experts