From c6bd873403a8ac6538a3fe3b3c2fe39c16b146a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Wed, 12 Jul 2023 12:40:37 +0000 Subject: [PATCH] [CMake] Switch the CMP0091 policy (MSVC_RUNTIME_LIBRARY) to the new behaviour With the new behaviour, the /MD or similar options aren't added to e.g. CMAKE_CXX_FLAGS_RELEASE, but are added separately by CMake. They can be changed by the cmake variable CMAKE_MSVC_RUNTIME_LIBRARY or with the target property MSVC_RUNTIME_LIBRARY. LLVM has had its own custom CMake flags, e.g. LLVM_USE_CRT_RELEASE, which affects which CRT is used for release mode builds. Deprecate these and direct users to use CMAKE_MSVC_RUNTIME_LIBRARY directly instead (and do a best effort attempt at setting CMAKE_MSVC_RUNTIME_LIBRARY based on the existing LLVM_USE_CRT_ flags). This only handles the simple cases, it doesn't handle multi-config generators with different LLVM_USE_CRT_* variables for different configs though, but that's probably fine - we should move over to the new upstream CMake mechanism anyway, and push users towards that. Change code in compiler-rt, that previously tried to override the CRT choice to /MT, to set CMAKE_MSVC_RUNTIME_LIBRARY instead of meddling in the old variables. This resolves the policy issue in https://github.com/llvm/llvm-project/issues/63286, and should handle the issues that were observed originally when the minimum CMake version was bumped, in https://github.com/llvm/llvm-project/issues/62719 and https://github.com/llvm/llvm-project/issues/62739. Differential Revision: https://reviews.llvm.org/D155233 --- cmake/Modules/CMakePolicy.cmake | 5 ---- compiler-rt/CMakeLists.txt | 25 ++++++++-------- compiler-rt/lib/orc/CMakeLists.txt | 2 +- llvm/CMakeLists.txt | 2 +- llvm/cmake/modules/ChooseMSVCCRT.cmake | 41 +++++++++++++------------- llvm/docs/CMake.rst | 5 ++-- llvm/lib/Support/CMakeLists.txt | 4 +-- 7 files changed, 39 insertions(+), 45 deletions(-) diff --git a/cmake/Modules/CMakePolicy.cmake b/cmake/Modules/CMakePolicy.cmake index 8a3445c40cb5..0ec32ad8637f 100644 --- a/cmake/Modules/CMakePolicy.cmake +++ b/cmake/Modules/CMakePolicy.cmake @@ -1,10 +1,5 @@ # CMake policy settings shared between LLVM projects -# CMP0091: MSVC runtime library flags are selected by an abstraction. -# New in CMake 3.15. https://cmake.org/cmake/help/latest/policy/CMP0091.html -if(POLICY CMP0091) - cmake_policy(SET CMP0091 OLD) -endif() # CMP0114: ExternalProject step targets fully adopt their steps. # New in CMake 3.19: https://cmake.org/cmake/help/latest/policy/CMP0114.html if(POLICY CMP0114) diff --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt index d004474028ab..fd0430e76c7e 100644 --- a/compiler-rt/CMakeLists.txt +++ b/compiler-rt/CMakeLists.txt @@ -387,20 +387,19 @@ if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "s390x") endif() if(MSVC) - # Replace the /M[DT][d] flags with /MT, and strip any definitions of _DEBUG, - # which cause definition mismatches at link time. # FIXME: In fact, sanitizers should support both /MT and /MD, see PR20214. - if(COMPILER_RT_HAS_MT_FLAG) - foreach(flag_var - CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE - CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO - CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE - CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO) - string(REGEX REPLACE "/M[DT]d" "/MT" ${flag_var} "${${flag_var}}") - string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}") - string(REGEX REPLACE "/D_DEBUG" "" ${flag_var} "${${flag_var}}") - endforeach() - endif() + set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded) + # Remove any /M[DT][d] flags, and strip any definitions of _DEBUG. + # TODO: We probably could remove this altogether. + foreach(flag_var + CMAKE_C_FLAGS CMAKE_C_FLAGS_DEBUG CMAKE_C_FLAGS_RELEASE + CMAKE_C_FLAGS_MINSIZEREL CMAKE_C_FLAGS_RELWITHDEBINFO + CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE + CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO) + string(REGEX REPLACE "[/-]M[DT]d" "" ${flag_var} "${${flag_var}}") + string(REGEX REPLACE "[/-]MD" "" ${flag_var} "${${flag_var}}") + string(REGEX REPLACE "[/-]D_DEBUG" "" ${flag_var} "${${flag_var}}") + endforeach() append_list_if(COMPILER_RT_HAS_Oy_FLAG /Oy- SANITIZER_COMMON_CFLAGS) append_list_if(COMPILER_RT_HAS_GS_FLAG /GS- SANITIZER_COMMON_CFLAGS) diff --git a/compiler-rt/lib/orc/CMakeLists.txt b/compiler-rt/lib/orc/CMakeLists.txt index 7943f3ff3374..38dd233f5e33 100644 --- a/compiler-rt/lib/orc/CMakeLists.txt +++ b/compiler-rt/lib/orc/CMakeLists.txt @@ -131,7 +131,7 @@ else() # not Apple ) if (MSVC) - set(ORC_CFLAGS "${ORC_CFLAGS} /MD") + set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL") endif() else() set(ORC_BUILD_TYPE STATIC) diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 0a7a9fd7858f..1a9ad06bdf8b 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -684,7 +684,7 @@ if( WIN32 AND NOT CYGWIN ) endif() set(LLVM_NATIVE_TOOL_DIR "" CACHE PATH "Path to a directory containing prebuilt matching native tools (such as llvm-tblgen)") -set(LLVM_INTEGRATED_CRT_ALLOC "" CACHE PATH "Replace the Windows CRT allocator with any of {rpmalloc|mimalloc|snmalloc}. Only works with /MT enabled.") +set(LLVM_INTEGRATED_CRT_ALLOC "" CACHE PATH "Replace the Windows CRT allocator with any of {rpmalloc|mimalloc|snmalloc}. Only works with CMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded.") if(LLVM_INTEGRATED_CRT_ALLOC) if(NOT WIN32) message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC is only supported on Windows.") diff --git a/llvm/cmake/modules/ChooseMSVCCRT.cmake b/llvm/cmake/modules/ChooseMSVCCRT.cmake index 54bc379bd0c4..0305b6209985 100644 --- a/llvm/cmake/modules/ChooseMSVCCRT.cmake +++ b/llvm/cmake/modules/ChooseMSVCCRT.cmake @@ -62,33 +62,32 @@ variables (LLVM_USE_CRT_DEBUG, etc) instead.") foreach(build_type ${CMAKE_CONFIGURATION_TYPES} ${CMAKE_BUILD_TYPE}) string(TOUPPER "${build_type}" build) - if (NOT LLVM_USE_CRT_${build}) - get_current_crt(LLVM_USE_CRT_${build} - MSVC_CRT_REGEX - CMAKE_CXX_FLAGS_${build}) - set(LLVM_USE_CRT_${build} - "${LLVM_USE_CRT_${build}}" - CACHE STRING "Specify VC++ CRT to use for ${build_type} configurations." - FORCE) - set_property(CACHE LLVM_USE_CRT_${build} - PROPERTY STRINGS ;${${MSVC_CRT}}) - endif(NOT LLVM_USE_CRT_${build}) - endforeach(build_type) - - foreach(build_type ${CMAKE_CONFIGURATION_TYPES} ${CMAKE_BUILD_TYPE}) - string(TOUPPER "${build_type}" build) - if ("${LLVM_USE_CRT_${build}}" STREQUAL "") - set(flag_string " ") - else() - set(flag_string " /${LLVM_USE_CRT_${build}} ") + if (NOT "${LLVM_USE_CRT_${build}}" STREQUAL "") if (NOT ${LLVM_USE_CRT_${build}} IN_LIST ${MSVC_CRT}) message(FATAL_ERROR "Invalid value for LLVM_USE_CRT_${build}: ${LLVM_USE_CRT_${build}}. Valid options are one of: ${${MSVC_CRT}}") endif() - message(STATUS "Using ${build_type} VC++ CRT: ${LLVM_USE_CRT_${build}}") + set(library "MultiThreaded") + if ("${LLVM_USE_CRT_${build}}" MATCHES "d$") + set(library "${library}Debug") + endif() + if ("${LLVM_USE_CRT_${build}}" MATCHES "^MD") + set(library "${library}DLL") + endif() + if(${runtime_library_set}) + message(WARNING "Conflicting LLVM_USE_CRT_* options") + else() + message(WARNING "The LLVM_USE_CRT_* options are deprecated, use the CMake provided CMAKE_MSVC_RUNTIME_LIBRARY setting instead") + endif() + set(CMAKE_MSVC_RUNTIME_LIBRARY "${library}" CACHE STRING "" FORCE) + message(STATUS "Using VC++ CRT: ${CMAKE_MSVC_RUNTIME_LIBRARY}") + set(runtime_library_set 1) endif() foreach(lang C CXX) - set_flag_in_var(CMAKE_${lang}_FLAGS_${build} MSVC_CRT_REGEX flag_string) + # Clear any potentially manually set options from these variables. + # Kept as temporary backwards compat (unsure if necessary). + # TODO: We probably should remove it. + set_flag_in_var(CMAKE_${lang}_FLAGS_${build} MSVC_CRT_REGEX "") endforeach(lang) endforeach(build_type) endmacro(choose_msvc_crt MSVC_CRT) diff --git a/llvm/docs/CMake.rst b/llvm/docs/CMake.rst index 67827bf46a51..c566b590a584 100644 --- a/llvm/docs/CMake.rst +++ b/llvm/docs/CMake.rst @@ -680,7 +680,7 @@ enabled sub-projects. Nearly all of these variable names begin with $ D:\llvm-project> cmake ... -DLLVM_INTEGRATED_CRT_ALLOC=D:\git\rpmalloc This flag needs to be used along with the static CRT, ie. if building the - Release target, add -DLLVM_USE_CRT_RELEASE=MT. + Release target, add -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded. **LLVM_INSTALL_DOXYGEN_HTML_DIR**:STRING The path to install Doxygen-generated HTML documentation to. This path can @@ -768,7 +768,8 @@ enabled sub-projects. Nearly all of these variable names begin with **LLVM_USE_CRT_{target}**:STRING On Windows, tells which version of the C runtime library (CRT) should be used. For example, -DLLVM_USE_CRT_RELEASE=MT would statically link the CRT into the - LLVM tools and library. + LLVM tools and library. This is deprecated; use ``CMAKE_MSVC_RUNTIME_LIBRARY`` + instead. **LLVM_USE_INTEL_JITEVENTS**:BOOL Enable building support for Intel JIT Events API. Defaults to OFF. diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt index 976ae88d37c4..87fe7bebf688 100644 --- a/llvm/lib/Support/CMakeLists.txt +++ b/llvm/lib/Support/CMakeLists.txt @@ -89,8 +89,8 @@ endif() # Override the C runtime allocator on Windows and embed it into LLVM tools & libraries if(LLVM_INTEGRATED_CRT_ALLOC) - if (CMAKE_BUILD_TYPE AND NOT ${LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE}} MATCHES "^(MT|MTd)$") - message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC only works with /MT or /MTd. Use LLVM_USE_CRT_${uppercase_CMAKE_BUILD_TYPE} to set the appropriate option.") + if (NOT CMAKE_MSVC_RUNTIME_LIBRARY OR CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "DLL$") + message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC only works with CMAKE_MSVC_RUNTIME_LIBRARY set to MultiThreaded or MultiThreadedDebug.") endif() string(REGEX REPLACE "(/|\\\\)$" "" LLVM_INTEGRATED_CRT_ALLOC "${LLVM_INTEGRATED_CRT_ALLOC}")