Bug 1451138 - Review Request: libomp - default OpenMP runtime used by clang
Summary: Review Request: libomp - default OpenMP runtime used by clang
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Igor Gnatenko
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1409871
TreeView+ depends on / blocked
 
Reported: 2017-05-15 22:35 UTC by Tom Stellard
Modified: 2018-01-18 16:13 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-01-18 16:13:42 UTC
Type: ---
Embargoed:
ignatenko: fedora-review+


Attachments (Terms of Use)

Description Tom Stellard 2017-05-15 22:35:42 UTC
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/tstellar/llvm/libomp.git/tree/libomp.spec?h=f26
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/tstellar/llvm/fedora-26-x86_64/00552163-libomp/libomp-4.0.0-1.fc26.src.rpm
Description: libomp is the default OpenMP runtime used by clang.
Fedora Account System Username:tstellar

Comment 1 Satish Balay 2017-05-16 03:04:16 UTC
[this is not a review - but feedback]

balay@asterix /home/balay/rpmbuild/RPMS/x86_64
$ rpm -qlp libomp-4.0.0-1.fc26.x86_64.rpm
/usr/lib64/libomp.so
balay@asterix /home/balay/rpmbuild/RPMS/x86_64
$ rpm -qlp libomp-devel-4.0.0-1.fc26.x86_64.rpm
/usr/include/omp.h
balay@asterix /home/balay/rpmbuild/RPMS/x86_64
$ 

The location of these files does not look correct. I would think /usr/include/omp.h would conflict with omp.h from gomp

Looking at gomp organization I see:

balay@asterix /home/balay
$ locate libgomp.so
/usr/lib/gcc/x86_64-redhat-linux/7/libgomp.so
/usr/lib/gcc/x86_64-redhat-linux/7/32/libgomp.so
/usr/lib64/libgomp.so.1
/usr/lib64/libgomp.so.1.0.0
balay@asterix /home/balay
$ locate /omp.h
/usr/lib/gcc/x86_64-redhat-linux/7/include/omp.h
balay@asterix /home/balay
$ 

So presumably clang omp include/libraries should be installed in clang include/library locations?

Here is how llvm binary distribution is packaged


balay@asterix /home/balay/soft
$ find clang+llvm-4.0.0-x86_64-linux-gnu-debian8 -type f -name omp.h
clang+llvm-4.0.0-x86_64-linux-gnu-debian8/lib/clang/4.0.0/include/omp.h
balay@asterix /home/balay/soft
$ find clang+llvm-4.0.0-x86_64-linux-gnu-debian8 -type f -name "libomp*"
clang+llvm-4.0.0-x86_64-linux-gnu-debian8/lib/libomp.so
balay@asterix /home/balay/soft
$ 


And perhaps the package should have a llvm prefix - say llvm-libomp?

Comment 2 Tom Stellard 2017-05-16 22:18:59 UTC
Thanks for the feedback, I moved the header file to the same location as the pre-build binary.  I considered adding the clang- prefix to the package, but I decided to leave it off to match what libgomp does.

Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/tstellar/llvm-4.0/libomp.git/tree/libomp.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/tstellar/llvm-4.0/fedora-rawhide-x86_64/00552673-libomp/libomp-4.0.0-1.fc27.src.rpm

Comment 3 Satish Balay 2017-05-17 14:50:02 UTC
Thanks!

One more issue:

I have:

clang-3.9.1-2.fc26.x86_64

However I'm able to install libomp-4.0.0 with it. [so the include dirs between the 2 packages don't match]

/usr/lib64/clang/3.9.1/include/ vs /usr/lib64/clang/4.0.0/include/omp.h

[I can grab and install llvm-4.0 and clang-4.0 packages manually but] perhaps there should be some rpm dependency info that should prevent these versions from being out-of-sync?

Also should libomp package be built with clang - and not gcc?

Comment 4 Tom Stellard 2017-05-24 10:00:28 UTC
(In reply to Satish Balay from comment #3)
> Thanks!
> 
> One more issue:
> 
> I have:
> 
> clang-3.9.1-2.fc26.x86_64
> 
> However I'm able to install libomp-4.0.0 with it. [so the include dirs
> between the 2 packages don't match]
> 
> /usr/lib64/clang/3.9.1/include/ vs /usr/lib64/clang/4.0.0/include/omp.h
> 
> [I can grab and install llvm-4.0 and clang-4.0 packages manually but]
> perhaps there should be some rpm dependency info that should prevent these
> versions from being out-of-sync?
> 

I'm not sure what to do about the version mismatch.  clang-4.0 will depend on libomp-4.0, so when you  install clang-4.0 you will get the correct libomp.  I guess if you want to mix and match versions like this, you will need to manually specify the libomp header/library paths when building clang.

> Also should libomp package be built with clang - and not gcc?

I think it is preferred that packages are built with the system compiler unless there is a compelling reason no to you.  What would be the reason to build with clang?  Also this would create a circular dependency: clang -> libomp -> clang.

Comment 5 Satish Balay 2017-05-24 13:12:42 UTC
(In reply to Tom Stellard from comment #4)
> (In reply to Satish Balay from comment #3)

> clang-4.0 will depend
> on libomp-4.0, so when you  install clang-4.0 you will get the correct
> libomp.

Ok. This is good.

> I guess if you want to mix and match versions like this, you will
> need to manually specify the libomp header/library paths when building clang.

I don't want to mix versions - my concern was - rpm/dnf should prevent mixing of versions. [if clang depends on libomp - then presumably rpm/dnf will prevent mixing of versions].

> 
> > Also should libomp package be built with clang - and not gcc?
> 
> I think it is preferred that packages are built with the system compiler
> unless there is a compelling reason no to you.  What would be the reason to
> build with clang?  Also this would create a circular dependency: clang ->
> libomp -> clang.

ok.

Comment 6 Igor Gnatenko 2017-11-16 18:01:56 UTC
> cd _build
> %make_install
%make_install -C _build

* Add Requires for proper version of clang-devel to -devel subpackage so you don't have unowned dirs.
* Requires should have %{?_isa}

APPROVED.

Comment 7 Gwyn Ciesla 2017-11-20 16:47:42 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libomp


Note You need to log in before you can comment on or make changes to this bug.