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

Don't depend on CompilerSupportLibraries_jll from OpenBLAS_jll #49842

Merged
merged 2 commits into from
May 17, 2023

Conversation

staticfloat
Copy link
Sponsor Member

This is important because CSL_jll loads in many other libraries that we may or may not care that much about, such as libstdc++ and libgomp.

We load libstdc++ eagerly on Linux, so that will already be loaded in all cases that we care about, however on macOS we don't generally want that loaded, and this suppresses that.

libgomp is needed by BB-provided software that uses OpenMP during compilation, however it can conflict with software compiled by the Intel compilers, such as MKL. It's best to allow MKL to load its OpenMP libraries first, so delaying loading libgomp until someone actually calls using CompilerSupportLibraries_jll is the right thing to do.

In the future, we want to rework JLLs such that libraries aren't eagerly loaded at JLL __init__() time, but rather they should be JIT loaded upon first usage of the library handle itself. This would allow BB to emit much more fine-grained dependency structures, so that the distribution of a set of libraries can happen together, but the loading of said libraries would be independent.

This is important because CSL_jll loads in many other libraries that we
may or may not care that much about, such as `libstdc++` and
`libgomp`.

We load `libstdc++` eagerly on Linux, so that will already be loaded in
all cases that we care about, however on macOS we don't generally want
that loaded, and this suppresses that.

`libgomp` is needed by BB-provided software that uses OpenMP during
compilation, however it can conflict with software compiled by the Intel
compilers, such as `MKL`.  It's best to allow MKL to load its OpenMP
libraries first, so delaying loading `libgomp` until someone actually
calls `using CompilerSupportLibraries_jll` is the right thing to do.

In the future, we want to rework JLLs such that libraries aren't eagerly
loaded at JLL `__init__()` time, but rather they should be JIT loaded
upon first usage of the library handle itself.  This would allow BB to
emit much more fine-grained dependency structures, so that the
distribution of a set of libraries can happen together, but the loading
of said libraries would be independent.
@staticfloat staticfloat added the backport 1.9 Change should be backported to release-1.9 label May 16, 2023
@vtjnash
Copy link
Sponsor Member

vtjnash commented May 16, 2023

It looks like this will also partially fix #49772, since the root cause there is that loading CSL causes libstdc++ to get loaded instead of the correct library for the platform (libc++) and that leads to segfaults for users.

@staticfloat staticfloat merged commit 98b64b2 into master May 17, 2023
@staticfloat staticfloat deleted the sf/dont_eagerly_load_libgomp branch May 17, 2023 19:00
@KristofferC KristofferC mentioned this pull request May 19, 2023
51 tasks
KristofferC pushed a commit that referenced this pull request May 19, 2023
Don't depend on `CompilerSupportLibraries_jll` from `OpenBLAS_jll`

(cherry picked from commit 98b64b2)
KristofferC pushed a commit that referenced this pull request May 27, 2023
Don't depend on `CompilerSupportLibraries_jll` from `OpenBLAS_jll`

(cherry picked from commit 98b64b2)
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label May 28, 2023
kpamnany pushed a commit that referenced this pull request Jun 21, 2023
Don't depend on `CompilerSupportLibraries_jll` from `OpenBLAS_jll`

(cherry picked from commit 98b64b2)
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

Successfully merging this pull request may close these issues.

5 participants