[PATCH] D13399: [CMake] Bug 14109 - CMake build for compiler-rt should use just-built clang

[Alexey Samsonov via cfe-commits]

samsonov added inline comments.

================
Comment at: runtime/CMakeLists.txt:33
@@ -32,1 +32,3 @@
+      )
+    set(cmake_3_4_USES_TERMINAL USES_TERMINAL 1)
   endif()
----------------
Should this also be named cmake_3_4_USES_TERMINAL_OPTIONS?

================
Comment at: runtime/CMakeLists.txt:40
@@ +39,3 @@
+  set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/compiler-rt-stamps/)
+  set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/compiler-rt-bins/)
+
----------------
This one is later re-defined from ExternalProject_Get_Property. Does it provide the same value there?

================
Comment at: runtime/CMakeLists.txt:42
@@ +41,3 @@
+
+  add_custom_target(compiler-rt-clear
+    DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/compiler-rt-cleared
----------------
Where do you use this target?

================
Comment at: runtime/CMakeLists.txt:47
@@ +46,3 @@
+    OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/compiler-rt-cleared
+    DEPENDS clang llvm-config
+    COMMAND ${CMAKE_COMMAND} -E remove_directory ${BINARY_DIR}
----------------
Why do you need these deps?

================
Comment at: runtime/CMakeLists.txt:71
@@ -49,2 +70,3 @@
+               -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
     INSTALL_COMMAND ""
     STEP_TARGETS configure build
----------------
Why did you remove -DCOMPILER_RT_ENABLE_WERROR=ON? I think it's fine to keep it enabled, as it's ok to ensure that ToT Clang builds ToT compiler-rt with no warnings.


http://reviews.llvm.org/D13399