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

add static build config. #510

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 11 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@

# Options

option(SLEEF_BUILD_STATIC_TEST_BINS "Build statically linked test executables" OFF)
Copy link
Contributor

@alexreinking alexreinking Mar 15, 2024

Choose a reason for hiding this comment

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

The behavior of option and set are controlled by policies. You must set cmake_minimum_required as the first line of your top-level program to have any backwards compatibility guarantees at all.

Ideally project() would be the second line since so many other commands depend on the internal state it configures.

Suggested change
option(SLEEF_BUILD_STATIC_TEST_BINS "Build statically linked test executables" OFF)
cmake_minimum_required(VERSION 3.18)
project(SLEEF LANGUAGES C VERSION 3.6.0)
# TODO: rename uses of this variable to use the standard one
set(SLEEF_VERSION_PATCHLEVEL ${SLEEF_VERSION_PATCH})
set(SLEEF_SOVERSION ${SLEEF_VERSION_MAJOR})
option(SLEEF_BUILD_STATIC_TEST_BINS "Build statically linked test executables" OFF)

Then, after your options, add the following lines:

if (SLEEF_ENABLE_CXX)
  enable_language(CXX)
endif ()
if (SLEEF_ENABLE_CUDA)
  enable_language(CUDA)
endif ()

No need to maintain that LANGLIST anymore. Just enable the requested languages.

Then, below, delete these lines:

cmake_minimum_required(VERSION 3.18)
if(${CMAKE_VERSION} VERSION_GREATER "3.14.99")
  cmake_policy(SET CMP0091 NEW)
endif()

CMP0091 is enabled in CMake 3.15+ so the cmake_policy line is a noop and so the whole if () block can be removed.

Also delete these lines:

set(SLEEF_VERSION_MAJOR 3)
set(SLEEF_VERSION_MINOR 6)
set(SLEEF_VERSION_PATCHLEVEL 0)
set(SLEEF_VERSION ${SLEEF_VERSION_MAJOR}.${SLEEF_VERSION_MINOR}.${SLEEF_VERSION_PATCHLEVEL})
set(SLEEF_SOVERSION ${SLEEF_VERSION_MAJOR})
set(LANGLIST C)
if (SLEEF_ENABLE_CUDA)
  set(LANGLIST ${LANGLIST} CUDA)
endif()
if (SLEEF_ENABLE_CXX)
  set(LANGLIST ${LANGLIST} CXX)
endif()

option(SLEEF_ENABLE_LTO "Enable LTO on GCC or ThinLTO on clang" OFF)
option(SLEEF_BUILD_LIBM "libsleef will be built." ON)
Expand Down Expand Up @@ -29,6 +28,13 @@ option(SLEEF_DISABLE_SSL "Disable testing with the SSL library" OFF)
option(SLEEF_ENABLE_CUDA "Enable CUDA" OFF)
option(SLEEF_ENABLE_CXX "Enable C++" OFF)

set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if(NOT DEFINED ${SLEEF_BUILD_SHARED_LIBS})
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
else()
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif()

Comment on lines +31 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be done with options instead? For instance,

option(BUILD_SHARED_LIBS "Build sleef as shared library" ON)
option(SLEEF_BUILD_SHARED_LIBS "Build sleef as shared library" ${BUILD_SHARED_LIBS})

Copy link
Contributor

Choose a reason for hiding this comment

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

@blapie -- These are not equivalent because this defines SLEEF_BUILD_SHARED_LIBS as a cache variable when no variable by that name (normal or cache) exists. So then changing BUILD_SHARED_LIBS only works as expected on the first configuration.

My blog post suggests the following:

Suggested change
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if(NOT DEFINED ${SLEEF_BUILD_SHARED_LIBS})
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
else()
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif()
if (DEFINED SLEEF_BUILD_SHARED_LIBS)
set(BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif ()

Then you can just look at the truthiness of BUILD_SHARED_LIBS down the line. If you care about documenting this variable in the CMake CLI/GUI, then this approach works:

Suggested change
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if(NOT DEFINED ${SLEEF_BUILD_SHARED_LIBS})
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS})
else()
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif()
set(SLEEF_BUILD_SHARED_LIBS "undefined"
CACHE BOOL "Override BUILD_SHARED_LIBS while configuring SLEEF")
if (NOT SLEEF_BUILD_SHARED_LIBS STREQUAL "undefined")
set(BUILD_SHARED_LIBS ${SLEEF_BUILD_SHARED_LIBS})
endif ()

# Function used to generate safe command arguments for add_custom_command
function(command_arguments PROPNAME)
set(quoted_args "")
Expand Down Expand Up @@ -157,9 +163,9 @@ separate build directory. Note: Please remove autogenerated file \
`CMakeCache.txt` and directory `CMakeFiles` in the current directory.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Everywhere you should just use SLEEF_SOURCE_DIR and SLEEF_BINARY_DIR since those are the standard and expected names. The project command defines them and the VERSION variables.

https://cmake.org/cmake/help/v3.18/command/project.html#project

endif()

if(SLEEF_ENABLE_LTO AND BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and BUILD_SHARED_LIBS cannot be specified at the same time")
endif(SLEEF_ENABLE_LTO AND BUILD_SHARED_LIBS)
if(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and _INTERNAL_SLEEF_BUILD_SHARED_LIBS cannot be specified at the same time")
endif(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
Comment on lines +166 to +168
Copy link
Contributor

Choose a reason for hiding this comment

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

With my other suggestions:

Suggested change
if(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and _INTERNAL_SLEEF_BUILD_SHARED_LIBS cannot be specified at the same time")
endif(SLEEF_ENABLE_LTO AND _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if (SLEEF_ENABLE_LTO AND BUILD_SHARED_LIBS)
message(FATAL_ERROR "SLEEF_ENABLE_LTO and (SLEEF_)BUILD_SHARED_LIBS cannot be enabled at the same time")
endif ()

Note that duplicating the condition in the endif hasn't been a requirement since at least 2.8.x but it might be as far back as 2.4.x. Definitely not needed in 3.18, which is your minimum.


if(SLEEF_ENABLE_LTO)
cmake_policy(SET CMP0069 NEW)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cmake_policy(SET CMP0069 NEW)

That policy is already enabled by cmake_minimum_required(VERSION 3.18). It's enabled for 3.9+.

Expand Down Expand Up @@ -305,7 +311,7 @@ if(SLEEF_SHOW_CONFIG)
message(" Native build dir: ${NATIVE_BUILD_DIR}")
endif(CMAKE_CROSSCOMPILING)
message(STATUS "Using option `${SLEEF_C_FLAGS}` to compile libsleef")
message(STATUS "Building shared libs : " ${BUILD_SHARED_LIBS})
message(STATUS "Building shared libs : " ${_INTERNAL_SLEEF_BUILD_SHARED_LIBS})
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
message(STATUS "Building shared libs : " ${_INTERNAL_SLEEF_BUILD_SHARED_LIBS})
message(STATUS "Building shared libs : " ${BUILD_SHARED_LIBS})

message(STATUS "Building static test bins: " ${SLEEF_BUILD_STATIC_TEST_BINS})
message(STATUS "MPFR : " ${LIB_MPFR})
if (MPFR_INCLUDE_DIR)
Expand Down
4 changes: 2 additions & 2 deletions Configure.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ include(CheckLanguage)

if (SLEEF_BUILD_STATIC_TEST_BINS)
set(CMAKE_FIND_LIBRARY_SUFFIXES ".a")
set(BUILD_SHARED_LIBS OFF)
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
set(_INTERNAL_SLEEF_BUILD_SHARED_LIBS OFF)
set(BUILD_SHARED_LIBS OFF)

set(CMAKE_EXE_LINKER_FLAGS "-static")
endif()

Expand Down Expand Up @@ -834,7 +834,7 @@ endif()

# Set common definitions

if (NOT BUILD_SHARED_LIBS)
if (NOT _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (NOT _INTERNAL_SLEEF_BUILD_SHARED_LIBS)
if (NOT BUILD_SHARED_LIBS)

set(COMMON_TARGET_DEFINITIONS SLEEF_STATIC_LIBS=1)
set(SLEEF_STATIC_LIBS 1)
endif()
Expand Down
2 changes: 1 addition & 1 deletion docs/build-with-cmake.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ optimized, or any other special set of flags.
- `SLEEF_SHOW_CONFIG` : Show relevant CMake variables upon configuring a build
- `SLEEF_SHOW_ERROR_LOG` : Show the content of CMakeError.log

- `BUILD_SHARED_LIBS` : Static libs are built if set to FALSE
- `SLEEF_BUILD_SHARED_LIBS` : Static libs are built if set to FALSE
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `SLEEF_BUILD_SHARED_LIBS` : Static libs are built if set to FALSE
- `SLEEF_BUILD_SHARED_LIBS` : Static libs are built if set to FALSE. If not manually set, then `BUILD_SHARED_LIBS` is used.

- `SLEEF_BUILD_GNUABI_LIBS` : Avoid building libraries with GNU ABI if set to FALSE
- `SLEEF_BUILD_INLINE_HEADERS` : Generate header files for inlining whole SLEEF functions

Expand Down
2 changes: 1 addition & 1 deletion src/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ set(COMMON_TARGET_PROPERTIES
C_STANDARD 99 # -std=gnu99
)

if (BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON) # -fPIC
endif()

Expand Down
20 changes: 7 additions & 13 deletions src/dft/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ set(COMMON_TARGET_PROPERTIES
C_STANDARD 99 # -std=gnu99
)

if (BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON) # -fPIC
endif()

Expand Down Expand Up @@ -373,8 +373,11 @@ foreach(T ${LIST_SUPPORTED_FPTYPE})
endforeach()

# Target libdft

add_library(${TARGET_LIBDFT} $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBDFT} SHARED $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
else()
add_library(${TARGET_LIBDFT} STATIC $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
endif()
Comment on lines +376 to +380
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBDFT} SHARED $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
else()
add_library(${TARGET_LIBDFT} STATIC $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)
endif()
add_library(${TARGET_LIBDFT} $<TARGET_OBJECTS:dftcommon_obj> $<TARGET_OBJECTS:${TARGET_LIBARRAYMAP_OBJ}>)

target_link_libraries(${TARGET_LIBDFT} ${TARGET_LIBSLEEF} ${LIBM})

foreach(T ${LIST_SUPPORTED_FPTYPE})
Expand Down Expand Up @@ -422,13 +425,4 @@ install(
COMPONENT sleef_Runtime
INCLUDES #
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
)

install(
TARGETS ${TARGET_LIBDFT}
DESTINATION dummy # provided above already
LIBRARY #
DESTINATION "${CMAKE_INSTALL_LIBDIR}"
COMPONENT sleef_Development
NAMELINK_ONLY
)
)
47 changes: 17 additions & 30 deletions src/libm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ set(COMMON_TARGET_PROPERTIES
C_STANDARD 99 # -std=gnu99
)

if (BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON) # -fPIC
endif()

Expand All @@ -359,7 +359,12 @@ endif()
# Original sleef sources
set(STANDARD_SOURCES rempitab.c)

add_library(${TARGET_LIBSLEEF} ${STANDARD_SOURCES})
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBSLEEF} SHARED ${STANDARD_SOURCES})
else()
add_library(${TARGET_LIBSLEEF} STATIC ${STANDARD_SOURCES})
endif()
Comment on lines +362 to +366
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBSLEEF} SHARED ${STANDARD_SOURCES})
else()
add_library(${TARGET_LIBSLEEF} STATIC ${STANDARD_SOURCES})
endif()
add_library(${TARGET_LIBSLEEF} ${STANDARD_SOURCES})

I'm going to stop highlighting all the places this needs to be reverted.


add_dependencies(${TARGET_LIBSLEEF} ${TARGET_HEADERS})
set_target_properties(${TARGET_LIBSLEEF} PROPERTIES
VERSION ${SLEEF_VERSION}
Expand Down Expand Up @@ -884,7 +889,11 @@ if(ENABLE_GNUABI)
endforeach()

# Create library
add_library(${TARGET_LIBSLEEFGNUABI} ${TARGET_LIBSLEEFGNUABI_OBJECTS} rempitab.c)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(${TARGET_LIBSLEEFGNUABI} SHARED ${TARGET_LIBSLEEFGNUABI_OBJECTS} rempitab.c)
else()
add_library(${TARGET_LIBSLEEFGNUABI} STATIC ${TARGET_LIBSLEEFGNUABI_OBJECTS} rempitab.c)
endif()

# Library properties
set_target_properties(${TARGET_LIBSLEEFGNUABI} PROPERTIES
Expand Down Expand Up @@ -973,7 +982,11 @@ endif()
# --------------------------------------------------------------------
# Build scalar-only library from sleefdp.c and sleefsp.c
if(SLEEF_BUILD_SCALAR_LIB)
add_library(sleefscalar sleefdp.c sleefsp.c rempitab.c)
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(sleefscalar SHARED sleefdp.c sleefsp.c rempitab.c)
else()
add_library(sleefscalar STATIC sleefdp.c sleefsp.c rempitab.c)
endif()
add_dependencies(sleefscalar ${TARGET_HEADERS})
set_target_properties(sleefscalar PROPERTIES
VERSION ${SLEEF_VERSION}
Expand Down Expand Up @@ -1004,15 +1017,6 @@ if(SLEEF_BUILD_SCALAR_LIB)
DESTINATION "${CMAKE_INSTALL_BINDIR}"
COMPONENT sleef_Runtime
)

install(
TARGETS sleefscalar
DESTINATION dummy # provided above already
LIBRARY #
DESTINATION "${CMAKE_INSTALL_LIBDIR}"
COMPONENT sleef_Development
NAMELINK_ONLY
)
endif()

# --------------------------------------------------------------------
Expand All @@ -1038,14 +1042,6 @@ install(
INCLUDES #
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
)
install(
TARGETS ${TARGET_LIBSLEEF}
DESTINATION dummy # provided above already
LIBRARY #
DESTINATION "${CMAKE_INSTALL_LIBDIR}"
COMPONENT sleef_Development
NAMELINK_ONLY
)
configure_file("sleef.pc.in" "${CMAKE_CURRENT_BINARY_DIR}/sleef.pc" @ONLY)
install(
FILES "${CMAKE_CURRENT_BINARY_DIR}/sleef.pc"
Expand All @@ -1068,13 +1064,4 @@ if(ENABLE_GNUABI)
DESTINATION "${CMAKE_INSTALL_BINDIR}"
COMPONENT sleef_Runtime
)

install(
TARGETS ${TARGET_LIBSLEEF}
DESTINATION dummy # provided above already
LIBRARY #
DESTINATION "${CMAKE_INSTALL_LIBDIR}"
COMPONENT sleef_Development
NAMELINK_ONLY
)
endif()
19 changes: 7 additions & 12 deletions src/quad/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ if(COMPILER_SUPPORTS_BUILTIN_MATH)
list(APPEND COMMON_TARGET_DEFINITIONS ENABLE_BUILTIN_MATH=1)
endif()

if (BUILD_SHARED_LIBS)
if (_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
list(APPEND COMMON_TARGET_PROPERTIES POSITION_INDEPENDENT_CODE ON)
endif()

Expand Down Expand Up @@ -214,7 +214,11 @@ foreach(SIMD ${SLEEF_SUPPORTED_QUAD_EXTENSIONS})
endif()
endforeach()

add_library(sleefquad rempitabqp.c ${SLEEFQUAD_OBJECTS})
if(_INTERNAL_SLEEF_BUILD_SHARED_LIBS)
add_library(sleefquad SHARED rempitabqp.c ${SLEEFQUAD_OBJECTS})
else()
add_library(sleefquad STATIC rempitabqp.c ${SLEEFQUAD_OBJECTS})
endif()

set_target_properties(sleefquad PROPERTIES
VERSION ${SLEEF_VERSION}
Expand Down Expand Up @@ -501,13 +505,4 @@ install(
COMPONENT sleef_Runtime
INCLUDES #
DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
)

install(
TARGETS sleefquad
DESTINATION dummy # provided above already
LIBRARY #
DESTINATION "${CMAKE_INSTALL_LIBDIR}"
COMPONENT sleef_Development
NAMELINK_ONLY
)
)
Loading