Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using Apple clang with USE_OPENMP=1 #5156

Open
dimpase opened this issue Feb 27, 2025 · 10 comments
Open

using Apple clang with USE_OPENMP=1 #5156

dimpase opened this issue Feb 27, 2025 · 10 comments

Comments

@dimpase
Copy link

dimpase commented Feb 27, 2025

Currently a macOS toolchain with Apple's clang 16+ and gfortran fails with make build if USE_OPENMP=1 is set.

The 1st reason for this is that clang rejects -fopenmp, for it needs -Xclang -fopenmp option, as written by R Project in https://mac.r-project.org/openmp/

This was discovered while I bumped into a Homebrew problem, described in Homebrew/homebrew-core#209091

Is there any interest in looking into it further? In principle, R Project might have a point here.

@martin-frbg
Copy link
Collaborator

I'm not entirely sure - according to the R link, one would need to obtain a third-party libomp for linking to work with the Apple version of LLVM, so it is far from a standard environment (and apparently one actively discouraged by Apple for whatever reason). At the very least, I'd need to check if a non-Apple clang would cope with the extra -Xclang

And as I understand it, your initial problem arises from trying to use the pkgconfig file from&for a gcc build to obtain your CFLAGS setting - I think the build would probably work correctly if you would run it as make CFLAGS="-Xclang -fopenmp" ... ?

@dimpase
Copy link
Author

dimpase commented Feb 27, 2025

Apple wants to push users away from OSS into using their own libraries and tools. Just as the botch Python they ship, etc.

I have no idea how much more performance one gets on macOS from openblas by enabling openmp.

Current Homebrew openblas is built with GNU OpenMP (libgomp). I have no idea about it being compatible with clang's idea of OpenMP. If it is compatible, one can think of using GNU OpenMP with clang, no need for R Project's implementation of OpenMP on macOS.

@rgommers
Copy link
Contributor

I don't think a change needs to be made here. As described in the linked R doc, the reason it doesn't work is because Apple has disabled OpenMP on purpose and doesn't ship libomp.dylib. So out of the box nothing will work here, and making the compile succeed without libomp.dylib present is a footgun. If a user or packager goes through the trouble of providing their own libomp build, then they can also be made responsible for passing -Xclang as part of that hacking around.

Conda-forge is shipping OpenBLAS built with OPENMP=1 without problems by the way (with their own Clang builds): https://github.com/conda-forge/openblas-feedstock

@dimpase
Copy link
Author

dimpase commented Feb 27, 2025

Homebrew does ship libomp, so it might make perfect sense to try using it with openblas Homebrew.

I just don't know how flexible openblas cmake build is to accommodate such a setup.

@dimpase
Copy link
Author

dimpase commented Feb 28, 2025

OK, I am making progress with building openblas with OpenMP support from LLVM's libomp, as installed by Homebrew,
using cmake build. Good news that it knows the correct flags -Xclang -fopenmp.

There are couple of issues/bugs I encountered in the build, see below. Overcoming them, I ran ctest on the result, all 116 tests passed.
Here is the linkage of the resulting dylib:

% otool -L lib/libopenblas.dylib 
lib/libopenblas.dylib:
        @rpath/libopenblas.0.dylib (compatibility version 0.0.0, current version 0.3.0)
        /opt/homebrew/opt/libomp/lib/libomp.dylib (compatibility version 5.0.0, current version 5.0.0)
        /opt/homebrew/opt/gcc/lib/gcc/current/libgfortran.5.dylib (compatibility version 6.0.0, current version 6.0.0)
        /opt/homebrew/opt/gcc/lib/gcc/current/libgomp.1.dylib (compatibility version 2.0.0, current version 2.0.0)
        /opt/homebrew/opt/gcc/lib/gcc/current/libquadmath.0.dylib (compatibility version 1.0.0, current version 1.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)

I called cmake as follows, in clean build/ directory

cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Release  -DBUILD_SHARED_LIBS=ON -DUSE_OPENMP=1
  1. cmake build does not try to get the information where to look for omp.h from FindOpenMP. I made a crude patch to fix it for the time being, but there must be a proper cmake way for this.
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -105,6 +105,7 @@ message(WARNING "CMake support is experimental. It does not yet support all buil
 
 if (USE_OPENMP)
   find_package(OpenMP REQUIRED)
+  include_directories("/opt/homebrew/include")
 endif ()
 
 include("${PROJECT_SOURCE_DIR}/cmake/utils.cmake")
  1. Apple's clang appears not to look at the current directory for include files. In particular it does not find lapacke_mangling.h which is generated at some point. I worked it around by manually copying it into one of the directories in -I options, but, again, there must be a proper cmake way.

Would you be interested in a PR fixing 1 and/or 2?

@dimpase
Copy link
Author

dimpase commented Feb 28, 2025

PS. There is also one more quirk to be fixed: openblas.pc is not make quite correct, I got

libdir=/usr/local/lib
libnameprefix=
libnamesuffix=
libsuffix=
includedir=/usr/local/include/openblas

openblas_config=USE_64BITINT= NO_CBLAS= NO_LAPACK= NO_LAPACKE= DYNAMIC_ARCH=OFF DYNAMIC_OLDER=OFF NO_AFFINITY=1 USE_OPENMP=1 VORTEX MAX_THREADS=10 
Name: OpenBLAS
Description: OpenBLAS is an optimized BLAS library based on GotoBLAS2 1.13 BSD version
Version: 0.3.29
URL: https://github.com/OpenMathLib/OpenBLAS
Libs: -L${libdir} -l${libnameprefix}openblas${libnamesuffix}${libsuffix} 
Cflags: -I${includedir} -Xclang -fopenmp 

Here libdir and includedir have totally wrong values

Although cflags are fine :-)

EDIT - or probably there is nothing to fix here, it's just default values, which can be changed by passing appropriate parameters to cmake.

@martin-frbg
Copy link
Collaborator

I wonder what is the point of getting only libomp from homebrew while continuing to use the Apple-flavored clang, when I guess you could get the entire toolchain from homebrew and solve the include path issue that way ?

@dimpase
Copy link
Author

dimpase commented Feb 28, 2025

include_directory(".") solves 2, and fixing 1 property is useful for a not so unusual case when the location of libomp one wants to use is non-standard.

Also, Homebrew does not mandate a full Homebrew toolchain.

@rgommers
Copy link
Contributor

The output of otool -L lib/libopenblas.dylib shows that both libgomp and libomp are linked. That won't work at all. @dimpase also looking at the conversation in Homebrew/homebrew-core#209091, I think your ask here is based on a misunderstanding of how things should work. You never ever want to link two OpenMP runtimes, and ideally never have more than one in your whole software stack.

Homebrew builds with GCC, so it will be pulling in libgomp. Hence that's what should be used.

@dimpase
Copy link
Author

dimpase commented Feb 28, 2025

Thus we see that it's possible to trick openblas' cmake build into producing an incorrect libopenblas, isn't it?
(And then tests don't show this, probably because the C side of openmp is not tested.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants