https://github.com/jhuber6 created https://github.com/llvm/llvm-project/pull/82004
Summary: The original discussion in https://reviews.llvm.org/D118493 was that `clang` should not be adding `-rpath` implicitly for toolchains. The main motivation behind that change was the 'Fedora' toolchain disallowing usage of `-rpath` in their tools. This patch introduces a new clang configuration called `CLANG_ALLOW_IMPLICT_RPATH` that defaults to on. The motivation behind this patch is the fact that the LLVM offloading toolchain is currently in a nearly unusable state for casual users. This is a problem for other runtimes like `libc++` or `libunwind`, however `libomptarget` and other offloading runtimes like HIP are much difficult to support. Currently, `libomptarget` is only supported to be used with the same build that created the executable. Furthermore, `libomptarget` is currently shipped by four different vendors, which are not mutually compatible. Because this is totally broken as far as I'm aware all the vendor compilers reverted the original patch turning this off. The presented solution was to advise users to use Clang configruation files. Since then, this was added to the documentation at the website https://openmp.llvm.org/SupportAndFAQ.html#q-what-are-the-llvm-components-used-in-offloading-and-how-are-they-found but this has not had any noticable effect on the number of people I've had to coach through their broken environment since the patch was reverted and is roughly equivalent to the currenlty accepted solution of using LD_LIBRARY_PATH. This would be solved with static linking, but offloading runtimes also manage thred pools and device resources that cannot be duplicated. The Fedora distributors were the only group that were against doing this by default, so the suggestion is that Fedora uses `-DCLANG_ALLOW_IMPLICIT_RPATH=OFF` when building. >From 7ddd4ad90cda446b94d72b707015767ea759d121 Mon Sep 17 00:00:00 2001 From: Joseph Huber <hube...@outlook.com> Date: Fri, 16 Feb 2024 09:18:12 -0600 Subject: [PATCH] [Clang] Add 'CLANG_ALLOW_IMPLICIT_RPATH' to enable toolchain use of -rpath Summary: The original discussion in https://reviews.llvm.org/D118493 was that `clang` should not be adding `-rpath` implicitly for toolchains. The main motivation behind that change was the 'Fedora' toolchain disallowing usage of `-rpath` in their tools. This patch introduces a new clang configuration called `CLANG_ALLOW_IMPLICT_RPATH` that defaults to on. The motivation behind this patch is the fact that the LLVM offloading toolchain is currently in a nearly unusable state for casual users. This is a problem for other runtimes like `libc++` or `libunwind`, however `libomptarget` and other offloading runtimes like HIP are much difficult to support. Currently, `libomptarget` is only supported to be used with the same build that created the executable. Furthermore, `libomptarget` is currently shipped by four different vendors, which are not mutually compatible. Because this is totally broken as far as I'm aware all the vendor compilers reverted the original patch turning this off. The presented solution was to advise users to use Clang configruation files. Since then, this was added to the documentation at the website https://openmp.llvm.org/SupportAndFAQ.html#q-what-are-the-llvm-components-used-in-offloading-and-how-are-they-found but this has not had any noticable effect on the number of people I've had to coach through their broken environment since the patch was reverted and is roughly equivalent to the currenlty accepted solution of using LD_LIBRARY_PATH. This would be solved with static linking, but offloading runtimes also manage thred pools and device resources that cannot be duplicated. The Fedora distributors were the only group that were against doing this by default, so the suggestion is that Fedora uses `-DCLANG_ALLOW_IMPLICIT_RPATH=OFF` when building. --- clang/CMakeLists.txt | 2 ++ clang/include/clang/Config/config.h.cmake | 3 +++ clang/lib/Driver/ToolChains/CommonArgs.cpp | 3 ++- clang/test/Driver/arch-specific-libdir-rpath.c | 7 ------- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt index 47fc2e4886cfc2..02e0a8f80d9c59 100644 --- a/clang/CMakeLists.txt +++ b/clang/CMakeLists.txt @@ -213,6 +213,8 @@ set(CLANG_SPAWN_CC1 OFF CACHE BOOL option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on linux-gnu" ON) +option(CLANG_ALLOW_IMPLICIT_RPATH "Allow clang to add -rpath automatically" ON) + set(CLANG_DEFAULT_LINKER "" CACHE STRING "Default linker to use (linker name or absolute path, empty for platform default)") diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake index 4015ac8040861c..6ca8cc75953094 100644 --- a/clang/include/clang/Config/config.h.cmake +++ b/clang/include/clang/Config/config.h.cmake @@ -63,6 +63,9 @@ /* Define if dladdr() is available on this platform. */ #cmakedefine CLANG_HAVE_DLADDR ${CLANG_HAVE_DLADDR} +/* Define if we have sys/resource.h (rlimits) */ +#cmakedefine01 CLANG_ALLOW_IMPLICIT_RPATH + /* Linker version detected at compile time. */ #cmakedefine HOST_LINK_VERSION "${HOST_LINK_VERSION}" diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 0fd7b8424eb4ba..cc8e4ffa571a82 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -1107,7 +1107,8 @@ void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC, void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs) { if (!Args.hasFlag(options::OPT_frtlib_add_rpath, - options::OPT_fno_rtlib_add_rpath, false)) + options::OPT_fno_rtlib_add_rpath, + CLANG_ALLOW_IMPLICIT_RPATH)) return; for (const auto &CandidateRPath : TC.getArchSpecificLibPaths()) { diff --git a/clang/test/Driver/arch-specific-libdir-rpath.c b/clang/test/Driver/arch-specific-libdir-rpath.c index 1e6bbbc5929ac2..de114c67862dea 100644 --- a/clang/test/Driver/arch-specific-libdir-rpath.c +++ b/clang/test/Driver/arch-specific-libdir-rpath.c @@ -1,13 +1,6 @@ // Test that the driver adds an arch-specific subdirectory in // {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' // -// Test the default behavior when neither -frtlib-add-rpath nor -// -fno-rtlib-add-rpath is specified, which is to skip -rpath -// RUN: %clang %s -### --target=x86_64-linux \ -// RUN: -fsanitize=address -shared-libasan \ -// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir 2>&1 \ -// RUN: | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s -// // Test that -rpath is not added under -fno-rtlib-add-rpath even if other // conditions are met. // RUN: %clang %s -### --target=x86_64-linux \ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
