[libomptarget][amdcgn] Add build dependency for llvm-link and opt

[protze.joachim]

Summary

D107156 and D107320 are not sufficient when OpenMP is built as llvm runtime because dependencies only work within the same cmake instance.
We could limit the dependency to cases where libomptarget/plugins are really built. But compared to the whole llvm project, building openmp runtime is negligible and postponing the build of OpenMP runtime after the dependencies are ready seems reasonable.

The direct dependency introduced in D107156 and D107320 is necessary for the case where OpenMP is built as llvm project.
This patch supersedes / includes D107320.

[JonChesterfield]

Thank you very much for getting to the bottom of this! One to add to the llvm-13 branch too I think

[protze.joachim]

In D108404#2955774, @JonChesterfield wrote:

Thank you very much for getting to the bottom of this! One to add to the llvm-13 branch too I think

Does the documentation for the release suggest to use LLVM_ENABLE_RUNTIME=openmp rather than LLVM_ENABLE_PROJECT=openmp?

[JonChesterfield]

I think we've written ENABLE_RUNTIMES as the recommended approach in some of the docs but don't know if those correspond to what the release uses. E.g. https://openmp.llvm.org/docs/SupportAndFAQ.html suggests ENABLE_RUNTIMES.

[protze.joachim]

During further testing. I found that nvptx-new had a similar issue. I also had to move the depend code in the runtime cmake file, but I could verify in the ninja.build file, that this change now really adds the expected necessary dependency.

[JonChesterfield]

Reading the ninja file to determine what cmake's dependency graph looked like is a great idea. Going to use that in future, thanks for the comment!

[Hahnfeld]

In D108404#2957095, @JonChesterfield wrote:

Reading the ninja file to determine what cmake's dependency graph looked like is a great idea. Going to use that in future, thanks for the comment!

(drive-by comment unrelated to the patch, but because of "reading ninja files")

You know about Ninja's "Extra Tools", in particular ninja -t browse and ninja -t graph? That should be a nicer UX than going through the machine-readable build.ninja 😉

llvm/runtimes/CMakeLists.txt

llvm_ExternalProject_Add(runtimes-${name}
                            TARGET_TRIPLE ${target}
                            ${EXTRA_ARGS})
 endfunction()
 
 if(runtimes)
   # Create a runtimes target that uses this file as its top-level CMake file.
   # The runtimes target is a configuration of all the runtime libraries
   # together in a single CMake invocaiton.
+  if("openmp" IN_LIST LLVM_ENABLE_RUNTIMES)
+    message(STATUS "Adding dependencies opt llvm-link")
+    set(extra_deps opt llvm-link)
+  endif()
   if(NOT LLVM_RUNTIME_TARGETS)
     runtime_default_target(
-      DEPENDS ${deps}
+      DEPENDS ${deps} ${extra_deps}
       PREFIXES ${prefixes})
     set(test_targets check-runtimes)
   else()
     if("default" IN_LIST LLVM_RUNTIME_TARGETS)
       runtime_default_target(
-        DEPENDS ${deps}
+        DEPENDS ${deps} ${extra_deps}
         PREFIXES ${prefixes})
       list(REMOVE_ITEM LLVM_RUNTIME_TARGETS "default")
     else()
       add_custom_target(runtimes)
       add_custom_target(runtimes-configure)
       add_custom_target(install-runtimes)
       add_custom_target(install-runtimes-stripped)
       if(LLVM_INCLUDE_TESTS)

openmp/libomptarget/DeviceRTL/CMakeLists.txt

 if(${LIBOMPTARGET_DEVICE_DEBUG})
   list(APPEND bc_flags -DOMPTARGET_DEBUG=-1)
 else()
   list(APPEND bc_flags -DOMPTARGET_DEBUG=0)
 endif()
 
 # Create target to build all Bitcode libraries.
 add_custom_target(omptarget-new-nvptx-bc)
+add_dependencies(omptarget-new-nvptx-bc opt llvm-link)
 
 # Generate a Bitcode library for all the compute capabilities the user requested
 foreach(sm ${nvptx_sm_list})
   # TODO: replace this with declare variant and isa selector.
   set(cuda_flags -Xclang -target-cpu -Xclang sm_${sm} "-D__CUDA_ARCH__=${sm}0")
   set(bc_files "")
   foreach(src ${src_files})
     get_filename_component(infile ${src} ABSOLUTE)
add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}_opt
       COMMENT "Optimizing LLVM bitcode ${bclib_name}"
   )
   set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${bclib_name})
 
   set(bclib_target_name "omptarget-new-nvptx-sm_${sm}-bc")
 
   add_custom_target(${bclib_target_name} ALL DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}_opt)
   add_dependencies(omptarget-new-nvptx-bc ${bclib_target_name})
+  add_dependencies(${bclib_target_name} opt llvm-link)
 
   # Copy library to destination.
   add_custom_command(TARGET ${bclib_target_name} POST_BUILD
                     COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name}
                     ${LIBOMPTARGET_LIBRARY_DIR})
 
   # Install bitcode library under the lib destination folder.
   install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${bclib_name} DESTINATION "${OPENMP_INSTALL_LIBDIR}")
 endforeach()

openmp/libomptarget/deviceRTLs/amdgcn/CMakeLists.txt

 foreach(mcpu ${mcpus})
   set(bc_files)
   add_cuda_bc_library(${cuda_sources})
 
   set(bc_libname lib${libname}-${mcpu}.bc)
   add_custom_command(
     OUTPUT ${bc_libname}
     COMMAND ${LINK_TOOL} ${bc_files} | ${OPT_TOOL} --always-inline -o ${OUTPUTDIR}/${bc_libname}
-    DEPENDS ${bc_files} opt)
+    DEPENDS ${bc_files} llvm-link opt)
 
   add_custom_target(lib${libname}-${mcpu} ALL DEPENDS ${bc_libname})
 
   install(FILES ${OUTPUTDIR}/${bc_libname}
      DESTINATION "${OPENMP_INSTALL_LIBDIR}"
   )
 endforeach()