Reference-LAPACK / lapack

LAPACK development repository
Other
1.49k stars 436 forks source link

Use more modern CMake #1022

Closed ACSimon33 closed 3 months ago

ACSimon33 commented 4 months ago

Description

At the moment, LAPACK sets the global compile flags via the CMAKE_Fortran_FLAGS cache variable. The problem with that is if someone wants to compile LAPACK as a sub-project inside a bigger CMake project, the CMAKE_Fortran_FLAGS that LAPACK sets will automatically be applied to all targets of the parent project, which is definitely not ideal.

There are two options to fix this:

Since we are already using target_compile_options(...) for some libraries, I think we should use the second option and not change any variables directly.

994 already pointed out that we currently need at least CMake 3.12 anyway because we are linking object libraries. So, the jump to CMake 3.13 wouldn't be completely unreasonable. The issue also mentioned the use of Fortran_PREPROCESS, which is actually a CMake 3.18 feature. We can still use CMake < 3.18, because non-existing target properties are just ignored. The only place where it is necessary is if BUILD_INDEX64_EXT_API is enabled because we need to preprocess all Fortran files in that case.


This MR changes all occurrences of CMAKE_Fortran_FLAGS and CMAKE_*_LINKER_FLAGS to use the functions instead. I compared the compiler and linker flags of the targets before and after the changes. The compiler flags are identical (possibly in a different order). The linker flags are fewer than before because CMAKE_Fortran_FLAGS was added to the compiler and linker flags, which isn't the case anymore. For most flags, this is ok because they are not needed in the linking phase. Those that are needed during compilation and linking are added explicitly via add_link_options(...) (I hope I didn't miss any; please check).

I bumped the CMake minimum version to 3.13 and 3.18 if BUILD_INDEX64_EXT_API is enabled. If the user uses a CMake < 3.18, BUILD_INDEX64_EXT_API is now disabled by default so that the default config works without changing any variables.

Additionally, I fixed the formatting of the CMake file.

Fixes #994

Checklist

langou commented 4 months ago

Thanks @ACSimon33. I approved the PR. I am letting it sit here for a few days if some folks want to express some concerns. If no one expresses concerns, I'll merge in a few days. Thanks a lot. Julien.