[CMake] Switch the CMP0091 policy (MSVC_RUNTIME_LIBRARY) to the new behaviour

[mstorsjo]

Summary

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.

I'm tagging people who were involved in the previous reported
issues, @hans, @glandium, @dschuff - I'd appreciate if you could
test this in your build configs (I've tried to test the cases
mentioned).

Also @sunho, this touches the ORC runtime library on Windows,
can you verify that it still works as intended in your setups?

[glandium]

I can confirm this doesn't seem to break our builds.

[phosek]

LGTM

[hans]

Confirmed that Chromium's toolchain build works with this.

[dschuff]

Works for me, thanks for the heads-up.

[mstorsjo]

Thanks for testing! I'll go ahead and land this on Monday probably, and then follow up with a second patch to clean up the supposedly unnecessary messing around with the flags variables.

I didn't get the compiler-rt orc stuff tested yet though (the tests fail for me before this patch too - the tests end up looking for liborc_rt-x86_64.a while the build produced orc_rt-x86_64.a). But the changes in that area seem quite trivial.

[sunho]

LGTM. For ORC side failure, I'll take a look and submit a patch. I believe that shouldn't block this patch, as it seems to be failing long time ago.

[mstorsjo]

In D155233#4504021, @sunho wrote:

LGTM. For ORC side failure, I'll take a look and submit a patch. I believe that shouldn't block this patch, as it seems to be failing long time ago.

Thanks for having a look!

[smeenai]

compiler-rt/CMakeLists.txt

391–402

https://bugs.llvm.org/show_bug.cgi?id=20214 was marked as fixed back in 2014. I realized that we'd been building compiler-rt with /MD for years (our flags were set up in a way that wasn't caught by the flag replacement below, I guess), and it seems to be working fine. Should we make this user-configurable now?

[mstorsjo]

compiler-rt/CMakeLists.txt

391–402

Hmm, regarding ASAN, I think there's two separate concerns - which CRT the ASAN DLL itself is linked against (which is what this code here would affect) and what kind of CRT for the user code that ASAN intercepts. The referenced bug seems to be about the latter. So while the bug is fixed, it might still be the case that the sanitizer code needs to becompiled in one specific way. (But the comment indeed seems outdated.)

Curiosly, how did you pass /MD to the compiler so that it passed through unmodified here?

As for making it user-configurable - at least some parts of the compiler-rt libraries and/or their tests require this - see the original issue at https://github.com/llvm/llvm-project/issues/62719. If we can fix that issue at the root, to allow getting rid of this override here, that'd be nice. But I wouldn't be surprised if at least some of the sanitizer libraries require being compiled in a specific way.

[smeenai]

compiler-rt/CMakeLists.txt

391–402

It turns out to be kinda dumb :D In a (possibly misguided) attempt to minimize build worker disk usage, I'd only imported portions of the Windows SDK, and I omitted libcmt.lib since I didn't think it was needed anywhere. That meant that COMPILER_RT_HAS_MT_FLAG was false, which is why we kept the /MD. Seems like we've been skating on thin ice though and I should just import libcmt and go with the static linking.

cmake/Modules/CMakePolicy.cmake

 # 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)
   cmake_policy(SET CMP0114 OLD)
 endif()
 # CMP0116: Ninja generators transform `DEPFILE`s from `add_custom_command()`
 # New in CMake 3.20. https://cmake.org/cmake/help/latest/policy/CMP0116.html
 if(POLICY CMP0116)
   cmake_policy(SET CMP0116 OLD)
 endif()

compiler-rt/CMakeLists.txt

 # GCC versions.  The presence of those fragments, in particular for the
 # interceptors, changes backtraces seen in asan error cases, which causes
 # testsuite failures.
 if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "s390x")
   append_list_if(COMPILER_RT_HAS_FNO_PARTIAL_INLINING_FLAG -fno-partial-inlining SANITIZER_COMMON_CFLAGS)
 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.
+    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}}")
   append_list_if(COMPILER_RT_HAS_Oy_FLAG /Oy- SANITIZER_COMMON_CFLAGS)
   append_list_if(COMPILER_RT_HAS_GS_FLAG /GS- SANITIZER_COMMON_CFLAGS)
 
   # Disable thread safe initialization for static locals. ASan shouldn't need
   # it. Thread safe initialization assumes that the CRT has already been
   # initialized, but ASan initializes before the CRT.
   list(APPEND SANITIZER_COMMON_CFLAGS /Zc:threadSafeInit-)
 endif()

compiler-rt/lib/orc/CMakeLists.txt

if (WIN32)
 
     set(ORC_SOURCES
       ${ORC_COMMON_SOURCES}
       coff_platform.cpp
       coff_platform.per_jd.cpp
       )
 
     if (MSVC)
-      set(ORC_CFLAGS "${ORC_CFLAGS} /MD")
+      set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL")
     endif()
   else()
     set(ORC_BUILD_TYPE STATIC)
 
     set(ORC_IMPL_HEADERS
       ${ORC_COMMON_IMPL_HEADERS}
       elfnix_platform.h
       )

llvm/CMakeLists.txt

 set(LLVM_LIT_ARGS "${LIT_ARGS_DEFAULT}" CACHE STRING "Default options for lit")
 
 # On Win32 hosts, provide an option to specify the path to the GnuWin32 tools.
 if( WIN32 AND NOT CYGWIN )
   set(LLVM_LIT_TOOLS_DIR "" CACHE PATH "Path to GnuWin32 tools")
 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.")
   endif()
   if(LLVM_USE_SANITIZER)
     message(FATAL_ERROR "LLVM_INTEGRATED_CRT_ALLOC cannot be used along with LLVM_USE_SANITIZER!")
   endif()
   if(CMAKE_BUILD_TYPE AND uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")

llvm/cmake/modules/ChooseMSVCCRT.cmake

message(FATAL_ERROR
       "LLVM_USE_CRT is deprecated. Use the CMAKE_BUILD_TYPE-specific
 variables (LLVM_USE_CRT_DEBUG, etc) instead.")
   endif()
 
   make_crt_regex(MSVC_CRT_REGEX ${MSVC_CRT})
 
   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)
 
 
 # List of valid CRTs for MSVC
 set(MSVC_CRT
   MD
   MDd
   MT
   MTd)
 
 choose_msvc_crt(MSVC_CRT)
 

llvm/docs/CMake.rst

**LLVM_INTEGRATED_CRT_ALLOC**:PATH
   the respective allocator, for example:
 
   .. code-block:: console
 
     $ D:\git> git clone https://github.com/mjansson/rpmalloc
     $ 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
   either be absolute or relative to the *CMAKE_INSTALL_PREFIX*. Defaults to
   ``${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html``.
 
 **LLVM_LINK_LLVM_DYLIB**:BOOL
   If enabled, tools will be linked with the libLLVM shared library. Defaults

 **LLVM_UBSAN_FLAGS**:STRING
   Defines the set of compile flags used to enable UBSan. Only used if
   ``LLVM_USE_SANITIZER`` contains ``Undefined``. This can be used to override
   the default set of UBSan flags.
 
 **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.
 
 **LLVM_USE_LINKER**:STRING
   Add ``-fuse-ld={name}`` to the link invocation. The possible value depend on
   your compiler, for clang the value can be an absolute path to your custom
   linker, otherwise clang will prefix the name with ``ld.`` and apply its usual

llvm/lib/Support/CMakeLists.txt

 
 # Link Z3 if the user wants to build it.
 if(LLVM_WITH_Z3)
   set(system_libs ${system_libs} ${Z3_LIBRARIES})
 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}")
 
   if(NOT EXISTS "${LLVM_INTEGRATED_CRT_ALLOC}")
     message(FATAL_ERROR "Cannot find the path to `git clone` for the CRT allocator! (${LLVM_INTEGRATED_CRT_ALLOC}). Currently, rpmalloc, snmalloc and mimalloc are supported.")
   endif()