From c368931b50a581f975a053ab0798666257532b77 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 23 Jan 2025 12:02:03 +0100 Subject: [PATCH 1/6] cmake: Increase minimum required version Setting the minimum required version to a value below 3.10 is deprecated in the current version. Hence, set the value to 3.10 in order to avoid deprecation warnings. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0b8e7c..b8a60a6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. See the AUTHORS file for names of contributors. -cmake_minimum_required(VERSION 3.1) +cmake_minimum_required(VERSION 3.10) project(Crc32c VERSION 1.1.0 LANGUAGES C CXX) # C standard can be overridden when this is used as a sub-project. From 6873b9537828fc2ec1ed905f58bd2dc6bf47158d Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 23 Jan 2025 12:09:48 +0100 Subject: [PATCH 2/6] cmake: Remove conditions from `else()` and `endif()` --- CMakeLists.txt | 66 +++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b8a60a6..2b9f498 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ if(NOT CMAKE_C_STANDARD) set(CMAKE_C_STANDARD 11) set(CMAKE_C_STANDARD_REQUIRED OFF) set(CMAKE_C_EXTENSIONS OFF) -endif(NOT CMAKE_C_STANDARD) +endif() # C++ standard can be overridden when this is used as a sub-project. if(NOT CMAKE_CXX_STANDARD) @@ -19,7 +19,7 @@ if(NOT CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) -endif(NOT CMAKE_CXX_STANDARD) +endif() # https://github.com/izenecloud/cmake/blob/master/SetCompilerWarningAll.cmake if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") @@ -27,9 +27,9 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") set(CMAKE_CXX_WARNING_LEVEL 4) if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - else(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") + else() set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4") - endif(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") + endif() # Disable C++ exceptions. string(REGEX REPLACE "/EH[a-z]+" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") @@ -39,21 +39,21 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") # Disable RTTI. string(REGEX REPLACE "/GR" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /GR-") -else(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") +else() # Use -Wall for clang and gcc. if(NOT CMAKE_CXX_FLAGS MATCHES "-Wall") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") - endif(NOT CMAKE_CXX_FLAGS MATCHES "-Wall") + endif() # Use -Wextra for clang and gcc. if(NOT CMAKE_CXX_FLAGS MATCHES "-Wextra") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra") - endif(NOT CMAKE_CXX_FLAGS MATCHES "-Wextra") + endif() # Use -Werror for clang and gcc. if(NOT CMAKE_CXX_FLAGS MATCHES "-Werror") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror") - endif(NOT CMAKE_CXX_FLAGS MATCHES "-Werror") + endif() # Disable C++ exceptions. string(REGEX REPLACE "-fexceptions" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") @@ -62,7 +62,7 @@ else(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") # Disable RTTI. string(REGEX REPLACE "-frtti" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-rtti") -endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") +endif() option(CRC32C_BUILD_TESTS "Build CRC32C's unit tests" ON) option(CRC32C_BUILD_BENCHMARKS "Build CRC32C's benchmarks" ON) @@ -115,9 +115,9 @@ int main() { set(OLD_CMAKE_REQURED_FLAGS ${CMAKE_REQUIRED_FLAGS}) if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} /arch:AVX") -else(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") +else() set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -msse4.2") -endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") +endif() check_cxx_source_compiles(" #if defined(_MSC_VER) #include @@ -194,21 +194,21 @@ if(CRC32C_USE_GLOG) # https://github.com/google/glog/issues/185 if(CRC32C_HAVE_NO_DEPRECATED) set_property(TARGET glog APPEND PROPERTY COMPILE_OPTIONS -Wno-deprecated) - endif(CRC32C_HAVE_NO_DEPRECATED) + endif() # glog triggers sign comparison warnings on gcc. if(CRC32C_HAVE_NO_SIGN_COMPARE) set_property(TARGET glog APPEND PROPERTY COMPILE_OPTIONS -Wno-sign-compare) - endif(CRC32C_HAVE_NO_SIGN_COMPARE) + endif() # glog triggers unused parameter warnings on clang. if(CRC32C_HAVE_NO_UNUSED_PARAMETER) set_property(TARGET glog APPEND PROPERTY COMPILE_OPTIONS -Wno-unused-parameter) - endif(CRC32C_HAVE_NO_UNUSED_PARAMETER) + endif() set(CRC32C_TESTS_BUILT_WITH_GLOG 1) -endif(CRC32C_USE_GLOG) +endif() configure_file( "src/crc32c_config.h.in" @@ -230,15 +230,15 @@ if(HAVE_ARM64_CRC32C) if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") # TODO(pwnall): Insert correct flag when VS gets ARM64 CRC32C support. target_compile_options(crc32c_arm64 PRIVATE "/arch:NOTYET") - else(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + else() target_compile_options(crc32c_arm64 PRIVATE "-march=armv8-a+crc+crypto") - endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") -endif(HAVE_ARM64_CRC32C) + endif() +endif() # CMake only enables PIC by default in SHARED and MODULE targets. if(BUILD_SHARED_LIBS) set_property(TARGET crc32c_arm64 PROPERTY POSITION_INDEPENDENT_CODE TRUE) -endif(BUILD_SHARED_LIBS) +endif() # SSE4.2 code is built separately, so we don't accidentally compile unsupported # instructions into code that gets run without SSE4.2 support. @@ -252,15 +252,15 @@ target_sources(crc32c_sse42 if(HAVE_SSE42) if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") target_compile_options(crc32c_sse42 PRIVATE "/arch:AVX") - else(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + else() target_compile_options(crc32c_sse42 PRIVATE "-msse4.2") - endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") -endif(HAVE_SSE42) + endif() +endif() # CMake only enables PIC by default in SHARED and MODULE targets. if(BUILD_SHARED_LIBS) set_property(TARGET crc32c_sse42 PROPERTY POSITION_INDEPENDENT_CODE TRUE) -endif(BUILD_SHARED_LIBS) +endif() # Must be included before CMAKE_INSTALL_INCLUDEDIR is used. include(GNUInstallDirs) @@ -304,7 +304,7 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") set_property(TARGET crc32c APPEND PROPERTY COMPILE_OPTIONS "/WX") set_property(TARGET crc32c_arm64 APPEND PROPERTY COMPILE_OPTIONS "/WX") set_property(TARGET crc32c_sse42 APPEND PROPERTY COMPILE_OPTIONS "/WX") -endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") +endif() if(CRC32C_BUILD_TESTS) enable_testing() @@ -323,7 +323,7 @@ if(CRC32C_BUILD_TESTS) APPEND PROPERTY COMPILE_OPTIONS -Wno-missing-field-initializers) set_property(TARGET gmock APPEND PROPERTY COMPILE_OPTIONS -Wno-missing-field-initializers) - endif(CRC32C_HAVE_NO_MISSING_FIELD_INITIALIZERS) + endif() add_executable(crc32c_tests "") target_sources(crc32c_tests @@ -344,11 +344,11 @@ if(CRC32C_BUILD_TESTS) # Warnings as errors in Visual Studio for this project's targets. if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") set_property(TARGET crc32c_tests APPEND PROPERTY COMPILE_OPTIONS "/WX") - endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + endif() if(CRC32C_USE_GLOG) target_link_libraries(crc32c_tests glog) - endif(CRC32C_USE_GLOG) + endif() add_test(NAME crc32c_tests COMMAND crc32c_tests) @@ -368,10 +368,10 @@ if(CRC32C_BUILD_TESTS) # workaround can be removed once the Windows SDK on our CI is upgraded. set_property(TARGET crc32c_capi_tests APPEND PROPERTY COMPILE_OPTIONS "/wd5105") - endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + endif() add_test(NAME crc32c_capi_tests COMMAND crc32c_capi_tests) -endif(CRC32C_BUILD_TESTS) +endif() if(CRC32C_BUILD_BENCHMARKS) add_executable(crc32c_bench "") @@ -390,13 +390,13 @@ if(CRC32C_BUILD_BENCHMARKS) if(CRC32C_USE_GLOG) target_link_libraries(crc32c_bench glog) - endif(CRC32C_USE_GLOG) + endif() # Warnings as errors in Visual Studio for this project's targets. if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") set_property(TARGET crc32c_bench APPEND PROPERTY COMPILE_OPTIONS "/WX") - endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") -endif(CRC32C_BUILD_BENCHMARKS) + endif() +endif() if(CRC32C_INSTALL) install(TARGETS crc32c @@ -432,4 +432,4 @@ if(CRC32C_INSTALL) "${PROJECT_BINARY_DIR}/${PROJECT_NAME}ConfigVersion.cmake" DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}" ) -endif(CRC32C_INSTALL) +endif() From 41c8ae23bf3d811051e55e8107153fae9c936efc Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 23 Jan 2025 12:16:33 +0100 Subject: [PATCH 3/6] cmake: Use `string(APPEND)` --- CMakeLists.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2b9f498..2505397 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,40 +28,40 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") if(CMAKE_CXX_FLAGS MATCHES "/W[0-4]") string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") else() - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4") + string(APPEND CMAKE_CXX_FLAGS " /W4") endif() # Disable C++ exceptions. string(REGEX REPLACE "/EH[a-z]+" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /EHs-c-") + string(APPEND CMAKE_CXX_FLAGS " /EHs-c-") add_definitions(-D_HAS_EXCEPTIONS=0) # Disable RTTI. string(REGEX REPLACE "/GR" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /GR-") + string(APPEND CMAKE_CXX_FLAGS " /GR-") else() # Use -Wall for clang and gcc. if(NOT CMAKE_CXX_FLAGS MATCHES "-Wall") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") + string(APPEND CMAKE_CXX_FLAGS " -Wall") endif() # Use -Wextra for clang and gcc. if(NOT CMAKE_CXX_FLAGS MATCHES "-Wextra") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra") + string(APPEND CMAKE_CXX_FLAGS " -Wextra") endif() # Use -Werror for clang and gcc. if(NOT CMAKE_CXX_FLAGS MATCHES "-Werror") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror") + string(APPEND CMAKE_CXX_FLAGS " -Werror") endif() # Disable C++ exceptions. string(REGEX REPLACE "-fexceptions" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions") + string(APPEND CMAKE_CXX_FLAGS " -fno-exceptions") # Disable RTTI. string(REGEX REPLACE "-frtti" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-rtti") + string(APPEND CMAKE_CXX_FLAGS " -fno-rtti") endif() option(CRC32C_BUILD_TESTS "Build CRC32C's unit tests" ON) From 022ca16c52a20d65ae3a8d13a889a83e037cb777 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 23 Jan 2025 13:34:12 +0100 Subject: [PATCH 4/6] cmake: Set install includes destination Specify the `INCLUDES DESTINATION` in the `install(TARGETS)` command instead of setting a public include directory. The effect is the same. The new approach has the advantage that including `GNUInstallDirs` can be deferred. --- CMakeLists.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2505397..3d90ebd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -262,9 +262,6 @@ if(BUILD_SHARED_LIBS) set_property(TARGET crc32c_sse42 PROPERTY POSITION_INDEPENDENT_CODE TRUE) endif() -# Must be included before CMAKE_INSTALL_INCLUDEDIR is used. -include(GNUInstallDirs) - add_library(crc32c "" # TODO(pwnall): Move the TARGET_OBJECTS generator expressions to the PRIVATE # section of target_sources when cmake_minimum_required becomes 3.9 or above. @@ -293,7 +290,6 @@ target_sources(crc32c target_include_directories(crc32c PUBLIC $ - $ ) set_target_properties(crc32c @@ -399,11 +395,13 @@ if(CRC32C_BUILD_BENCHMARKS) endif() if(CRC32C_INSTALL) + include(GNUInstallDirs) install(TARGETS crc32c EXPORT Crc32cTargets RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} + INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} ) install( FILES From c5d6576c57f21f8f3e347faaed86543a712a814f Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 23 Jan 2025 14:32:28 +0100 Subject: [PATCH 5/6] cmake: Remove `include_directories()` call Make sure that the include directory is added to the targets that need it, rather than all targets in the current directory. --- CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3d90ebd..09e0e8d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -215,8 +215,6 @@ configure_file( "${PROJECT_BINARY_DIR}/include/crc32c/crc32c_config.h" ) -include_directories("${PROJECT_BINARY_DIR}/include") - # ARM64 CRC32C code is built separately, so we don't accidentally compile # unsupported instructions into code that gets run without ARM32 support. add_library(crc32c_arm64 OBJECT "") @@ -289,6 +287,7 @@ target_sources(crc32c target_include_directories(crc32c PUBLIC + $ $ ) From 5fad7e71ee27f3b1431cd0099ef45e0356e1f49c Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 23 Jan 2025 13:23:43 +0100 Subject: [PATCH 6/6] cmake: Remove object libraries Make sure that architecture specific compile flags are specified on the source file level rather than at the target level. This removes the need for object libraries. --- CMakeLists.txt | 86 ++++++++++++++++---------------------------------- 1 file changed, 28 insertions(+), 58 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 09e0e8d..a47a864 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -215,76 +215,48 @@ configure_file( "${PROJECT_BINARY_DIR}/include/crc32c/crc32c_config.h" ) -# ARM64 CRC32C code is built separately, so we don't accidentally compile -# unsupported instructions into code that gets run without ARM32 support. -add_library(crc32c_arm64 OBJECT "") -target_sources(crc32c_arm64 - PRIVATE - "${PROJECT_BINARY_DIR}/include/crc32c/crc32c_config.h" - "src/crc32c_arm64.cc" - "src/crc32c_arm64.h" +add_library(crc32c + "${PROJECT_BINARY_DIR}/include/crc32c/crc32c_config.h" + "include/crc32c/crc32c.h" + "src/crc32c_arm64.cc" + "src/crc32c_arm64_check.h" + "src/crc32c_arm64.h" + "src/crc32c.cc" + "src/crc32c_internal.h" + "src/crc32c_portable.cc" + "src/crc32c_prefetch.h" + "src/crc32c_read_le.h" + "src/crc32c_round_up.h" + "src/crc32c_sse42.cc" + "src/crc32c_sse42_check.h" + "src/crc32c_sse42.h" ) + if(HAVE_ARM64_CRC32C) if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") # TODO(pwnall): Insert correct flag when VS gets ARM64 CRC32C support. - target_compile_options(crc32c_arm64 PRIVATE "/arch:NOTYET") + set_property(SOURCE "src/crc32c_arm64.cc" PROPERTY + COMPILE_OPTIONS "/arch:NOTYET" + ) else() - target_compile_options(crc32c_arm64 PRIVATE "-march=armv8-a+crc+crypto") + set_property(SOURCE "src/crc32c_arm64.cc" PROPERTY + COMPILE_OPTIONS "-march=armv8-a+crc+crypto" + ) endif() endif() -# CMake only enables PIC by default in SHARED and MODULE targets. -if(BUILD_SHARED_LIBS) - set_property(TARGET crc32c_arm64 PROPERTY POSITION_INDEPENDENT_CODE TRUE) -endif() - -# SSE4.2 code is built separately, so we don't accidentally compile unsupported -# instructions into code that gets run without SSE4.2 support. -add_library(crc32c_sse42 OBJECT "") -target_sources(crc32c_sse42 - PRIVATE - "${PROJECT_BINARY_DIR}/include/crc32c/crc32c_config.h" - "src/crc32c_sse42.cc" - "src/crc32c_sse42.h" -) if(HAVE_SSE42) if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") - target_compile_options(crc32c_sse42 PRIVATE "/arch:AVX") + set_property(SOURCE "src/crc32c_sse42.cc" PROPERTY + COMPILE_OPTIONS "/arch:AVX" + ) else() - target_compile_options(crc32c_sse42 PRIVATE "-msse4.2") + set_property(SOURCE "src/crc32c_sse42.cc" PROPERTY + COMPILE_OPTIONS "-msse4.2" + ) endif() endif() -# CMake only enables PIC by default in SHARED and MODULE targets. -if(BUILD_SHARED_LIBS) - set_property(TARGET crc32c_sse42 PROPERTY POSITION_INDEPENDENT_CODE TRUE) -endif() - -add_library(crc32c "" - # TODO(pwnall): Move the TARGET_OBJECTS generator expressions to the PRIVATE - # section of target_sources when cmake_minimum_required becomes 3.9 or above. - $ - $ -) -target_sources(crc32c - PRIVATE - "${PROJECT_BINARY_DIR}/include/crc32c/crc32c_config.h" - "src/crc32c_arm64.h" - "src/crc32c_arm64_check.h" - "src/crc32c_internal.h" - "src/crc32c_portable.cc" - "src/crc32c_prefetch.h" - "src/crc32c_read_le.h" - "src/crc32c_round_up.h" - "src/crc32c_sse42.h" - "src/crc32c_sse42_check.h" - "src/crc32c.cc" - - # Only CMake 3.3+ supports PUBLIC sources in targets exported by "install". - $<$:PUBLIC> - "include/crc32c/crc32c.h" -) - target_include_directories(crc32c PUBLIC $ @@ -297,8 +269,6 @@ set_target_properties(crc32c # Warnings as errors in Visual Studio for this project's targets. if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") set_property(TARGET crc32c APPEND PROPERTY COMPILE_OPTIONS "/WX") - set_property(TARGET crc32c_arm64 APPEND PROPERTY COMPILE_OPTIONS "/WX") - set_property(TARGET crc32c_sse42 APPEND PROPERTY COMPILE_OPTIONS "/WX") endif() if(CRC32C_BUILD_TESTS)