[PATCH] D46857: [CMake] Detect the compiler runtime and standard library

[Chris Bieneman via Phabricator via llvm-commits]

beanz accepted this revision.
beanz added a comment.
This revision is now accepted and ready to land.

This all looks reasonable to me. I had a few small comments below. There are a few isolated bits of code cleanup that you included in this patch, and I'd prefer to see those land separately.



================
Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:329
+  endif()
+  execute_process(
+      COMMAND ${COMPILER_COMMAND}
----------------
phosek wrote:
> vitalybuka wrote:
> > why not to use try_compile?
> In this case I'm only using `--print-libgcc-file-name` to find out what the runtime is so we don't need to compile anything. I could possibly completely drop this and just use check_link_libraries below and then see if output contains either `libclang_rt.builtins` or `libgcc`, the downside is that I'd need to iterate over all libraries in that list and try to `MATCH` each one of them against the regex which might be slower than actually invoking the compiler again. WDYT?
`--print-libgcc-file-name` is broken on Darwin (it returns an incorrect result). I suspect that this is actually alright because you're not actually using the file, but we should probably look into fixing that.


================
Comment at: compiler-rt/cmake/Modules/HandleCompilerRT.cmake:1
-function(find_compiler_rt_library name dest)
-  set(dest "" PARENT_SCOPE)
+function(find_compiler_rt_library name variable)
   set(CLANG_COMMAND ${CMAKE_CXX_COMPILER} ${SANITIZER_COMMON_CFLAGS}
----------------
This just looks like some generally good code cleanup. Can you put it in a separate patch? No review necessary.


================
Comment at: compiler-rt/cmake/config-ix.cmake:129
 # abilities.
-set(SIMPLE_SOURCE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/simple.cc)
+set(SIMPLE_SOURCE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/simple.c)
 file(WRITE ${SIMPLE_SOURCE} "#include <stdlib.h>\n#include <stdio.h>\nint main() { printf(\"hello, world\"); }\n")
----------------
This is also just a generally good fix. Can you also commit this separately?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D46857