[LLVM] Add per-target runtime directory to rpath

[jhuber6]

Summary:
This simply adds the LLVM_DEFAULT_TARGET_TRIPLE to the LLVM build's
rpath if present. This keeps things hermetic for the library (offload)
that depends on it.

The reason this is required is because llvm-gpu-loader calls
DynamicLibrary on the Offload runtime. However, in a shared library
build the actual call is in libLLVMSupport.so, which does not have this
RPath, so dlopen delegates to that which does not know how to find it.
The only options to fix this are to use dlopen directly in the loader,
or add the rpath to the LLVM binaries.

I think this makes sense for LLVM, because the target-specific directory
can contain LLVM related libraries.

[llvmorg-github-actions]

@llvm/pr-subscribers-offload

Author: Joseph Huber (jhuber6)

Changes

Summary:
We moved the directory from the normal lib/ to the triple qualified one,
which meant that the search path now needs to search two libraries. We
need to have this tool find both the LLVM libraries and the Offload
library when builting without static linking.

---

Full diff: https://github.com/llvm/llvm-project/pull/199755.diff

2 Files Affected:

• (modified) llvm/tools/llvm-gpu-loader/CMakeLists.txt (+9)
• (modified) offload/liboffload/CMakeLists.txt (+2-4)

diff --git a/llvm/tools/llvm-gpu-loader/CMakeLists.txt b/llvm/tools/llvm-gpu-loader/CMakeLists.txt
index 37377ddc0506b..797029335bb25 100644
--- a/llvm/tools/llvm-gpu-loader/CMakeLists.txt
+++ b/llvm/tools/llvm-gpu-loader/CMakeLists.txt
@@ -12,3 +12,12 @@ add_llvm_tool(llvm-gpu-loader
   DEPENDS
   intrinsics_gen
 )
+
+# The offload project is the only runtime that uses LLVM libraries. We need to
+# set the dynamic path to discover it when static linking is disabled.
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set_property(TARGET llvm-gpu-loader APPEND PROPERTY
+    BUILD_RPATH "$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
+  set_property(TARGET llvm-gpu-loader APPEND PROPERTY
+    INSTALL_RPATH "$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
+endif()
diff --git a/offload/liboffload/CMakeLists.txt b/offload/liboffload/CMakeLists.txt
index a03eebf25d7aa..447e6963e96a3 100644
--- a/offload/liboffload/CMakeLists.txt
+++ b/offload/liboffload/CMakeLists.txt
@@ -38,10 +38,8 @@ target_compile_definitions(LLVMOffload PRIVATE
   DEBUG_PREFIX="Liboffload"
 )
 
-set_target_properties(LLVMOffload PROPERTIES
-                      POSITION_INDEPENDENT_CODE ON
-                      INSTALL_RPATH "$ORIGIN"
-                      BUILD_RPATH "$ORIGIN:${CMAKE_CURRENT_BINARY_DIR}/..")
+set_target_properties(LLVMOffload PROPERTIES POSITION_INDEPENDENT_CODE ON)
+llvm_setup_rpath(LLVMOffload)
 install(TARGETS LLVMOffload LIBRARY COMPONENT offload DESTINATION "${OFFLOAD_INSTALL_LIBDIR}")
 
 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/API/OffloadAPI.h DESTINATION ${CMAKE_INSTALL_PREFIX}/include/offload COMPONENT offload)

[jplehr]

I can confirm that this resolves the issues we are seeing in https://lab.llvm.org/buildbot/#/builders/10 for libc on GPU.

[Meinersbur]

Typo in summary: DyanmicLibrary

[Meinersbur]

LGTM

[Meinersbur]

This bit can be removed then

[jhuber6]

Suggested change

	if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
	set(_per_target_suffix "/${LLVM_DEFAULT_TARGET_TRIPLE}")
	endif()

[petrhosek]

Note that if LLVM binaries (executables or shared libraries) are built against libc++.so, now they would pick up the one from per-target runtime directory which may or may not be desirable, but in either case it's a potentially breaking change.

[jhuber6]

Note that if LLVM binaries (executables or shared libraries) are built against libc++.so, now they would pick up the one from per-target runtime directory which may or may not be desirable, but in either case it's a potentially breaking change.

I'd assume that's more desirable than it isn't, but I'm always afraid that changes like this will subtly break things. I suppose we'll see when this starts landing in people's builds.