[PATCH] D14513: [CMake] Add support for building the llvm test-suite as part of an LLVM build using clang and lld

[James Molloy via llvm-commits]

jmolloy requested changes to this revision.
jmolloy added a reviewer: jmolloy.
jmolloy added a comment.
This revision now requires changes to proceed.

Hi Chris,

This is awesome! This is so much better than the approach I was taking for test-suite (recursion, like the cross-compile build does). I have a few comments.

Cheers,

James


================
Comment at: cmake/modules/LLVMExternalProjectUtils.cmake:7
@@ +6,3 @@
+  if (CMAKE_GENERATOR MATCHES "Make")
+    # Use special command for Makefiles to support parallelism.
+    set(${out_var} "$(MAKE)" "${target}" PARENT_SCOPE)
----------------
Is there a ninja equivalent, do you know?

================
Comment at: cmake/modules/LLVMExternalProjectUtils.cmake:45
@@ +44,3 @@
+  endforeach()
+  if(TARGET compiler-rt AND CLANG_IN_TOOLCHAIN)
+    list(APPEND TOOLCHAIN_BINS compiler-rt)
----------------
It's confusing that this is testing CLANG_IN_TOOLCHAIN before it is set. Is this deliberate? if so it could do with some comments.

================
Comment at: cmake/modules/LLVMExternalProjectUtils.cmake:77
@@ +76,3 @@
+    COMMAND ${CMAKE_COMMAND} -E remove_directory ${STAMP_DIR}
+    COMMENT "Clobberring ${name} build and stamp directories"
+    ${cmake_3_2_USES_TERMINAL}
----------------
Clobbering*

================
Comment at: cmake/modules/LLVMExternalProjectUtils.cmake:105
@@ +104,3 @@
+    COMMAND ${CMAKE_COMMAND} -E touch ${CMAKE_CURRENT_BINARY_DIR}/${name}-clobber-stamp
+    COMMENT "Clobberring bootstrap build and stamp directories"
+    )
----------------
Clobbering*


http://reviews.llvm.org/D14513