[test-suite] Bitcode tests: Update cmake to build driver and halide runtime only once.

[asbirlea]

Summary

The simd_ops bitcode tests used to build the common driver and the halide runtime for each test.
Update cmake so they are build just once.
Note that in the future the halide runtime could be taken up one directory level and used in common for
multiple tests and benchmark. However building just the one .bc file into a static library failed on OSX.

[MatzeB]

Looks good to me when the code duplication issue is addressed:

Bitcode/simd_ops/CMakeLists.txt

9 ↗

(On Diff #66684)

You can probably simplify this by leaving out the ${CMAKE_CURRENT_SOURCE_DIR}/ prefixes.

11–14 ↗

(On Diff #66684)

Could you factor this code into a helper function (test_suite_add_build_dependencies(target) maybe?) so we avoid the code duplication with SingleMultiSource.cmake?

[asbirlea]

Address comments.
@MatzeB: Could you please lgtm the change?

[MatzeB]

I already wanted to express with my first message that you can address the issue as you see fit and I am fine with it. Anyway here's an explicit LGTM!

[asbirlea]

@mehdi_amini: Do you have any insight into why having a single .bc into a static library is not allowed on OSX and how/if this can be bypassed?
The link error I see is "ar: no archive members specified", which I found comes up if you try something like adding a single header into a static library and force it to use CXX linker language, just like I did for the .bc.
I haven't found a solution to go around this, and it would be nice to factor out building the halide runtime between all bitcode tests coming from the Halide suite.

[MatzeB]

In D23127#505111, @asbirlea wrote:

@mehdi_amini: Do you have any insight into why having a single .bc into a static library is not allowed on OSX and how/if this can be bypassed?
The link error I see is "ar: no archive members specified", which I found comes up if you try something like adding a single header into a static library and force it to use CXX linker language, just like I did for the .bc.
I haven't found a solution to go around this, and it would be nice to factor out building the halide runtime between all bitcode tests coming from the Halide suite.

Not sure if that is what you are discussing here, but maybe the macOS build currently fails because function names are mangled with a "_" prefix, will this work correctly when you link with generic .bc files (constructed on linux I presume)?

[asbirlea]

Hmm, not the issue I was referring to right now. Looks like cmake cannot have just the .bc file in a static lib, it needs to go together with a .cpp file, then things work fine on both linux and osx.
However, it may be the linking issue we've been seeing on osx aarch64 green dragon, where the methods _*halide_name* are not found, and they do exist as *halide_name* in the halide runtime. I thought that issue is more likely related to the fact that the bot command line seems to not have the runtime file present at all, than with the name mangling...

[mehdi_amini]

In D23127#505111, @asbirlea wrote:

@mehdi_amini: Do you have any insight into why having a single .bc into a static library is not allowed on OSX and how/if this can be bypassed?

It is allowed:

echo "" | clang -x c - -c -emit-llvm -o test.o
ar -rv test.a test.o
ar: creating archive test.a
a - test.o

(Now of course, if you're using ToT LLVM/Clang, you need to set DYLD_LIBRARY_PATH to points to your libLTO.dylib, there is no way the system ar can have support for "future" bitcode )

The link error I see is "ar: no archive members specified", which I found comes up if you try something like adding a single header into a static library and force it to use CXX linker language, just like I did for the .bc.

Can you post the command you're using?

[asbirlea]

I've edited the cmake file to create two libraries:

add_library(simd_ops STATIC simd_ops.cpp)
add_library(halide_runtime STATIC ${ARCH}_halide_runtime.bc)

And duplicated all the current lines using simd_ops, with halide_runtime.
Running ninja test-suite gives the ar link error.

*Unintentionally early comment submit*

[asbirlea]

Relevant cmake part:

add_library(simd_ops STATIC simd_ops.cpp)
target_link_libraries(simd_ops)
test_suite_add_build_dependencies(simd_ops)

add_library(halide_runtime STATIC ${ARCH}_halide_runtime.bc)
target_link_libraries(halide_runtime)
test_suite_add_build_dependencies(halide_runtime)
set_target_properties(halide_runtime PROPERTIES LINKER_LANGUAGE CXX)

foreach(sourcebc ${uosources})
  string(REGEX REPLACE ".[cp]+$" "" pathbc ${sourcebc})
  string(REGEX REPLACE ".*/" "" namebc ${pathbc})
  string(REPLACE "." "" namebc ${namebc})
  set(Source ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_tests/${namebc}.bc ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_scalar_tests/scalar_${namebc}.bc)
  set(PROG simd_ops_${namebc})
  llvm_multisource()
  target_link_libraries(${PROG} halide_runtime)
  target_link_libraries(${PROG} simd_ops)
endforeach()

[asbirlea]

I'll land this today in the current format, but I'd like to follow up on the right way to create a library from a bitcode file.

Complete error on OSX:

Linking CXX static library Bitcode/simd_ops/libhalide_runtime.a
FAILED: : && /usr/local/Cellar/cmake/3.5.2/bin/cmake -E remove Bitcode/simd_ops/libhalide_runtime.a && /usr/bin/ar qc Bitcode/simd_ops/libhalide_runtime.a   && /usr/bin/ranlib Bitcode/simd_ops/libhalide_runtime.a && :
ar: no archive members specified

@mehdi_amini: Looks like what's missing is the "-rv" argument from your example?

test-suite/trunk/Bitcode/simd_ops/CMakeLists.txt

 set(NO_REFERENCE_OUTPUT true)
 file(GLOB uosources ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_tests/*.bc)
 SET_SOURCE_FILES_PROPERTIES(${uosources} PROPERTIES LANGUAGE CXX)
 file(GLOB scalar_sources ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_scalar_tests/*.bc)
 SET_SOURCE_FILES_PROPERTIES(${scalar_sources} PROPERTIES LANGUAGE CXX)
 SET_SOURCE_FILES_PROPERTIES(${CMAKE_CURRENT_SOURCE_DIR}/simd_op_check_runtime.bc PROPERTIES LANGUAGE CXX)
 
 list(APPEND LDFLAGS -lpthread -ldl)
+add_library(simd_ops STATIC simd_ops.cpp ${ARCH}_halide_runtime.bc)
+target_link_libraries(simd_ops)
+test_suite_add_build_dependencies(simd_ops)
+set_target_properties(simd_ops PROPERTIES LINKER_LANGUAGE CXX)
 
 foreach(sourcebc ${uosources})
   string(REGEX REPLACE ".[cp]+$" "" pathbc ${sourcebc})
   string(REGEX REPLACE ".*/" "" namebc ${pathbc})
   string(REPLACE "." "" namebc ${namebc})
-  set(Source ${CMAKE_CURRENT_SOURCE_DIR}/simd_ops.cpp ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_halide_runtime.bc ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_tests/${namebc}.bc ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_scalar_tests/scalar_${namebc}.bc)
+  set(Source ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_halide_runtime.bc ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_tests/${namebc}.bc ${CMAKE_CURRENT_SOURCE_DIR}/${ARCH}_scalar_tests/scalar_${namebc}.bc)
   set(PROG simd_ops_${namebc})
   llvm_multisource()
+  target_link_libraries(${PROG} simd_ops)
 endforeach()
 
 

test-suite/trunk/cmake/modules/SingleMultiSource.cmake

if(NOT DEFINED PROGRAM_IS_NONDETERMINISTIC)
     endif()
     if(REFERENCE_OUTPUT)
       llvm_test_verify("${DIFFPROG} %o ${REFERENCE_OUTPUT}")
     endif()
   endif()
   llvm_add_test(${testfile} ${executable})
 endfunction()
 
+macro (test_suite_add_build_dependencies executable)
+  if (NOT TEST_SUITE_USE_PERF)
+    add_dependencies(${executable} timeit-target)
+  endif()
+  add_dependencies(${executable} timeit-host fpcmp-host)
+endmacro()
+
 macro(test_suite_add_executable name mainsource)
   list(FIND PROGRAMS_TO_SKIP ${name} name_idx)
   # Should we skip this?
   if(${name_idx} EQUAL -1)
     get_unique_exe_name(executable ${mainsource})
     add_executable(${executable} ${ARGN})
     append_compile_flags(${executable} ${CFLAGS})
     append_compile_flags(${executable} ${CPPFLAGS})
if(${name_idx} EQUAL -1)
 
     # Fall back to old style involving RUN_OPTIONS and STDIN_FILENAME if
     # llvm_test_run() was not called yet.
     if(NOT TESTSCRIPT)
       llvm_test_traditional(${executable_path}.test ${executable_path} ${name})
     else()
       llvm_add_test(${executable_path}.test ${executable_path})
     endif()
-    if (NOT TEST_SUITE_USE_PERF)
-      add_dependencies(${executable} timeit-target)
-    endif()
-    add_dependencies(${executable} timeit-host fpcmp-host)
+    test_suite_add_build_dependencies(${executable})
   endif()
 endmacro()
 
 # Configure the current directory as a SingleSource subdirectory - i.e. every
 # file in *.{c,cpp,cc} is treated as its own test.
 macro(llvm_singlesource)
   file(GLOB sources *.c *.cpp *.cc)
   foreach(source ${sources})