lapack
lapack copied to clipboard
cmake build: more modularity, linker fix, pkg-config fixup
This set of changes stems from the packaging of netlib code in pkgsrc, where we decided to provide blas, lapack, cblas, and lapacke as separate packages and hence need clean support in the build system for this and also for combining the wrappers lapacke and cblas with any prescribed optimized blas or lapack library.
- LAPACK option to be able to just build BLAS
- consistent logic for USE_OPTIMIZED_BLAS and USE_OPTIMIZED_LAPACK (including clear failure instead of silent fallback if BLAS_LIBARIES or LAPACK_LIBRARIES do not work)
- Fix linking of CBLAS on NetBSD with pkgsrc, use Fortran and not C for linking (same as LAPACKE already).
- Allow building of shared and static libs at the same time.
- Trying to fix up pkg-config files for all cases of internal or external libs
The last one is not in really proper shape yet. The idea is that the .pc files contain
Libs.private= -L/fooprefix -lfoo
if foo is your optimized BLAS/LAPACK, not assuming presence of a .pc file for that itself. This works for user-supplied BLAS_LIBRARIES so far, but inserts the full path to the .so file from its search … which does not fit the case of static linking. And well, yes, you do not know if both static and shared libs are present. So maybe it's a complex problem. My use case so far is fine as I explicitly provide BLAS_LIBRARIES and actually this is what I try to get into other upstream packages that need BLAS or LAPACK: Just use BLAS_LIBS or LAPACK_LIBS as provided by the user (the packager).
Would be nice if (most of) this can be included in the official LAPACK CMake build so that we do not have to carry and update the patches all the time.
Description
Checklist
- [ ] The documententation has been updated
- [ ] If the PR solves a specific issue, it is set to be closed on merge.
This build failure on MacOS seems to be bogus … I did not touch the Makefile build, anyway. Btw., if you are not intending to replace the Makefile build with CMake, that would be good to know. I don't insist on using CMake, but it seemed the more maintained route to getting the shared libs.
Codecov Report
Merging #556 (43eb7a2) into master (0448336) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #556 +/- ##
=======================================
Coverage 82.37% 82.37%
=======================================
Files 1894 1894
Lines 190679 190679
=======================================
Hits 157065 157065
Misses 33614 33614
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 0448336...43eb7a2. Read the comment docs.
This build failure on MacOS seems to be bogus … I did not touch the Makefile build, anyway. Btw., if you are not intending to replace the Makefile build with CMake, that would be good to know. I don't insist on using CMake, but it seemed the more maintained route to getting the shared libs.
Hi! I just re-ran the problematic MacOS test. It is all good, the problem was in Travis CI. :)
About the build systems... Recently, the LAPACK's community on Github had a quite nice discussion about build systems (#488). We decided to keep both Makefile and CMake build systems active for now. Ideally, we should maintain both systems with equivalent basic compiling options. Moreover, both should be continuously tested (using Travis CI for instance). However, CMake is more versatile, and we already have some options that are only supported by the CMake build system. I think that is the case for dynamic libraries too.
This being said, I don't see a problem in providing new features only for the CMake build systems. Now, if it is a typo or bug fix in the build systems, we shall also correct it on the Makefile side
Oh. The -frecursive thing creeps me out. LAPACK is not thread-safe without it? Or are you just not sure? It is a significant difference in build systems if one has the flag and the other not. Why not? Of course we use OpenBLAS for serious work, but multithreaded applications are the norm and the reference should be correct … The discussion at the other issues has me confused about the current state. Is it just some tests? The tradeoff with -frecursive is just that you need big stack (ulimit -s unlimited is the way to go with any Fortran code …)?
I wonder if I introduced multitreading issues in switching from Makefile to CMake; should add -frecursive to the build, then. I don't get why CMake couldn't do this, though (after testing compiler support).
Pkgsrc had patches to get the shared libs out of the Makefiles before, like just about any distro, none of which ever contributed them here? And I actually detest CMake, but it seemed like the way forward here. Should we rather try to revive those patches? I guess I could add options to the Makefiles, too, and switch back. Curious.
if(BUILD_SHARED_LIBS AND BUILD_STATIC_LIBS)
There is no BUILD_STATIC_LIBS
option in CMake. There is no such option in the latest CMake Variable documentation and the CMake developers explicitly stated this in CMake issue 18609 Please document and support BUILD_STATIC_LIBS two years ago.
If you need static and shared libraries, I strongly suggest to run CMake twice because the position-independent code in a shared library differs significantly from code in static libraries and it cannot be optimized as well. For example, the compiler cannot perform inlining because it would break symbol interposition (i.e., the ability to replace library functions by using LD_PRELOAD
).
Here is a simple example demo highlighting the performance impact: Effects of Position Independent Code
There is no
BUILD_STATIC_LIBS
option in CMake.
Well, I just added the option to this particular build, not CMake itself. So it is present just for the conditional you presented. This is also what your reference states. If you want to build both libs, you need to define another target. This is what the patch does. The name appears in other projects, too, it's just that it used by informal convention, not given by CMake itself.
If you need static and shared libraries, I strongly suggest to run CMake twice
This is of course an alternative, which needs more scripting around the build for the packager, as you cannot do a make install
, but have to either to it twice, overwriting stuff, or pick the missing files from the second build.
because the position-independent code
This can be discussed. There used to be a world where you just used PIC for shared libs and tried to get away without it, even including stuff like -fomit-frame-pointer
. But the concerned performance impact is muchly reduced with the x86-64 instruction set. I am not sure how applicable your point about inlining is. Such optimization between compilation units is something rather recent in open source compilers (I presume -flto
would do the trick), but by no means standard.
Regarding LAPACK, I presume that if your application would benefit from inlining code from the library, your problem size is too small to actually benefit from efficient linear algebra algorithms. And then there is the point that for actual time-critical computations, you use a platform-optimized BLAS/LAPACK.
Then, there is the general question whether static libraries should be built with PIC or not, see e.g.: https://github.com/conda-forge/boost-feedstock/issues/11 . To combine them with code that uses PIC, you need to have them built that way or you get those friendly relocation errors. My impression was that, on 64 bit x86 at least, you tend to always use PIC, at least when packaging for general use.
One might argue that one could drop the static lib at all in that case … and that it's justification is that it is bare-bones also without position independence. It's no easy topic. My addition of BUILD_STATIC_LIBS
does nothing to prevent you from building the static libs separately, anyway.
Regarding LAPACK, I presume that if your application would benefit from inlining code from the library, your problem size is too small to actually benefit from efficient linear algebra algorithms.
(Emphasis mine)
Position-independent code inhibits inlining within the library and LAPACK might benefit from inlining certain functions a lot, e.g., xROT.
Then, there is the general question whether static libraries should be built with PIC or not, see e.g.: conda-forge/boost-feedstock#11 . To combine them with code that uses PIC, you need to have them built that way or you get those friendly relocation errors. My impression was that, on 64 bit x86 at least, you tend to always use PIC, at least when packaging for general use.
One might argue that one could drop the static lib at all in that case … and that it's justification is that it is bare-bones also without position independence. It's no easy topic. My addition of BUILD_STATIC_LIBS does nothing to prevent you from building the static libs separately, anyway.
If you build a static library with position-independent code (PIC), then you have none of the benefits of shared libraries (code re-use and easy updates) and you gave up on advantages of object code (better optimization). The code optimization can be re-enabled by building the static library PIC with hidden symbols by passing the compiler option -fvisibility=hidden
; this option can be given to a CMake build via environment variables (i.e., without modifying the build setup). This makes sense, e.g., when one has to create a portable shared library that depends only on the standard C library implementation. In any other case, I fail to come up with a good reason why one might want to have PIC static libraries unless one tries to solve build problems in the wrong place (Conda libraries being found instead of host system libraries comes to my mind).
PIC in a static library is simply bad practice. Thus, the user should take care of this problem.
Another reason for static libraries is to build a binary that does not depend on its build environment and can be transferred and run elsewhere, or perhaps also to have minimal overall program binary size as unused symbols are not included. Better optimization could also be a consideration. This used to be more common in the past in our use case (HPC cluster). Nowadays the number of cases where you have static versions of all needed libraries is relatively small and people like to throw around huge containers instead. I am primarily considering static libs in LAPACK out of tradition. Especially in binary distros you have the mechanism of shared BLAS/LAPACK to be able to switch implementations at runtime when you care about performance.
Your point about inlining inside the library is interesting. It sounds like something stupid that is just a side-effect. I'll educate me on which functions this affects in what manner and test with differing compilers we have installed.
But there is the point that a cursory check of static libs on my Ubuntu desktop system suggest that it is common practice to build also the static libs with PIC. But then … let's see what actually happens! Using this test (correct me if I'm wrong):
do
printf "%s: " "$f"
if readelf --relocs "$f" | awk '$3~/^R_/ && $5!~/^\.debug/{print $3}' \
|grep -q -w -e R_X86_64_32 -e R_X86_64_32S; then
echo "no"
else
echo "yes"
fi
done
- On my Ubuntu 20.04 system: The libblas.a from pkgsrc using Ubuntu's gcc says yes.
- On CentOS with my self-built gcc-8.4.0 toolchain: libblas.a from pkgsrc says no.
So … it seems to me that CMake isn't enforcing PIC here, but the toolchain of Ubuntu is?
Checking what the CMake build actually does …
make VERBOSE=1
gfortran -O -march=native -O2 -fno-math-errno -ftree-vectorize
-frecursive -DNDEBUG -O2 -ffixed-form -c […]/zher2k.f -o CMakeFiles/blas_static.dir/zher2k.f.o
[…]
ar qc ../../lib/libblas.a CMakeFiles/blas_static.dir/isamax.f.o […]
[…]
gfortran -Dblas_EXPORTS […] -fPIC -ffixed-form -c […]/zher2k.f -o CMakeFiles/blas.dir/zher2k.f.o
[…]
gfortran -fPIC […] -shared -Wl,-soname,libblas.so.3 -o ../../lib/libblas.so.3.9.1 […]
CMake prepares different directories and builds the objects separately for static and shared builds, without PIC in the former case. So what are we arguing about? ;-)
That I get PICy libraries on Ubuntu is apparently a decision they took with their toolchain to enable ASLR for all binaries. Checked the build, no PIC in compiler flags, but PICy objects.
Your point about inlining inside the library is interesting. It sounds like something stupid that is just a side-effect. I'll educate me on which functions this affects in what manner and test with differing compilers we have installed.
The compiler is applying its inlining heuristics wherever it can.
But there is the point that a cursory check of static libs on my Ubuntu desktop system suggest that it is common practice to build also the static libs with PIC. But then … let's see what actually happens! Using this test (correct me if I'm wrong):
What you are seeing is code for position-independent executables (PIE for short), a feature aimed at enhancing security. PIE is enabled by default on several modern distributions including Ubuntu 20.04 but neither on CentOS 7 nor on CentOS 8 (execute gcc -v 2>&1 | egrep -o -e '[^ ]*-pie'
; if --enable-default-pie
is shown, then GCC is configured to build PIE code by default). In comparison to PIC, PIE does not suppress optimizations because the compiler knows that no symbol interposition will take place.
[snip]
CMake prepares different directories and builds the objects separately for static and shared builds, without PIC in the former case. So what are we arguing about? ;-)
In the build output above, there is no PIC option for the static library because the static library is a separate target. If the user builds a static library with PIC code then CMake will build the exact same object code twice because of the separate targets. In this case, there is no gain from the new CMake code. It is possible to re-use the object files with an intermediate object library target but then CMake stops being able to infer whether PIC should be enabled or not if both static and shared library should be built. There is also no gain if the user wants to pass different compiler flags for the static library, for example, to enable link-time optimization or PIE code. The user has to build LAPACK twice again.
I fail to see the value-add of a static library target in addition to the generic library target. In my opinion, the added code duplication does not lead to noticeable benefits for the user.
I don't quite get where we differ on the diagnostics of PIE or PIC in the static libs, but let's put that aside, as it is established that the static libs resulting from my change are built correctly from your perspective, right?
(Edit: I think I get it now. Thanks for pushing to me clarify my mental PIE PICture. I indeed had those terms not clearly defined. But this is irrelevant to this PR as no static lib is built with PIC here.)
So all my change does is to enable a single round of
cmake
make
make install
to get shared and static libraries installed. It is not something unheard of to build both. I just checked a autotools/libtool-based project I maintain …
./configure --enable-shared --enable-static
make
does build both shared and static libs, both from separate sets of object files built with and without PIC (in src/.libs/foo.o and src/foo.o, respectively). It's not exactly strange to expect that to work. I admit that I am not sure if it's well-defined that the executables of the project are linked with the shared libs, I must admit. But this does not matter for LAPACK.
I am not willing to die on that hill. I can work around the lack of a combined build by building twice and merging the resulting files.
So, what about the other changes?
[...] I just checked a autotools/libtool-based project I maintain …
./configure --enable-shared --enable-static make
does build both shared and static libs, both from separate sets of object files built with and without PIC (in src/.libs/foo.o and src/foo.o, respectively). It's not exactly strange to expect that to work. I admit that I am not sure if it's well-defined that the executables of the project are linked with the shared libs, I must admit. But this does not matter for LAPACK.
Speaking about linking executables, if both static and shared libraries are built, then the static library is never tested (in the current setup).
So, what about the other changes?
I will look at them.
I am further scratching my head about properly packaging all the reference lapack. I think I'll open a separate issue about the ILP64 builds and headers.
But it would be good to know if my changes here have a chance or not before I create the next PR or push more to the current one.
I still like the simultaneous build of shared and static libs, which does properly build with and without PIC, for packaging. But I could work around it if you really oppose the lines I added for that.
I put some related questions on #461 . Installing separate headers for 32 bit and 64 bit index variant, just like OpenBLAS does, eases packaging, where I build those variants separately. The separation also makes sense since not all platforms support 64 bit integers. The case of a single header for both variants creates a conflict between the packages, or demands a third package just for the headers, in addition to having to specify weird preprocessor flags.
Does the project have an opinion on mangling the headers to default to 64 bit indices, like OpenBLAS, or instead ensuring that the .pc and .cmake files contain the proper flags (which they do not right now)?
Static libraries do not contain lists with their dependencies but pkg-config comes in handy here. Example:
$ pkg-config --libs lapack
-L/usr/lib/x86_64-linux-gnu/openblas-pthread/ -lopenblas
$ pkg-config --libs --static openblas
-L/usr/lib/x86_64-linux-gnu/openblas-pthread/ -lopenblas -lm -lpthread -lgfortran -lm -lpthread -lgfortran
If a software package A depends on another software package B, the Requires
keyfield makes pkg-config recursively read pkg-config files. pkg-config makes one assumption here: If a user wants to link statically, then all dependencies shall be linked statically as well. The LAPACK CMake setup allows a user to rely on an existing BLAS library. For example, when using OpenBLAS as BLAS implementation, the pkg-config output becomes
$ env PKG_CONFIG_PATH=/tmp/prefix.lapack+openblas+vanilla/lib/pkgconfig/ pkg-config --libs --static lapack
-L/tmp/prefix.lapack+openblas+vanilla/lib -L/usr/lib/x86_64-linux-gnu/openblas-pthread -llapack -lblas -lgfortran -lpthread -lm
This works because
- there is a
Requires.private
keyword field in the LAPACK pkg-config file and - the alternative BLAS implementation provides a pkg-config file.
Here is one of the proposed changes:
diff --git a/lapack.pc.in b/lapack.pc.in
index 316c87101..ab5b588bd 100644
--- a/lapack.pc.in
+++ b/lapack.pc.in
@@ -5,5 +5,5 @@ Name: LAPACK
Description: FORTRAN reference implementation of LAPACK Linear Algebra PACKage
Version: @LAPACK_VERSION@
URL: http://www.netlib.org/lapack/
-Libs: -L${libdir} -llapack
-Requires.private: blas
+Libs: -L${libdir} -l@LAPACKLIB@
+Libs.private: @BLAS_PCLIBS@
Given a LAPACK build with an external BLAS library selected with USE_OPTIMIZED_BLAS
, the commit replaces Requires.private
with the actual library name that was found by find_package(BLAS)
. On my computer, pkg-config returns then a shared library path:
$ env PKG_CONFIG_PATH=/tmp/prefix.lapack+openblas/lib/pkgconfig/ pkg-config --libs --static lapack
-L/tmp/prefix.lapack+openblas/lib -llapack /usr/lib/x86_64-linux-gnu/libopenblas.so
Obviously this pull request changes the pkg-config behavior: Instead of recursively looking for static libraries and flags for static linking, the dependency is linked dynamically. Whether the new behavior is right or wrong depends on the user's needs:
- If the alternative BLAS implementation is only available as a static library, then the new approach will cause linker errors.
- If the alternative BLAS implementation does not provide a pkg-config file, then the new approach will avoid linker errors or stop pkg-config from finding the
blas.pc
file of another third-party BLAS implementation.
diff --git a/CBLAS/src/CMakeLists.txt b/CBLAS/src/CMakeLists.txt
index fb1d764c6..445d8f846 100644
--- a/CBLAS/src/CMakeLists.txt
+++ b/CBLAS/src/CMakeLists.txt
@@ -116,7 +116,6 @@ list(REMOVE_DUPLICATES SOURCES)
add_library(${CBLASLIB} ${SOURCES})
set_target_properties(
${CBLASLIB} PROPERTIES
- LINKER_LANGUAGE C
VERSION ${LAPACK_VERSION}
SOVERSION ${LAPACK_MAJOR_VERSION}
)
Could you please write a comment or explain why LINKER_LANGUAGE C
is not necessary or wrong?
I still like the simultaneous build of shared and static libs, which does properly build with and without PIC, for packaging. But I could work around it if you really oppose the lines I added for that.
By default I am opposed to building shared and static libraries automatically with the same compiler and linker options. In CMake, the immediate effect is code duplication. This problem could be solved by introducing new CMake functions. Next, the compile time is doubled because the codes in shared and static libraries are fundamentally different from each other. Finally, the major problem for me is the value proposition of a simultaneous build. A shared library is for most users on most systems the best choice because of the easy updates and the simplified use because shared libraries contain tables listing their dependencies. There are use cases for static libraries, for example, small executables that do not contain unreachable library code or when code must be cross-compiled. In these scenarios the simultaneous build will not help because the shared library won't be used; special compiler and linker flags might be needed, too. For example, to remove dead code with the GNU linker, every function must be put in its own ELF section by the compiler generating the object files. One can build a shared library with the necessary options but they inhibit certain optimizations (see the GCC documentation of -ffunction-sections
).
I still like the simultaneous build of shared and static libs, By default I am opposed to building shared and static libraries
Thanks for taking the time. I will answer this one first since it is easy:
- If you just say no, I can accept that and will remove those bits. I will build it twice in the packaging, then. I have more headache about about fixing up the headers for ilp64 or maybe settling the .pc file stuff.
- About interference between static and dynamic builds, optimizations: Where is it? CMake does separate builds if static and dynamic libs asked at the same time. And it is common for Autotools projects to do the same with libtool. It's existing practice. Nothing changes besides duplicated CMakefile code and more convenience to me as packager.
- The use case: This is about packaging for a user base, some of which might need the static lib, some will need the shared one. End of discussion;-)
diff --git a/CBLAS/src/CMakeLists.txt b/CBLAS/src/CMakeLists.txt index fb1d764c6..445d8f846 100644 --- a/CBLAS/src/CMakeLists.txt +++ b/CBLAS/src/CMakeLists.txt @@ -116,7 +116,6 @@ list(REMOVE_DUPLICATES SOURCES) add_library(${CBLASLIB} ${SOURCES}) set_target_properties( ${CBLASLIB} PROPERTIES - LINKER_LANGUAGE C VERSION ${LAPACK_VERSION} SOVERSION ${LAPACK_MAJOR_VERSION} )
Could you please write a comment or explain why
LINKER_LANGUAGE C
is not necessary or wrong?
This patch was introduced with this comment:
2021-04-21 cblas: Fix link to Fortran libraries by using Fortran compiler as linker
On NetBSD.
In PKGSRC_FORTRAM=gfortran case, libcblas has no RPATH=/usr/pkg/gccXX/lib
and libgfortran and libquadmath are not found.
In PKGSRC_FORTRAN=g95 case, libcblas has no
RPATH=/usr/pkg/lib/gcc-lib/x86_64--netbsd/4.1.2 and libf95 is not found.
Use Fortran compiler as linker instread of C compiler to fix link.
I am unsure … CBLAS/src does contain C and Fortran sources. Considering that Fortran needs extra runtime, I presume libgfortran would be potentially missing if the BLAS being linked to isn't actually a Fortran library that itself pulls libgfortran in. LAPACKE is all-C, right?
I am in the process to changing it, but the initial packaging of BLAS stuff just used BLAS and LAPACK from an optimized (possibly C-based) implementation and put CBLAS from the netlib on top. In this case, CBLAS would be the one needing the Fortran runtime library, not the underlying BLAS, which poses some questions regarding static libs again … but well, you don't need to convince me that nowadays you mostly use shared libs with their recorded dependencies.
Any harm caused in linking CBLAS with Fortran?
* the alternative BLAS implementation provides a pkg-config file.
Which happens to be named blas.pc in your case, not openblas.pc? I don't see how you can rely on that.
Given a LAPACK build with an external BLAS library selected with
USE_OPTIMIZED_BLAS
, the commit replacesRequires.private
with the actual library name that was found byfind_package(BLAS)
No. It replaces it with the BLAS_LIBRARIES I specified it to use;-) The case of CMake finding it itself and inserting a shared library path is bad and I am welcoming suggestions on how to do this differently. With BLAS stuff, builds searching for an implementation themselves are a nightmare. We possibly have several implementations available (so that HPC users can build their custom software with them), but I ensure that a consistent default one is used for the packaging builds, because otherwise packages nuke themselves out with inconsistent linkage. Guessing is bad here. The BLAS situation is too messy. I have a really hard time explaining it to ‘normal’ computer people. Binary distributions go a differing path, with runtime switching of shared libs, but that is not applicable in my case with pkgsrc trees that offer readily-built software and libraries to build own stuff to differing users.
So, how can I phrase this correctly, to have proper linker flags/dependencies in the .pc file for both a located package and me having specified flags to use via BLAS_LIBRARIES?
Oh. The -frecursive thing creeps me out. LAPACK is not thread-safe without it? Or are you just not sure? It is a significant difference in build systems if one has the flag and the other not. Why not? Of course we use OpenBLAS for serious work, but multithreaded applications are the norm and the reference should be correct … The discussion at the other issues has me confused about the current state. Is it just some tests? The tradeoff with -frecursive is just that you need big stack (ulimit -s unlimited is the way to go with any Fortran code …)?
Sorry for the late reply to these questions. There is a recent discussion about the need for recursive flags following https://github.com/Reference-LAPACK/lapack/pull/486#issuecomment-778272365. DSYEVR was not thread-safe (See https://www.netlib.org/lapack/bug_list.html#_strong_span_class_green_bug112_span_strong_dsyevr_does_not_seem_to_be_thread_safe). Also, DSYEVR wasn't changed since 77a7afb, so it is remained not being thread-safe.
I wonder if I introduced multitreading issues in switching from Makefile to CMake; should add -frecursive to the build, then. I don't get why CMake couldn't do this, though (after testing compiler support).
Pkgsrc had patches to get the shared libs out of the Makefiles before, like just about any distro, none of which ever contributed them here? And I actually detest CMake, but it seemed like the way forward here. Should we rather try to revive those patches? I guess I could add options to the Makefiles, too, and switch back. Curious.
It is certainly better to have multiple build systems working with shared libs options. I think it is a good improvement if these patches can be used in LAPACK.
* the alternative BLAS implementation provides a pkg-config file.
Which happens to be named blas.pc in your case, not openblas.pc? I don't see how you can rely on that.
You cannot rely on it. For example, with Intel MKL the Requires.static
keyword value would have to be, e.g., mkl-static-lp64-iomp
, cf. Intel® Math Kernel Library (Intel® MKL) and pkg-config tool.
I still like the simultaneous build of shared and static libs, By default I am opposed to building shared and static libraries [snip] 2. About interference between static and dynamic builds, optimizations: Where is it? CMake does separate builds if static and dynamic libs asked at the same time. And it is common for Autotools projects to do the same with libtool. It's existing practice. Nothing changes besides duplicated CMakefile code and more convenience to me as packager.
The interference is in the compiler flags if one assumes that static libraries are preferred over shared libraries because of the custom build options that can be used.
CMake automatically searches the environment for relevant compiler and linker flags and these are used for all targets. Hence, if the environment variables CFLAGS
contains -ffunction-sections
, then both shared and static library code will be built with this option; this flag undesirable though for shared library code. With respect to linker flags, CMake exposes the variables CMAKE_SHARED_LINKER_FLAGS
and CMAKE_STATIC_LINKER_FLAGS
so this problem could be resolved.
* the alternative BLAS implementation provides a pkg-config file.
Which happens to be named blas.pc in your case, not openblas.pc? I don't see how you can rely on that.
Given a LAPACK build with an external BLAS library selected with
USE_OPTIMIZED_BLAS
, the commit replacesRequires.private
with the actual library name that was found byfind_package(BLAS)
No. It replaces it with the BLAS_LIBRARIES I specified it to use;-) The case of CMake finding it itself and inserting a shared library path is bad and I am welcoming suggestions on how to do this differently. With BLAS stuff, builds searching for an implementation themselves are a nightmare. We possibly have several implementations available (so that HPC users can build their custom software with them), but I ensure that a consistent default one is used for the packaging builds, because otherwise packages nuke themselves out with inconsistent linkage. Guessing is bad here. The BLAS situation is too messy. I have a really hard time explaining it to ‘normal’ computer people. Binary distributions go a differing path, with runtime switching of shared libs, but that is not applicable in my case with pkgsrc trees that offer readily-built software and libraries to build own stuff to differing users.
Have you looked at Spack? Spack allows you to have builds of the same software but with dependencies on different implementations and it can create environment module files.
So, how can I phrase this correctly, to have proper linker flags/dependencies in the .pc file for both a located package and me having specified flags to use via BLAS_LIBRARIES?
We could let the user pass the name of a pkg-config dependency and optionally its location. If the location is not given, the user could still set the environment variable PKG_CONFIG_PATH
. The implementation would be not be hard because CMake has had a pkg-config module for ages called FindPkgConfig. Updating lapack.pc
would be easy then (insert the user-provided package name).
Also, DSYEVR wasn't changed since 77a7afb, so it is remained not being thread-safe.
$ FFLAGS=-Os cmake ../
[…]
-- Performing Test _recursiveFlag
-- Performing Test _recursiveFlag - Failed
-- Performing Test _frecursiveFlag
-- Performing Test _frecursiveFlag - Success
-- Performing Test _MrecursiveFlag
-- Performing Test _MrecursiveFlag - Failed
[…]
$ make
[…]
0%] Building Fortran object BLAS/SRC/CMakeFiles/blas.dir/isamax.f.o
cd /data/projekte/pkgsrc/lapack-3.9.1/b/BLAS/SRC && /usr/bin/f95 -Os -frecursive -O2 -DNDEBUG -O2 -ffixed-form -c /data/projekte/pkgsrc/lapack-3.9.1/BLAS/SRC/isamax.f -o CMakeFiles/blas.dir/isamax.f.o
So this is handled in the current build. Good. This flag setting is good. What I don't like is that the build overrides the user-specified FFLAGS. When I say -Os, I would like that to be in effect, not -O2:-( Any specific reason that the user FFLAGS are not appended instead of prepended? If there are nasty flags handed in, that's the user's fault. I don't mind sane defaults in the build itself, but the choice should be honoured.
[Edit: FFLAGS=-Os cmake -DCMAKE_BUILD_TYPE=foo $SRC
does the right thing. The recursive flag is still added.]
It is certainly better to have multiple build systems working with shared libs options. I think it is a good improvement if these patches can be used in LAPACK.
I am not sure if the pkgsrc patches were particularily nice. You can grab them in the pkgsrc CVS (yes, CVS) history, or just in an earlier release tarball. I need to limit the time I spend on this stuff. The actually more interesting thing I need to clear up is to figure out why/if OpenBLAS does not work well with SuiteSparse And I need to install actual software for users. And do my actual job …
I have some comments about the changes related to "consistent logic for USE_OPTIMIZED_BLAS and USE_OPTIMIZED_LAPACK". As I understood, and please correct me if I am wrong, there are at least three use cases where the behavior of the build system will be affected. These are:
a) When the user provides USE_OPTIMIZED_BLAS = OFF and BLAS_LIBRARIES=<provided>
- Current behavior: use BLAS_LIBRARIES as optimized BLAS
- Proposed behavior: don't use BLAS_LIBRARIES provided by the user. Do not raise any configuration error.
b) When the user provides USE_OPTIMIZED_LAPACK = OFF and LAPACK_LIBRARIES=<provided>
- Current behavior: use LAPACK_LIBRARIES as optimized LAPACK
- Proposed behavior: don't use LAPACK_LIBRARIES provided by the user. Do not raise any configuration error.
c) When the user provides USE_OPTIMIZED_LAPACK = ON and LAPACK_LIBRARIES=<provided>
- Current behavior: the CMake tries to find the package with
find_package(LAPACK)
. This possibly changes the value of the variable LAPACK_LIBRARIES. - Proposed behavior: use LAPACK_LIBRARIES provided by the user.
I don't like the new behaviors for (a) and for (b). I prefer either the current one a FATAL message terminating the build. It seems odd to just ignore the input. I like (c) :)
Which happens to be named blas.pc in your case, not openblas.pc? I don't see how you can rely on that.
You cannot rely on it.
Exactly my point. The established way to tell about a BLAS implementation (especially BLAS, perhaps less so for CBLAS if one wants to find the header, too) is to hand in the needed linker flags, e.g. BLAS_LIBS=-L/foo/bar/lib -lfooblas
.
The ideal handling would be one of
- Use provided libs in Libs.private as given by user.
- Use proper detected/selected by user pkg-config module in Requires.private.
I see pkg-config handling for BLAS stuff really as seconday. Generally, I like .pc files being around and rely on them for my own software. But the BLAS situation is special, where you simply don't know which .pc file to choose without asking the user's preference. Also details like ILP64 or not. It's just messed up as it is and glossing over pkg-config doesn't suddenly make it simple.
So, what do you want from this PR. Should I test if BLAS_LIBRARIES is user-specified and use that for BLAS_PCLIBS, otherwise keep the current state and you later fix it up to work for pkg-config module selections if so desired?
libdir=@CMAKE_INSTALL_FULL_LIBDIR@
includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@
Name: LAPACK
Description: FORTRAN reference implementation of LAPACK Linear Algebra PACKage
Version: @LAPACK_VERSION@
URL: http://www.netlib.org/lapack/
Libs: -L${libdir} -l@LAPACKLIB@
@PC_LAPACK_PRIVATE@
And set PC_LAPACK_PRIVATE to either the correct Libs.private
and Requires.private
lines?
Maybe we should split your contributions into different PRs to ease discussion with the community. "LAPACK option to be able to just build BLAS" and "consistent logic for USE_OPTIMIZED_BLAS and USE_OPTIMIZED_LAPACK", for instance, are orthogonal to the use of BUILD_STATIC_LIBS
, right? I can help you if needed.
diff --git a/CBLAS/src/CMakeLists.txt b/CBLAS/src/CMakeLists.txt index fb1d764c6..445d8f846 100644 --- a/CBLAS/src/CMakeLists.txt +++ b/CBLAS/src/CMakeLists.txt @@ -116,7 +116,6 @@ list(REMOVE_DUPLICATES SOURCES) add_library(${CBLASLIB} ${SOURCES}) set_target_properties( ${CBLASLIB} PROPERTIES - LINKER_LANGUAGE C VERSION ${LAPACK_VERSION} SOVERSION ${LAPACK_MAJOR_VERSION} )
Could you please write a comment or explain why
LINKER_LANGUAGE C
is not necessary or wrong?This patch was introduced with this comment:
2021-04-21 cblas: Fix link to Fortran libraries by using Fortran compiler as linker On NetBSD. In PKGSRC_FORTRAM=gfortran case, libcblas has no RPATH=/usr/pkg/gccXX/lib and libgfortran and libquadmath are not found. In PKGSRC_FORTRAN=g95 case, libcblas has no RPATH=/usr/pkg/lib/gcc-lib/x86_64--netbsd/4.1.2 and libf95 is not found. Use Fortran compiler as linker instread of C compiler to fix link.
I am unsure … CBLAS/src does contain C and Fortran sources. Considering that Fortran needs extra runtime, I presume libgfortran would be potentially missing if the BLAS being linked to isn't actually a Fortran library that itself pulls libgfortran in. LAPACKE is all-C, right?
I am in the process to changing it, but the initial packaging of BLAS stuff just used BLAS and LAPACK from an optimized (possibly C-based) implementation and put CBLAS from the netlib on top. In this case, CBLAS would be the one needing the Fortran runtime library, not the underlying BLAS, which poses some questions regarding static libs again … but well, you don't need to convince me that nowadays you mostly use shared libs with their recorded dependencies.
Any harm caused in linking CBLAS with Fortran?
I have no strong opinion on what is the best option for linking C with Fortran code. What I do know is that some users have problems in installing libgfortran on MacOS, and a possible solution is to use gfortran to link.
Certainly, Christoph and @drhpc have more experience with that than I. I would just like to remark that this PR keeps the same behavior from before when the new flag BUILD_STATIC_LIBS is OFF. Moreover, I believe most of the users wouldn't use this flag unless they know what they are doing since:
-
the default value of BUILD_STATIC_LIBS is OFF, and
-
BUILD_STATIC_LIBS is not a CMake configuration option, in the sense that there is no
option(BUILD_STATIC_LIBS, ...)
. So the user has to read the manual or the CMakelists.txt to know about it.
I like the way you designed it, and the idea of having a new feature. However, I think this shall be well documented in the manual, or at least in the CMakelists. Something like:
- what it does:
(...) enable a single round of
cmake make make install
to get shared and static libraries installed.
-
When this is preferable instead of building twice
-
Remark cases where this option shouldn't be used, if they exist. (Maybe we open a new issue to include a testing workflow that makes sure some use cases work as expected)
What do you think?