[test-suite] Prevent timeit and fpcmp from being rebuilt every time make is called.

[mcrosier]

Summary

Internally, our build system has an option to build External tests in parallel by calling make per benchmark. Unfortunately, this causes a race condition because the timeit and fpcmp tools are built every time make is run (e.g., one instance of make tries to execute timeit, while another is rebuilding it).

This patch adds a custom_command level underneath the custom_targets for timeit and fpcmp to prevent the executables from being rebuilt every time make is called.

The custom_target rule will still be run every time, but the binaries will be detected as being up to date and skip rebuilding.

I’m a bit shaky on my CMake but I think this is the proper way to resolve the issue.

Please take a look.

Chad

[MatzeB]

My cmake-fu isn't that strong either. But using add_custom_command looks like the right thing to do. LGTM.

tools/CMakeLists.txt

1

Is this change necessary or just here by accident?

[mehdi_amini]

I probable lack context but it is not clear to me why we have custom_target to begin with? Why aren't fpcmp and timeit just declared with add_executable for which the dependency would be correctly tracked?

[MatzeB]

Because we need to compile then for the host machine, even if everything else is cross compiled to a different target machine.

[mehdi_amini]

I see, wouldn't the proper way would be to cross-compile the way we do for llvm-tblgen?

[MatzeB]

In D15938#320936, @joker.eph wrote:

I see, wouldn't the proper way would be to cross-compile the way we do for llvm-tblgen?

Yes that might very well be the better solution. http://reviews.llvm.org/D15004 stands at the same conclusion. I just couldn't get it to work in an hour and decided to put something in that works right now. I'd be happy if someone with more cmake experience would help out here.

[mcrosier]

tools/CMakeLists.txt

1

As far as I've read while trying to find a way to compile the same source in two ways, this change to the dependency for timeit-target is necessary, because cmake somehow loses the ability to find the source file in the same directory unless specifically shown where it is.

[mcrosier]

Committed in r257059.

tools/CMakeLists.txt

-add_executable(timeit-target timeit.c)
+add_executable(timeit-target ${CMAKE_CURRENT_SOURCE_DIR}/timeit.c)
 
 # FIXME: Replicate Makefile.tools's logic for determining whether to use fpcmp/fpcmp.sh
 set(TEST_SUITE_HOST_CC "cc" CACHE STRING "C compiler targetting the host")
 mark_as_advanced(TEST_SUITE_HOST_CC)
+
 add_custom_target(fpcmp
+    DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/fpcmp)
+add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/fpcmp
   COMMAND ${TEST_SUITE_HOST_CC} ${CMAKE_CURRENT_SOURCE_DIR}/fpcmp.c -o ${CMAKE_CURRENT_BINARY_DIR}/fpcmp
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/fpcmp.c
 )
+
 add_custom_target(timeit
+    DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/timeit)
+add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/timeit
   COMMAND ${TEST_SUITE_HOST_CC} ${CMAKE_CURRENT_SOURCE_DIR}/timeit.c -o ${CMAKE_CURRENT_BINARY_DIR}/timeit
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/timeit.c
 )