Use llvm openmp on amd64 when the compiler chosen is clang

[jmd]

Test Plan

Ask for updated exp-run now that devel/openmp is based on LLVM 4.0 . Fix any fallout.

[bapt]

Fix issues on FreeBSD 10:
Defne _OPENMP for clang < 3.8
Add -lomp in LDFLAGS for clang < 3.8

[bapt]

Alwais pass -lomp to ldflags to ensure libraries using openmp gets properly linked to it

[jbeich]

USES=cmake may need a fix as FindOpenMP.cmake ignores LDFLAGS during check_c_source_compiles() which fails to find -lomp via its own detection. See audio/libsoxr for an example, then try adding

OPENMP_CMAKE_ON+=       -DOpenMP_C_FLAGS="${CPPFLAGS} ${CFLAGS} ${LIBS} ${LDFLAGS}"
OPENMP_CMAKE_ON+=       -DOpenMP_CXX_FLAGS="${CPPFLAGS} ${CXXFLAGS} ${LIBS} ${LDFLAGS}"

Mk/Uses/compiler.mk

100

"as is" this may not work after rP423014 in case some ports only respect LIBS or LDFLAGS.

105

Why do you need to add -lomp twice for clang37 and earlier?

105

Why not fix -lomp (devel/openmp) to not underlink -lm? Adding workarounds to consumers only hides the issue.

$ cc -v
FreeBSD clang version 3.9.0 (tags/RELEASE_390/final 280324) (based on LLVM 3.9.0)
Target: x86_64-unknown-freebsd12.0
Thread model: posix
InstalledDir: /usr/bin

$ cc -xc /dev/null -fopenmp -L/usr/local/lib
/usr/lib/crt1.o: In function `_start':
/usr/src/lib/csu/amd64/crt1.c:72: undefined reference to `main'
/usr/local/lib/libomp.so: undefined reference to `scalbnl'
/usr/local/lib/libomp.so: undefined reference to `fmaxl'
/usr/local/lib/libomp.so: undefined reference to `logbl'
/usr/local/lib/libomp.so: undefined reference to `scalbnf'
/usr/local/lib/libomp.so: undefined reference to `logb'
/usr/local/lib/libomp.so: undefined reference to `logbf'
/usr/local/lib/libomp.so: undefined reference to `scalbn'
cc: error: linker command failed with exit code 1 (use -v to see invocation)

[jmd]

Request from bapt via swills for me to commandeer this.

[jmd]

Add USE_GCC to math/openblas if OPENMP is configured (see D9448 for a discussion, the Fortran bits will pull in GCC's OpenMP in conflict with LLVM's).

Remove double linking against -lomp for clang37 and older as per @jbeich 's comment.

[jbeich]

Mk/Uses/compiler.mk

104

-lm should no longer be necessary after rP437204

107

lang/gcc* haven't been adapted for libcxxrt, so mixing ports built against libc++ and libstdc++ will lead to crashes. To avoid maintainers carelessly adding USES=compiler:openmp only to regress runtime on i386 (still Tier1 atm) try reusing the code from USES=compiler:gcc-c++11-lib.

math/openblas/Makefile

76

Looks fine given bug 199603 is stalled.

[jmd]

Remove -lm.

[jmd]

Thanks for your comments!

Mk/Uses/compiler.mk

100

what do you suggest here?

104

Yes, I checked that

cc -xc /dev/null -fopenmp -L/usr/local/lib
/usr/lib/crt1.o: In function `_start':
/usr/src/lib/csu/amd64/crt1.c:(.text+0x17b): undefined reference to `main'
cc: error: linker command failed with exit code 1 (use -v to see invocation)

hence: will remove.

107

Are you referring to Uses/compiler.mk?

.if ${_COMPILER_ARGS:Mgcc-c++11-lib}
USE_GCC=        yes
CHOSEN_COMPILER_TYPE=   gcc
.if ${COMPILER_FEATURES:Mlibc++}
CXXFLAGS+=      -nostdinc++ -isystem /usr/include/c++/v1
LDFLAGS+=       -L${WRKDIR}

_USES_configure+=       200:gcc-libc++-configure
gcc-libc++-configure:
        @${LN} -fs /usr/lib/libc++.so ${WRKDIR}/libstdc++.so
.endif
.endif

i.e., you suggest adding the _USES_configure+= clause?

[jbeich]

Mk/Uses/compiler.mk

100

Replace with _USES_POST+=localbase:ldflags.

105

Cosmetic: try ordering lines within blocks similar to regular ports i.e., *_DEPENDS, USES, *FLAGS.

107

Not quite, the whole block under .if ${COMPILER_FEATURES:Mlibc++} is required to use libc++ with any lang/gcc* port.

[jmd]

Address two comments by @jbeich

[jbeich]

Mk/Uses/compiler.mk

101

clang34 (as in 10.3) ignores -fopenmp which hints at how immature OpenMP support in old Clang versions compared to GCC. Maybe bring D9448 bits but limit to ${COMPILER_VERSION} < 38. Adding RUN_DEPENDS to replace LIB_DEPENDS below isn't necessary as libomp.so is ABI-compatible between devel/openmp and devel/llvm40 (even llvm39).

105

-lomp is a workaround for ${COMPILER_VERSION} < 37, later versions don't need it. If you switch to llvm40 on 10.3 this can be dropped.

[jbeich]

Mk/Uses/compiler.mk

105

LDFLAGS+=-lomp may also overlink libomp.so to binaries that weren't built with -fopenmp.

[jbeich]

Hmm, devel/openmp with base clang is so useless on 10.3.

$ uname -r
10.3-RELEASE

$ pkg install ca_root_nss
$ fetch https://computing.llnl.gov/tutorials/openMP/samples/C/omp_hello.c

$ pkg install openmp
$ cc -D_OPENMP -fopenmp -lomp -isystem/usr/local/include -L/usr/local/lib omp_hello.c
cc: warning: argument unused during compilation: '-fopenmp'
$ ./a.out
Hello World from thread = 0
Number of threads = 1

$ pkg install llvm40
$ clang40 -fopenmp omp_hello.c
$ ./a.out
Hello World from thread = 1
Hello World from thread = 2
Hello World from thread = 3
Hello World from thread = 4
Hello World from thread = 0
Number of threads = 8
Hello World from thread = 5
Hello World from thread = 6
Hello World from thread = 7

$ pkg install gcc5
$ gcc5 -Wl,-rpath,/usr/local/lib/gcc5 -fopenmp omp_hello.c
$ ./a.out
Hello World from thread = 5
Hello World from thread = 1
Hello World from thread = 4
Hello World from thread = 0
Number of threads = 8
Hello World from thread = 7
Hello World from thread = 2
Hello World from thread = 6
Hello World from thread = 3

$ pkg install gcc42 # or copy from 9.3 jail
$ gcc42 -fopenmp omp_hello.c
$ ./a.out
Hello World from thread = 7
Hello World from thread = 4
Hello World from thread = 6
Hello World from thread = 2
Hello World from thread = 0
Number of threads = 8
Hello World from thread = 3
Hello World from thread = 1
Hello World from thread = 5

In D6362#166098, @bapt wrote:

Alwais pass -lomp to ldflags to ensure libraries using openmp gets properly linked to it

Unlike bug 211134? Even then -lomp should be -fopenmp, so if it ends up in libdata/pkgconfig/*.pc files USE_GCC consumers would use the correct one.

[jmd]

Mk/Uses/compiler.mk

101

Well, I do prefer the devel/llvm40 solution (if I say so myself) but bapt prefers this solution here. I do however absolutely agree with your observation that for 10.3, devel/llvm40 may be the only useful option available. Will adjust patch accordingly (i.e., merging over what I had in D9448.

[bapt]

The point of having devel/openmp is to have less runtime dependencies for users (way smaller packages than having to install llvm40).

[bapt]

but yes I fear for 10.3 we don't have much choices :(

[jmd]

Merge in D9448 for 10.3 (i.e., old LLVM toolchain in base). For this, we will now depend on devel/llvm40. This includes a patch to devel/llvm40 that adds its library path to the search path. Given how we depend on both devel/lvm40 and devel/openmp in the 10.3 case, this patch *could* be dropped, however, it makes devel/llvm40 in its own right a more "standard" OpenMP compiler (i.e., -fopenmp implies finding -lomp), hence I'd like to keep it.

Also, side-note: once this is in tree for amd64 and the dust has settled, I am hoping to extend this solution to other architectures w/ llvm toolchain supported by LLVM's OpenMP implementation (e.g., i386 and @emaste suggested arm64 today).

[jmd]

In D6362#219969, @bapt wrote:

but yes I fear for 10.3 we don't have much choices :(

Yes, I think not pulling in too big dependencies is a good reason. My logic was that devel/llvm40 is likely to exist on a users system (Mesa depends on it e.g.) but see the latest patch update for the "hybrid" solution. :-)

[emaste]

math/openblas/Makefile

76

Worth adding a comment in the Makefile about why this port explicitly uses GCC?

[brooks]

devel/llvm40/files/clang-patch-fopenmp.diff

9

What does this change do? I can't find a reference to CMAKE_INSTALL_PREFIX in the source tree. If it is in practice adding /usr/local/llvm40/lib/ to the search path then I'm a bit skeptical it's the right thing to do. I believe that would break or at least make difficult building programs that use -fopenmp and (e.g.) llvm39 libraries.

My concern about that is why I used manual full path linkage.

[jmd]

devel/llvm40/files/clang-patch-fopenmp.diff

9

I've seen some codes do this in their make systems:

$ clang++ -fopenmp -lomp omp.cpp -o a.out
/usr/bin/ld: cannot find -lomp
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

(i.e., assume -fopenmp implies that the compiler finds libomp.so). I will drop this part of the patch.

[antoine]

This is definitely not commit-ready.