[PATCH] [compiler-rt][builtins] CTest to execute builtins testsuite
Alexey Samsonov
vonosmas at gmail.com
Thu Jul 10 16:54:22 PDT 2014
Once again, sorry for delay with reviewing this.
================
Comment at: test/builtins/CMakeLists.txt:1
@@ +1,2 @@
+# CTest for builtins Unit test-suite
+#
----------------
Please specify somewhere that you're actually using a host compiler to build these tests, and link them against a just-built builtins library.
================
Comment at: test/builtins/CMakeLists.txt:184
@@ +183,3 @@
+# For tests that rely on common(*_lib.h) headers
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../lib/builtins)
+
----------------
${COMPILER_RT_SOURCE_DIR}/lib/builtins
================
Comment at: test/builtins/CMakeLists.txt:187
@@ +186,3 @@
+# gcc personality needs an extra source file
+set(gcc_personality_EXTRA Unit/gcc_personality_test_helper.cxx)
+
----------------
Is it picked up by CMake automatically?
================
Comment at: test/builtins/CMakeLists.txt:193
@@ +192,3 @@
+foreach(arch x86_64 i386 arm ppc mips)
+ if(CAN_TARGET_${arch})
+ foreach (src ${GENERIC_SOURCES} ${${arch}_SOURCES})
----------------
please check COMPILER_RT_CAN_EXECUTE_TESTS here instead - we don't need to add targets otherwise.
================
Comment at: test/builtins/CMakeLists.txt:196
@@ +195,3 @@
+ # Strip leading directories and file-name suffix for executable
+ string(REGEX REPLACE "[^/]*/" "" test ${src})
+ string(REPLACE "_test.c" "" test ${test})
----------------
consider using get_filename_component instead.
================
Comment at: test/builtins/CMakeLists.txt:209
@@ +208,3 @@
+ set_target_properties(${test} PROPERTIES
+ LINK_FLAGS "-L${CMAKE_BINARY_DIR}/lib/${COMPILER_RT_OS_DIR}")
+ endforeach ()
----------------
Use ${COMPILER_RT_LIBRARY_OUTPUT_DIR} instead.
================
Comment at: test/builtins/CMakeLists.txt:214
@@ +213,3 @@
+ COMPILE_OPTIONS "-fexceptions"
+ LINK_LIBRARIES "-lstdc++")
+ set_target_properties(trampoline_setup PROPERTIES
----------------
Why not LINK_FLAGS?
http://reviews.llvm.org/D4251
More information about the llvm-commits
mailing list