Do not relink test targets every time in compiler-rt

[george.karpenkov]

Summary

CMake's add_custom_target is considered to be *always* out of date.
This patch changes it to a combination of add_custom_target and add_custom_command which actually tracks dependencies' timestamps.

On my machine this reliably saves 6-7 seconds on each test group.
This can be a large difference when debugging small tests.

[kcc]

Vitaly, plz take a look.
George, please describe how you've tested this change.

[george.karpenkov]

Fixed the diff, the previous one was the reverse.

@vitalybuka could you please test with a multi-arch build? I do not have that working on my Linux machine.
@kcc: I have run check-all, check-msan, check-tsan, check-asan on a fresh checkout on Linux and Mac.
I have run check-tsan repeatedly to make sure linking is not reperformed.

I have modified tsan code to make sure test fail on erroneous modifications, and then reverted the change, making sure they do go back to green.

[kcc]

testing sounds good, thanks!
Still want Vitaly to review.

[vitalybuka]

check-asan-dynamic does not work for me. however it does not work even on mast without the patch.

Does it work for you?

[george.karpenkov]

@vitalybuka it does, but as I've said, I couldn't get multi-architecture build working (my Linux machine produces only 64bit binaries)
Could you tell me at which revision did it stop working? From what I recall, it did use to work after the CMake-refactoring patch.

[vitalybuka]

In D36912#848001, @george.karpenkov wrote:

@vitalybuka it does, but as I've said, I couldn't get multi-architecture build working (my Linux machine produces only 64bit binaries)
Could you tell me at which revision did it stop working? From what I recall, it did use to work after the CMake-refactoring patch.

I am looking into this. Problem is that I've update my linux distro after the time I reviewed your refactoring. Maybe root case is there.
So lets assume we have no evidence that something is wrong with this patch.

[vitalybuka]

check-asan-dynamic works works with lld, with or without your patch. so maybe previously I had newer linker.

cmake/Modules/AddCompilerRT.cmake

function(add_compiler_rt_test test_suite test_name arch)
   # link programs at all are included, but the changes needed for the test
   # trump. With MSVC we can't do that because CMake is set up to run link.exe
   # when linking, not the compiler. Here, we hack it to use the compiler
   # because we want to use -fsanitize flags.
   if(NOT MSVC)
     set(TEST_LINK_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${TEST_LINK_FLAGS}")
     separate_arguments(TEST_LINK_FLAGS)
   endif()
-
-  add_custom_command(
-    OUTPUT "${output_bin}"
-    COMMAND ${COMPILER_RT_TEST_COMPILER} ${TEST_OBJECTS} -o "${output_bin}"
+  add_custom_target(${test_name}
+    COMMAND ${COMPILER_RT_TEST_COMPILER} ${TEST_OBJECTS}
+            -o "${output_bin}"
             ${TEST_LINK_FLAGS}
-    DEPENDS ${TEST_DEPS}
-    )
-  add_custom_target(T${test_name} DEPENDS "${output_bin}")
-  set_target_properties(T${test_name} PROPERTIES FOLDER "Compiler-RT Tests")
+    DEPENDS ${TEST_DEPS})
+  set_target_properties(${test_name} PROPERTIES FOLDER "Compiler-RT Tests")
 
   # Make the test suite depend on the binary.
-  add_dependencies(${test_suite} T${test_name})
+  add_dependencies(${test_suite} ${test_name})
 endfunction()
 
 macro(add_compiler_rt_resource_file target_name file_name component)
   set(src_file "${CMAKE_CURRENT_SOURCE_DIR}/${file_name}")
   set(dst_file "${COMPILER_RT_OUTPUT_DIR}/${file_name}")
   add_custom_command(OUTPUT ${dst_file}
     DEPENDS ${src_file}
     COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src_file} ${dst_file}