Skip to content

CMake: Support building more than one precision #1

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
ruslo merged 2 commits into from
Nov 21, 2017
Merged

CMake: Support building more than one precision #1

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b423d9
CMake: Support building more than one precision
lukas-w Nov 21, 2017
ac2ad83
Minor CMake improvements
lukas-w Nov 21, 2017
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
255 changes: 135 additions & 120 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required (VERSION 3.0)
cmake_minimum_required (VERSION 3.1)

if (NOT DEFINED CMAKE_BUILD_TYPE)
set (CMAKE_BUILD_TYPE Release CACHE STRING "Build type")
Expand All @@ -18,6 +18,7 @@ option (ENABLE_THREADS "Use pthread for multithreading" OFF)
option (WITH_COMBINED_THREADS "Merge thread library" OFF)

option (ENABLE_FLOAT "single-precision" OFF)
option (ENABLE_DOUBLE "double-precision" ON)
option (ENABLE_LONG_DOUBLE "long-double precision" OFF)
option (ENABLE_QUAD_PRECISION "quadruple-precision" OFF)

Expand Down Expand Up @@ -263,145 +264,159 @@ endif ()

set (FFTW_VERSION 3.3.7)

set (PREC_SUFFIX)
if (ENABLE_FLOAT)
set (FFTW_SINGLE TRUE)
set (BENCHFFT_SINGLE TRUE)
set (PREC_SUFFIX f)
if (BUILD_SHARED_LIBS)
add_definitions (-DFFTW_DLL)
Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

GenerateExportHeader should be used instead:

Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

Example:

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

This is code I kept from the original CMakeLists.txt, so I'm hesitant to change it as it's not really related to the issue this PR is addressing. Or would you argue the change is necessary for clean use within Hunter?

Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

As far as I understand this code will be used in installed headers too (can't check because I can't install). Meaning the burden of setting FFTW_DLL moved to user. GenerateExportHeader designed to resolve all problems.

Or would you argue the change is necessary for clean use within Hunter?

I'm pretty sure you we will hit this problem while testing.

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

I see, thanks for explaining. I tested this PR on Windows and didn't have any problems regarding export symbols, so I think it's fine this way. ffftw.h lines 78 to 87 handles this fine as far as I can see:

#if defined(FFTW_DLL) && (defined(_WIN32) || defined(__WIN32__))
   /* annoying Windows syntax for shared-library declarations */
#  if defined(COMPILING_FFTW) /* defined in api.h when compiling FFTW */
#    define FFTW_EXTERN extern __declspec(dllexport)
#  else /* user is calling FFTW; import symbol */
#    define FFTW_EXTERN extern __declspec(dllimport)
#  endif
#else
#  define FFTW_EXTERN extern
#endif

I can do some further testing. If it turns out not to be sufficient, I'll open a separate PR upstream.

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

Have you tried to install it and add it to user's project with find_package(fftw3 CONFIG REQUIRED)?

Yes, I successfully built and ran a project that depends on FFTW3 using Hunter on MSVC, so I did

hunter_add_package(fftw3)
find_package(fftw3f REQUIRED)

I have the Hunter patches on my local machine and will submit them once this PR is settled.

FYI, I'm a developer on the LMMS project (https://github.com/LMMS/lmms). Adding FFTW3 (and some other packages) to Hunter is part of efforts to make LMMS compatible with MSVC. We plan on using Hunter to fetch dependencies on Windows.

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

Also, upstream still supports the autotools build system, so I don't think a GenerateExportHeader patch would be accepted because it could render the two systems incompatible.

Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

hunter_add_package(fftw3)
find_package(fftw3f REQUIRED)

It should be with CONFIG:

hunter_add_package(fftw3)
find_package(fftw3f CONFIG REQUIRED)

Note that my original question was about shared library (DLL on Windows). Hunter by default build static library, you have to add CMAKE_ARGS BUILD_SHARED_LIBS=ON:

As a summary it's up to you. I'm just pretty sure this will lead to opaque linker error on user's side.

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

It should be with CONFIG:

Using CONFIG will break compatibility with non-Hunter builds.

Hunter by default build static library

Ah, I wasn't aware of that. I tried using BUILD_SHARED_LIBS=ON but didn't get any linking errors. Anyway, I'll test this on more environments in the context of LMMS and will submit another patch in case I run into problems.

Copy link

@ruslo ruslo Nov 22, 2017

Choose a reason for hiding this comment

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

Using CONFIG will break compatibility with non-Hunter builds

I think you're already using CONFIG implicilty.

endif ()

if (ENABLE_LONG_DOUBLE)
set (FFTW_LDOUBLE TRUE)
set (BENCHFFT_LDOUBLE TRUE)
set (PREC_SUFFIX l)
endif ()
function (add_fftw_library PRECISION)
if (PRECISION MATCHES "^(SINGLE|LDOUBLE|QUAD)$")
set (FFTW_${PRECISION} TRUE)
set (BENCHFFT_${PRECISION} TRUE)
if (PRECISION STREQUAL SINGLE)
set (PREC_SUFFIX f)
else ()
string (SUBSTRING ${PRECISION} 0 1 PREC_SUFFIX)
string (TOLOWER ${PREC_SUFFIX} PREC_SUFFIX)
endif()
endif ()

if (ENABLE_QUAD_PRECISION)
set (FFTW_QUAD TRUE)
set (BENCHFFT_QUAD TRUE)
set (PREC_SUFFIX q)
endif ()
set (fftw3_lib fftw3${PREC_SUFFIX})
set (fftw3_lib fftw3${PREC_SUFFIX})

configure_file (cmake.config.h.in config.h @ONLY)
include_directories (${CMAKE_CURRENT_BINARY_DIR})
add_library (${fftw3_lib} ${SOURCEFILES})

if (BUILD_SHARED_LIBS)
add_definitions (-DFFTW_DLL)
endif ()
configure_file (cmake.config.h.in ${CMAKE_CURRENT_BINARY_DIR}/${fftw3_lib}/config.h @ONLY)
Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

You've changed location of config.h file but I see no change of C++ code. Is it correct?

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

Yes, the target_include_directories in the line below accounts for this change, so that #include "config.h" still works.

target_include_directories (${fftw3_lib} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/${fftw3_lib}")
Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

Use $<BUILD_INTERFACE:...> here:

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

This line is using PRIVATE, so $<BUILD_INTERFACE:...> won't make a difference here, as PRIVATE includes are only ever used during build, right?

Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

Yes, okay.


if (MSVC)
target_compile_definitions (${fftw3_lib} PRIVATE /bigobj)
endif ()
if (HAVE_SSE)
target_compile_options (${fftw3_lib} PRIVATE ${SSE_FLAG})
endif ()
if (HAVE_SSE2)
target_compile_options (${fftw3_lib} PRIVATE ${SSE2_FLAG})
endif ()
if (HAVE_AVX)
target_compile_options (${fftw3_lib} PRIVATE ${AVX_FLAG})
endif ()
if (HAVE_AVX2)
target_compile_options (${fftw3_lib} PRIVATE ${AVX2_FLAG})
endif ()
if (HAVE_FMA)
target_compile_options (${fftw3_lib} PRIVATE ${FMA_FLAG})
endif ()
if (HAVE_LIBM)
target_link_libraries (${fftw3_lib} m)
endif ()

add_library (${fftw3_lib} ${SOURCEFILES})
if (MSVC)
target_compile_definitions (${fftw3_lib} PRIVATE /bigobj)
endif ()
if (HAVE_SSE)
target_compile_options (${fftw3_lib} PRIVATE ${SSE_FLAG})
endif ()
if (HAVE_SSE2)
target_compile_options (${fftw3_lib} PRIVATE ${SSE2_FLAG})
endif ()
if (HAVE_AVX)
target_compile_options (${fftw3_lib} PRIVATE ${AVX_FLAG})
endif ()
if (HAVE_AVX2)
target_compile_options (${fftw3_lib} PRIVATE ${AVX2_FLAG})
endif ()
if (HAVE_FMA)
target_compile_options (${fftw3_lib} PRIVATE ${FMA_FLAG})
endif ()
if (HAVE_LIBM)
target_link_libraries (${fftw3_lib} m)
endif ()
list (APPEND subtargets ${fftw3_lib})

set (subtargets ${fftw3_lib})
if (ENABLE_THREADS AND Threads_FOUND)
if (WITH_COMBINED_THREADS)
target_link_libraries (${fftw3_lib} Threads::Threads)
else ()
add_library (${fftw3_lib}_threads ${fftw_threads_SOURCE})
target_link_libraries (${fftw3_lib}_threads ${fftw3_lib})
target_link_libraries (${fftw3_lib}_threads Threads::Threads)
list (APPEND subtargets ${fftw3_lib}_threads)
endif ()
endif ()

if (Threads_FOUND)
if (WITH_COMBINED_THREADS)
target_link_libraries (${fftw3_lib} ${CMAKE_THREAD_LIBS_INIT})
else ()
add_library (${fftw3_lib}_threads ${fftw_threads_SOURCE})
target_link_libraries (${fftw3_lib}_threads ${fftw3_lib})
target_link_libraries (${fftw3_lib}_threads ${CMAKE_THREAD_LIBS_INIT})
list (APPEND subtargets ${fftw3_lib}_threads)
if (ENABLE_OPENMP AND OPENMP_FOUND)
add_library (${fftw3_lib}_omp ${fftw_omp_SOURCE})
target_link_libraries (${fftw3_lib}_omp ${fftw3_lib})
target_link_libraries (${fftw3_lib}_omp ${CMAKE_THREAD_LIBS_INIT})
list (APPEND subtargets ${fftw3_lib}_omp)
target_compile_options (${fftw3_lib}_omp PRIVATE ${OpenMP_C_FLAGS})
endif ()
endif ()

if (OPENMP_FOUND)
add_library (${fftw3_lib}_omp ${fftw_omp_SOURCE})
target_link_libraries (${fftw3_lib}_omp ${fftw3_lib})
target_link_libraries (${fftw3_lib}_omp ${CMAKE_THREAD_LIBS_INIT})
list (APPEND subtargets ${fftw3_lib}_omp)
target_compile_options (${fftw3_lib}_omp PRIVATE ${OpenMP_C_FLAGS})
install(TARGETS ${fftw3_lib}
EXPORT FFTW3LibraryDepends
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})


# pkgconfig file
set (prefix ${CMAKE_INSTALL_PREFIX})
Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

I think this code, that doesn't depend on target, should be removed from function.

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

Hm… It doesn't depend on the target, that's right, but those are the input variables for the following configure_file(…) call, so I think we should still keep them immediately before that call for readability. No damage done except for a couple CPU cycles.

Copy link

@ruslo ruslo Nov 21, 2017

Choose a reason for hiding this comment

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

those are the input variables

There is also code below:

export (TARGETS ${fftw3_lib} NAMESPACE FFTW3:: FILE ${PROJECT_BINARY_DIR}/FFTW3LibraryDepends.cmake)
install(EXPORT FFTW3LibraryDepends
        NAMESPACE FFTW3::
        DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/fftw3${PREC_SUFFIX}
        COMPONENT Development)

Need to test if it will work correctly if called several times.

Copy link
Author

@lukas-w lukas-w Nov 21, 2017

Choose a reason for hiding this comment

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

That code below you quoted differs on each call of the function though. ${fftw3_lib} evaluates to fftw3, fftw3f ,fftw3l or fftw3q depending on the precision selected.

set (exec_prefix ${CMAKE_INSTALL_PREFIX})
set (libdir ${CMAKE_INSTALL_FULL_LIBDIR})
set (includedir ${CMAKE_INSTALL_FULL_INCLUDEDIR})
set (VERSION ${FFTW_VERSION})
configure_file (fftw.pc.in fftw${PREC_SUFFIX}.pc @ONLY)
install (FILES
${CMAKE_CURRENT_BINARY_DIR}/fftw${PREC_SUFFIX}.pc
DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig
COMPONENT Development)

# cmake file
set (FFTW3_LIBRARIES "FFTW3::${fftw3_lib}")
configure_file (FFTW3Config.cmake.in FFTW3${PREC_SUFFIX}Config.cmake @ONLY)
configure_file (FFTW3ConfigVersion.cmake.in FFTW3${PREC_SUFFIX}ConfigVersion.cmake @ONLY)
install (FILES
${CMAKE_CURRENT_BINARY_DIR}/FFTW3${PREC_SUFFIX}Config.cmake
${CMAKE_CURRENT_BINARY_DIR}/FFTW3${PREC_SUFFIX}ConfigVersion.cmake
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/fftw3${PREC_SUFFIX}
COMPONENT Development)

export (TARGETS ${fftw3_lib} NAMESPACE FFTW3:: FILE ${PROJECT_BINARY_DIR}/FFTW3LibraryDepends.cmake)
install(EXPORT FFTW3LibraryDepends
NAMESPACE FFTW3::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/fftw3${PREC_SUFFIX}
COMPONENT Development)


if (BUILD_TESTS)

set (bench bench${PREC_SUFFIX})
add_executable (${bench} ${fftw_libbench2_SOURCE} tests/bench.c tests/hook.c tests/fftw-bench.c)

if (ENABLE_THREADS AND NOT WITH_COMBINED_THREADS)
target_link_libraries (${bench} ${fftw3_lib}_threads)
else ()
target_link_libraries (${bench} ${fftw3_lib})
endif ()

enable_testing ()

if (Threads_FOUND)

macro (fftw_add_test problem)
add_test (NAME ${problem} COMMAND ${bench} -s ${problem})
endmacro ()

fftw_add_test (32x64)
fftw_add_test (ib256)

endif ()

target_include_directories (${bench} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/${fftw3_lib}")
endif ()

endfunction ()

if (ENABLE_FLOAT)
add_fftw_library (SINGLE)
endif ()
if (ENABLE_DOUBLE)
add_fftw_library (DOUBLE)
endif ()
if (ENABLE_LONG_DOUBLE)
add_fftw_library (LDOUBLE)
endif ()
if (ENABLE_QUAD_PRECISION)
add_fftw_library (QUAD)
endif ()

foreach(subtarget ${subtargets})
set_target_properties (${subtarget} PROPERTIES SOVERSION 3.5.7 VERSION 3)
install (TARGETS ${subtarget}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
endforeach ()
install(TARGETS ${fftw3_lib}
EXPORT FFTW3LibraryDepends
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})

install (FILES api/fftw3.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
if (EXISTS api/fftw3.f)
install (FILES api/fftw3.f DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
endif ()

if (BUILD_TESTS)

add_executable (bench ${fftw_libbench2_SOURCE} tests/bench.c tests/hook.c tests/fftw-bench.c)

if (ENABLE_THREADS AND NOT WITH_COMBINED_THREADS)
target_link_libraries (bench ${fftw3_lib}_threads)
else ()
target_link_libraries (bench ${fftw3_lib})
endif ()


enable_testing ()

if (Threads_FOUND)

macro (fftw_add_test problem)
add_test (NAME ${problem} COMMAND bench -s ${problem})
endmacro ()

fftw_add_test (32x64)
fftw_add_test (ib256)

endif ()
endif ()

# pkgconfig file
set (prefix ${CMAKE_INSTALL_PREFIX})
set (exec_prefix ${CMAKE_INSTALL_PREFIX})
set (libdir ${CMAKE_INSTALL_FULL_LIBDIR})
set (includedir ${CMAKE_INSTALL_FULL_INCLUDEDIR})
set (VERSION ${FFTW_VERSION})
configure_file (fftw.pc.in fftw${PREC_SUFFIX}.pc @ONLY)
install (FILES
${CMAKE_CURRENT_BINARY_DIR}/fftw${PREC_SUFFIX}.pc
DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig
COMPONENT Development)

# cmake file
set (FFTW3_LIBRARIES "FFTW3::${fftw3_lib}")
configure_file (FFTW3Config.cmake.in FFTW3${PREC_SUFFIX}Config.cmake @ONLY)
configure_file (FFTW3ConfigVersion.cmake.in FFTW3${PREC_SUFFIX}ConfigVersion.cmake @ONLY)
install (FILES
${CMAKE_CURRENT_BINARY_DIR}/FFTW3${PREC_SUFFIX}Config.cmake
${CMAKE_CURRENT_BINARY_DIR}/FFTW3${PREC_SUFFIX}ConfigVersion.cmake
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/fftw3${PREC_SUFFIX}
COMPONENT Development)

export (TARGETS ${fftw3_lib} NAMESPACE FFTW3:: FILE ${PROJECT_BINARY_DIR}/FFTW3LibraryDepends.cmake)
install(EXPORT FFTW3LibraryDepends
NAMESPACE FFTW3::
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/fftw3${PREC_SUFFIX}
COMPONENT Development)