[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