Skip to content

fix $BLAS_LIB_MT for OpenBLAS, ensure -lpthread is included #3584

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.

Sign up for GitHub

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

Merged
akesandgren merged 2 commits into from
Feb 22, 2021
Merged

fix $BLAS_LIB_MT for OpenBLAS, ensure -lpthread is included #3584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f84e06
fix BLAS_LIB_MT for OpenBLAS
Feb 20, 2021
f12e4d9
fix test
Feb 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions easybuild/toolchains/linalg/openblas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class OpenBLAS(LinAlg):
"""
BLAS_MODULE_NAME = ['OpenBLAS']
BLAS_LIB = ['openblas']
BLAS_LIB_MT = ['openblas']
Copy link
Contributor

@akesandgren akesandgren Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually add -lpthread. And the libopenblas.{a,so} we build actually depends on -lpthread even for the non-MT version since there is only one lib and it's built with OpenMP enabled.

Copy link
Contributor Author

@smoors smoors Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually add -lpthread.

yes it does. did you actually check it? it’s because BLAS_LIB_MT was not defined that -lpthread was not added to LIBBLAS_MT. defining it here fixes it (for me at least).

And the libopenblas.{a,so} we build actually depends on -lpthread even for the non-MT version since there is only one lib and it’s built with OpenMP enabled.

that’s true for the OpenBLAS versions in the official EB repo but people might still want to build it without OpenMP (for example on macOS), and it does not hurt to add it, right?

anyway I don't mind closing this if you think it's not worth it.

Copy link
Contributor

@akesandgren akesandgren Feb 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I didn't check the actual output (got sidetracked as usual), was making the comment so you would correct me if needed :-)

BLAS_FAMILY = TC_CONSTANT_OPENBLAS

LAPACK_MODULE_NAME = ['OpenBLAS']
Expand Down
2 changes: 1 addition & 1 deletion test/framework/toolchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1449,7 +1449,7 @@ def test_old_new_iccifort(self):
libscalack_intel4 = "-lmkl_scalapack_lp64 -lmkl_blacs_intelmpi_lp64 -lmkl_intel_lp64 -lmkl_sequential "
libscalack_intel4 += "-lmkl_core"

libblas_mt_fosscuda = "-lopenblas -lgfortran"
libblas_mt_fosscuda = "-lopenblas -lgfortran -lpthread"
libscalack_fosscuda = "-lscalapack -lopenblas -lgfortran"
libfft_mt_fosscuda = "-lfftw3_omp -lfftw3 -lpthread"

Expand Down