[LLVM] `tools/llvm-gpu-loader` introduces silent automagic dependencies on HSA and CUDA runtimes

[mgorny]

#132096 introduced new llvm-gpu-loader tool that can link to HSA and CUDA runtimes.

Both checks were added deep down the source tree, in tools/llvm-gpu-loader/CMakeLists.txt:

llvm-project/llvm/tools/llvm-gpu-loader/CMakeLists.txt

Line 27 in bcedb36

find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm)

llvm-project/llvm/tools/llvm-gpu-loader/CMakeLists.txt

Line 38 in bcedb36

find_package(CUDAToolkit 11.2 QUIET)

Neither of them features a proper option() to control the dependency, and on top of that, they are both marked QUIET, so it's really easy to overlook this (as we did), and end up building packages that randomly depend on either of the runtimes, or miss the specific function.

Please:

1. Do not use QUIET for package checks that control features included in installed tools.
2. Do not add dependency checks deep down the source tree — LLVM has a bunch of standard locations such as top CMakeLists.txt and cmake/config-ix.cmake, where people actually look for changes like these. We don't really have the resources to monitor all 528 CMakeLists.txt files in LLVM alone.
3. Add option()s to control enabling the specific dependencies, so that we can cleanly control whether they are enabled or not, rather than being automatically linked if the build system incidentally happens to have them installed.

[jhuber6]

The typical solution is to just remove the linking entirely by making it just dynamically load everything. I could do the same trick here. If you need a fix this moment, disable it with LLVM_TOOL_LLVM_GPU_LOADER_BUILD=OFF. I will try to clean this up soon, there's also issues related to gcc8 I'm trying to resolve.

[Jannik2099]

The typical solution is to just remove the linking entirely by making it just dynamically load everything

this doesn't solve the issue, it just solves the specific symptom of "broken binaries". You still end up with a non-deterministic build that silently changes behaviour depending on what's installed on the build machine.

[jhuber6]

this doesn't solve the issue, it just solves the specific symptom of "broken binaries". You still end up with a non-deterministic build that silently changes behaviour depending on what's installed on the build machine.

I'm not following, the build becomes the same regardless of present libraries and the errors get pushed onto runtime where it becomes "This tool can't run because it failed to load 'X' library".

[mgorny]

The typical solution is to just remove the linking entirely by making it just dynamically load everything. I could do the same trick here. If you need a fix this moment, disable it with LLVM_TOOL_LLVM_GPU_LOADER_BUILD=OFF. I will try to clean this up soon, there's also issues related to gcc8 I'm trying to resolve.

If this means it will build them also without CUDA/HSA headers, then I think that would be the best solution. Thanks!

[jhuber6]

If this means it will build them also without CUDA/HSA headers, then I think that would be the best solution. Thanks!

It's kind of annoying because it requires defining all the headers yourself, but it's what we did with the other tools so they can build by default. I would've put this in offload/ or something since that already has code dedicated to doing stuff like this, but it creates an unfortunate dependency loop because we need this tool for the runtimes to do testing and offload is build as a runtime.

[mgorny]

llvm/shared? Or perhaps we should just have top-level shared directory with header-only stuff that's reused between different projects (instead of, say, libc/shared).

[jhuber6]

Yeah, I've floated that. We have DLWrap.h in offload/ which helps with defining these, could maybe move that to some support library in LLVM or something since it's just a header.

[Jannik2099]

I'm not following, the build becomes the same regardless of present libraries and the errors get pushed onto runtime where it becomes "This tool can't run because it failed to load 'X' library".

Sorry, I may have misunderstood the amdhsa.cpp and nvptx.cpp TUs. Are the hsa / cuda headers used from the system installation that is silently being discovered, or from offload/plugins-nextgen ?

It's totally fine in the latter case, of course

[jhuber6]

Sorry, I may have misunderstood the amdhsa.cpp and nvptx.cpp TUs. Are the hsa / cuda headers used from the system installation that is silently being discovered, or from offload/plugins-nextgen ?

It's totally fine in the latter case, of course

This is separate because of dependency stuff, but it's the same idea. You manually declare the functions and then load them with dlopen.

[sylvestre]

This is also breaking a bunch of jobs on apt.llvm.org