[PATCH] D89492: [compiler-rt] Enable building builtins using top-level CMake file

[David Tenty via Phabricator via llvm-commits]

daltenty added a comment.

In D89492#2333051 <https://reviews.llvm.org/D89492#2333051>, @phosek wrote:

> This is an alternative to D70744 <https://reviews.llvm.org/D70744>. I'm not a fan of introducing compiler-rt/builtins as a new top-level CMake, so I was trying to come up with an alternative and this is what I came up with, let me know what you think.

I think this is an improvement, it was awkward to have two top-level entry points to the cmake anyway. This probably allows for some further de-duplication down the road as well.



================
Comment at: compiler-rt/cmake/builtin-config-ix.cmake:49
 endif()
 
 set(ALL_BUILTIN_SUPPORTED_ARCH
----------------



================
Comment at: compiler-rt/cmake/builtin-config-ix.cmake:200
+
+  if (CRT_SUPPORTED_ARCH AND OS_NAME MATCHES "Linux")
+    set(COMPILER_RT_HAS_CRT TRUE)
----------------
OS_NAME doesn't currently get set in builtin-config-ix, we'll need to duplicate the setup from config-ix.


================
Comment at: compiler-rt/cmake/builtin-config-ix.cmake:207
 
 message(STATUS "Builtin supported architectures: ${BUILTIN_SUPPORTED_ARCH}")
----------------
It would be nice to print the list of supported crt arch here as well.


================
Comment at: compiler-rt/lib/builtins/CMakeLists.txt:6
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  cmake_minimum_required(VERSION 3.4.3)
   project(CompilerRTBuiltins C ASM)
----------------
We probably should retain the new cmake minimum


================
Comment at: llvm/runtimes/CMakeLists.txt:415
                              # Builtins were built separately above
                              CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
                                         -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
----------------
We should probably disable the crt build here now.


================
Comment at: llvm/runtimes/CMakeLists.txt:527
                              # Builtins were built separately above
                              CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
                                         -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
----------------
Ditto the other comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89492/new/

https://reviews.llvm.org/D89492