Skip to content

Commit

Permalink
[CMake] Switch the CMP0091 policy (MSVC_RUNTIME_LIBRARY) to the new b…
Browse files Browse the repository at this point in the history
…ehaviour

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
#63286, and should
handle the issues that were observed originally when the
minimum CMake version was bumped, in
#62719 and
#62739.

Differential Revision: https://reviews.llvm.org/D155233
  • Loading branch information
mstorsjo committed Jul 17, 2023
1 parent f69a9f3 commit c6bd873
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 45 deletions.
5 changes: 0 additions & 5 deletions cmake/Modules/CMakePolicy.cmake
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
25 changes: 12 additions & 13 deletions compiler-rt/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/lib/orc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion llvm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
41 changes: 20 additions & 21 deletions llvm/cmake/modules/ChooseMSVCCRT.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions llvm/docs/CMake.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Support/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down

0 comments on commit c6bd873

Please sign in to comment.