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

[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