[PATCH] [compiler-rt][builtins] CTest to execute builtins testsuite
Faraz Shahbazker
Faraz.Shahbazker at imgtec.com
Fri Jul 11 00:24:58 PDT 2014
I don't mind the delay, please take your time. This is not exactly high priority.
Thanks for your comments & corrections, I agree with most. Few clarifications below.
================
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)
+
----------------
Alexey Samsonov wrote:
> Is it picked up by CMake automatically?
No. Gets picked on line 199 as ${{test}_EXTRA}
================
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})
----------------
Alexey Samsonov wrote:
> please check COMPILER_RT_CAN_EXECUTE_TESTS here instead - we don't need to add targets otherwise.
It is checked below for add_test(line 202). My plan was to be able to cross-build the test-suite even when we can't execute it. The dev environment is usually much faster and I think it might be useful to fix all build issues before moving to the target for execution.
If this doesn't seem like a good idea, can we just remove the check from this file and modify the upper test/CMakeLists.txt instead? It already has a COMPILER_RT_CAN_EXECUTE_TESTS block. Enclosing add_subdirectory in that block should give the desired effect and be consistent with the way sanitizer tests are controlled.
================
Comment at: test/builtins/CMakeLists.txt:214
@@ +213,3 @@
+ COMPILE_OPTIONS "-fexceptions"
+ LINK_LIBRARIES "-lstdc++")
+ set_target_properties(trampoline_setup PROPERTIES
----------------
Alexey Samsonov wrote:
> Why not LINK_FLAGS?
No hard reason - just following cmake docs. Linkers are known to be finicky about the order of command-line options. My guess is, to construct a correct invocation, cmake should be able to distinguish libraries from all other linker options.
http://reviews.llvm.org/D4251
More information about the llvm-commits
mailing list