[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

foreach(name ${LLVM_RUNTIME_TARGETS})
         add_dependencies(install-${runtime_name} install-${runtime_name}-${name})
         add_dependencies(install-${runtime_name}-stripped install-${runtime_name}-${name}-stripped)
       endforeach()
       foreach(component ${LLVM_RUNTIME_DISTRIBUTION_COMPONENTS})
         add_dependencies(${component} ${component}-${name})
         add_dependencies(install-${component} install-${component}-${name})
         add_dependencies(install-${component}-stripped install-${component}-${name}-stripped)
       endforeach()
+      if("$name" STREQUAL "openmp")
+        add_dependencies(runtimes-${name} opt llvm-link)
+      endif()
     endforeach()
 
     foreach(multilib ${LLVM_RUNTIME_MULTILIBS})
       foreach(name ${LLVM_RUNTIME_MULTILIB_${multilib}_TARGETS})
         runtime_register_target(${name}+${multilib} ${name}
           DEPENDS runtimes-${name}
           CMAKE_ARGS -DLLVM_RUNTIMES_PREFIX=${name}/
                      -DLLVM_RUNTIMES_LIBDIR_SUBDIR=${multilib})

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()