[libcxx-commits] [PATCH] D88458: [CMake] Cache compiler-rt library results

[Saleem Abdulrasool via Phabricator via libcxx-commits]

compnerd added a comment.

If we require LLVM for any of the projects, perhaps we should hoist the `HandleCompilerRT.cmake` into LLVM and avoid having to maintain the copy everywhere.



================
Comment at: compiler-rt/cmake/Modules/HandleCompilerRT.cmake:6
 #    COMPILER_RT_LIBRARY-<name>-<target> to NOTFOUND
-function(cache_compiler_rt_library err_flag name target library_file)
+macro(cache_compiler_rt_library err_flag name target library_file)
   if(err_flag OR NOT EXISTS "${library_file}")
----------------
Why change this to a macro?  Its setting a cached variable, which should be possible from within a function right?


================
Comment at: libcxx/cmake/Modules/HandleCompilerRT.cmake:1
-function(find_compiler_rt_library name dest)
+macro(cache_compiler_rt_library err_flag name target library_file)
+  if(err_flag OR NOT EXISTS "${library_file}")
----------------
Please make this a `function` as well.


================
Comment at: libcxx/cmake/Modules/HandleCompilerRT.cmake:15
+  if(NOT CMAKE_CXX_COMPILER_ID MATCHES Clang)
+    set(${variable} "NOTFOUND" PARENT_SCOPE)
+    return()
----------------
I think that this deserves a comment explaining what is going on.  Using clang's runtime with gcc is possible.


================
Comment at: libcxx/cmake/Modules/HandleCompilerRT.cmake:21
   endif()
-  set(dest "" PARENT_SCOPE)
-  set(CLANG_COMMAND ${CMAKE_CXX_COMPILER} ${LIBCXX_COMPILE_FLAGS}
-      "--rtlib=compiler-rt" "--print-libgcc-file-name")
-  if (CMAKE_CXX_COMPILER_ID MATCHES Clang AND CMAKE_CXX_COMPILER_TARGET)
-    list(APPEND CLANG_COMMAND "--target=${CMAKE_CXX_COMPILER_TARGET}")
+  if (NOT target AND CMAKE_CXX_COMPILER_TARGET)
+    set(target "${CMAKE_CXX_COMPILER_TARGET}")
----------------
Where is `target` supposed to be defined?  This is tautologically `if(CMAKE_CXX_COMPILER_TARGET)` no?


================
Comment at: libcxxabi/cmake/Modules/HandleCompilerRT.cmake:1
-function(find_compiler_rt_library name dest)
+macro(cache_compiler_rt_library err_flag name target library_file)
+  if(err_flag OR NOT EXISTS "${library_file}")
----------------
Please make this a `function`


================
Comment at: libunwind/cmake/Modules/HandleCompilerRT.cmake:1
-function(find_compiler_rt_library name dest)
+macro(cache_compiler_rt_library err_flag name target library_file)
+  if(err_flag OR NOT EXISTS "${library_file}")
----------------
Same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88458