llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver Author: Joseph Huber (jhuber6) <details> <summary>Changes</summary> Summary: Currently we pass `-L` and `-l` to get the HIP library. Because we are attached to a single HIP installation it's far better to pass it by filename. This is because the `-L` could be out of order with other user libraries and those could override it. If someone uses HIP with a specific ROCm installation they most likely want that library, otherwise incompatibilities can occur. This is still overridable with command line flags if users want to pass a different one for some reason. This PR also refactors the handling to be more generic for future additions. --- Full diff: https://github.com/llvm/llvm-project/pull/176019.diff 10 Files Affected: - (modified) clang/include/clang/Driver/ToolChain.h (+4-4) - (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (-15) - (modified) clang/lib/Driver/ToolChains/Gnu.cpp (+1-1) - (modified) clang/lib/Driver/ToolChains/Linux.cpp (+20-11) - (modified) clang/lib/Driver/ToolChains/Linux.h (+3-2) - (modified) clang/lib/Driver/ToolChains/MSVC.cpp (+11-6) - (modified) clang/lib/Driver/ToolChains/MSVC.h (+2-2) - (modified) clang/test/Driver/hip-runtime-libs-linux.hip (+5-5) - (modified) clang/test/Driver/hipstdpar.c (+1-1) - (modified) clang/test/Driver/rocm-detect.hip (+3-3) ``````````diff diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h index 1425714d34110..16573a6589813 100644 --- a/clang/include/clang/Driver/ToolChain.h +++ b/clang/include/clang/Driver/ToolChain.h @@ -805,10 +805,10 @@ class ToolChain { getDeviceLibs(const llvm::opt::ArgList &Args, const Action::OffloadKind DeviceOffloadingKind) const; - /// Add the system specific linker arguments to use - /// for the given HIP runtime library type. - virtual void AddHIPRuntimeLibArgs(const llvm::opt::ArgList &Args, - llvm::opt::ArgStringList &CmdArgs) const {} + /// Add the system specific libraries for the active offload kinds. + virtual void addOffloadRTLibs(unsigned ActiveKinds, + const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) const {} /// Return sanitizers which are available in this toolchain. virtual SanitizerMask getSupportedSanitizers() const; diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp index 10a1a412eea08..f83d6d40af79d 100644 --- a/clang/lib/Driver/ToolChains/CommonArgs.cpp +++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp @@ -3045,21 +3045,6 @@ void tools::addOpenMPDeviceRTL(const Driver &D, << LibOmpTargetName << ArchPrefix; } } -void tools::addHIPRuntimeLibArgs(const ToolChain &TC, Compilation &C, - const llvm::opt::ArgList &Args, - llvm::opt::ArgStringList &CmdArgs) { - if ((C.getActiveOffloadKinds() & Action::OFK_HIP) && - (!Args.hasArg(options::OPT_nostdlib) || - TC.getTriple().isKnownWindowsMSVCEnvironment()) && - !Args.hasArg(options::OPT_no_hip_rt) && !Args.hasArg(options::OPT_r)) { - TC.AddHIPRuntimeLibArgs(Args, CmdArgs); - } else { - // Claim "no HIP libraries" arguments if any - for (auto *Arg : Args.filtered(options::OPT_no_hip_rt)) { - Arg->claim(); - } - } -} void tools::addOpenCLBuiltinsLib(const Driver &D, const llvm::opt::ArgList &DriverArgs, diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp index 7ecdbe7c57650..250f4182c4207 100644 --- a/clang/lib/Driver/ToolChains/Gnu.cpp +++ b/clang/lib/Driver/ToolChains/Gnu.cpp @@ -451,7 +451,7 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA, addLinkerCompressDebugSectionsOption(ToolChain, Args, CmdArgs); AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA); - addHIPRuntimeLibArgs(ToolChain, C, Args, CmdArgs); + ToolChain.addOffloadRTLibs(C.getActiveOffloadKinds(), Args, CmdArgs); // The profile runtime also needs access to system libraries. getToolChain().addProfileRTLibs(Args, CmdArgs); diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp index cdbf21fb90263..a8a3b68eb53d9 100644 --- a/clang/lib/Driver/ToolChains/Linux.cpp +++ b/clang/lib/Driver/ToolChains/Linux.cpp @@ -832,19 +832,28 @@ void Linux::AddHIPIncludeArgs(const ArgList &DriverArgs, RocmInstallation->AddHIPIncludeArgs(DriverArgs, CC1Args); } -void Linux::AddHIPRuntimeLibArgs(const ArgList &Args, - ArgStringList &CmdArgs) const { - CmdArgs.push_back( - Args.MakeArgString(StringRef("-L") + RocmInstallation->getLibPath())); +void Linux::addOffloadRTLibs(unsigned ActiveKinds, const ArgList &Args, + ArgStringList &CmdArgs) const { + if (Args.hasArg(options::OPT_nostdlib) || + Args.hasArg(options::OPT_no_hip_rt) || Args.hasArg(options::OPT_r)) + return; - if (Args.hasFlag(options::OPT_frtlib_add_rpath, - options::OPT_fno_rtlib_add_rpath, false)) { - SmallString<0> p = RocmInstallation->getLibPath(); - llvm::sys::path::remove_dots(p, true); - CmdArgs.append({"-rpath", Args.MakeArgString(p)}); - } + llvm::SmallVector<std::pair<StringRef, StringRef>> Libraries; + if (ActiveKinds & Action::OFK_HIP) + Libraries.emplace_back(RocmInstallation->getLibPath(), "libamdhip64.so"); - CmdArgs.push_back("-lamdhip64"); + for (auto [Path, Library] : Libraries) { + if (Args.hasFlag(options::OPT_frtlib_add_rpath, + options::OPT_fno_rtlib_add_rpath, false)) { + SmallString<0> p = Path; + llvm::sys::path::remove_dots(p, true); + CmdArgs.append({"-rpath", Args.MakeArgString(p)}); + } + + SmallString<0> p = Path; + llvm::sys::path::append(p, Library); + CmdArgs.push_back(Args.MakeArgString(p)); + } } void Linux::AddIAMCUIncludeArgs(const ArgList &DriverArgs, diff --git a/clang/lib/Driver/ToolChains/Linux.h b/clang/lib/Driver/ToolChains/Linux.h index 97bad77cb1caa..b4bdc1b1206b1 100644 --- a/clang/lib/Driver/ToolChains/Linux.h +++ b/clang/lib/Driver/ToolChains/Linux.h @@ -37,12 +37,13 @@ class LLVM_LIBRARY_VISIBILITY Linux : public Generic_ELF { llvm::opt::ArgStringList &CC1Args) const override; void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; - void AddHIPRuntimeLibArgs(const llvm::opt::ArgList &Args, - llvm::opt::ArgStringList &CmdArgs) const override; void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; void addSYCLIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; + + void addOffloadRTLibs(unsigned ActiveKinds, const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) const override; RuntimeLibType GetDefaultRuntimeLibType() const override; unsigned GetDefaultDwarfVersion() const override; CXXStdlibType GetDefaultCXXStdlibType() const override; diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index fcae5b7a18f34..229be08ea7a5c 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -325,7 +325,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA, A.renderAsInput(Args, CmdArgs); } - addHIPRuntimeLibArgs(TC, C, Args, CmdArgs); + TC.addOffloadRTLibs(C.getActiveOffloadKinds(), Args, CmdArgs); TC.addProfileRTLibs(Args, CmdArgs); @@ -516,11 +516,16 @@ void MSVCToolChain::addSYCLIncludeArgs(const ArgList &DriverArgs, SYCLInstallation->addSYCLIncludeArgs(DriverArgs, CC1Args); } -void MSVCToolChain::AddHIPRuntimeLibArgs(const ArgList &Args, - ArgStringList &CmdArgs) const { - CmdArgs.append({Args.MakeArgString(StringRef("-libpath:") + - RocmInstallation->getLibPath()), - "amdhip64.lib"}); +void MSVCToolChain::addOffloadRTLibs(unsigned ActiveKinds, const ArgList &Args, + ArgStringList &CmdArgs) const { + if (Args.hasArg(options::OPT_no_hip_rt) || Args.hasArg(options::OPT_r)) + return; + + if (ActiveKinds & Action::OFK_HIP) { + CmdArgs.append({Args.MakeArgString(StringRef("-libpath:") + + RocmInstallation->getLibPath()), + "amdhip64.lib"}); + } } void MSVCToolChain::printVerboseInfo(raw_ostream &OS) const { diff --git a/clang/lib/Driver/ToolChains/MSVC.h b/clang/lib/Driver/ToolChains/MSVC.h index 5c17edce087c7..80a6cbd9bc15b 100644 --- a/clang/lib/Driver/ToolChains/MSVC.h +++ b/clang/lib/Driver/ToolChains/MSVC.h @@ -98,8 +98,8 @@ class LLVM_LIBRARY_VISIBILITY MSVCToolChain : public ToolChain { void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; - void AddHIPRuntimeLibArgs(const llvm::opt::ArgList &Args, - llvm::opt::ArgStringList &CmdArgs) const override; + void addOffloadRTLibs(unsigned ActiveKinds, const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) const override; void addSYCLIncludeArgs(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args) const override; diff --git a/clang/test/Driver/hip-runtime-libs-linux.hip b/clang/test/Driver/hip-runtime-libs-linux.hip index eda87d0aa4b6c..d814cae241813 100644 --- a/clang/test/Driver/hip-runtime-libs-linux.hip +++ b/clang/test/Driver/hip-runtime-libs-linux.hip @@ -58,10 +58,10 @@ // RUN: --rocm-path=%S/Inputs/rocm %s 2>&1 \ // RUN: | FileCheck -check-prefixes=ROCM-PATH %s -// ROCM-PATH: "-L[[HIPRT:.*/Inputs/rocm/lib]]" "-lamdhip64" -// ROCM-RPATH: "-L[[HIPRT:.*/Inputs/rocm/lib]]" "-rpath" "[[HIPRT]]" "-lamdhip64" -// ROCM-RPATH-CANONICAL: "-rpath" "{{.*/rocm/lib}}" "-lamdhip64" -// ROCM-REL: "-L[[HIPRT:.*/opt/rocm-3.10.0/lib]]" "-lamdhip64" +// ROCM-PATH: "{{.*/Inputs/rocm/lib/libamdhip64.so}}" +// ROCM-RPATH: "-rpath" "{{.*/Inputs/rocm/lib}}" +// ROCM-RPATH-CANONICAL: "-rpath" "{{.*/rocm/lib}}" +// ROCM-REL: "{{.*/opt/rocm-3.10.0/lib/libamdhip64.so}}" // NOHIPRT-NOT: "-L{{.*/Inputs/rocm/lib}}" // NOHIPRT-NOT: "-rpath" "{{.*/Inputs/rocm/lib}}" -// NOHIPRT-NOT: "-lamdhip64" +// NOHIPRT-NOT: "libamdhip64.so" diff --git a/clang/test/Driver/hipstdpar.c b/clang/test/Driver/hipstdpar.c index b759c5fb2084a..7d02e0258853e 100644 --- a/clang/test/Driver/hipstdpar.c +++ b/clang/test/Driver/hipstdpar.c @@ -20,4 +20,4 @@ // HIPSTDPAR-COMPILE: "-idirafter" "{{.*/Inputs/hipstdpar}}" // HIPSTDPAR-COMPILE: "-include" "hipstdpar_lib.hpp" // HIPSTDPAR-LINK: "-rpath" -// HIPSTDPAR-LINK: "-l{{.*hip.*}}" +// HIPSTDPAR-LINK: "{{.*hip.*}}" diff --git a/clang/test/Driver/rocm-detect.hip b/clang/test/Driver/rocm-detect.hip index b28b2bc6379dd..2dde98dff070a 100644 --- a/clang/test/Driver/rocm-detect.hip +++ b/clang/test/Driver/rocm-detect.hip @@ -104,17 +104,17 @@ // HIP-PATH: "-mlink-builtin-bitcode" "[[ROCM_PATH]]/amdgcn/bitcode/oclc_isa_version_1010.bc" // HIP-PATH: "-idirafter" "[[HIP_PATH:.*/myhip]]/include" -// HIP-PATH: "-L[[HIP_PATH]]/lib" {{.*}}"-lamdhip64" +// HIP-PATH: "[[HIP_PATH]]/lib/libamdhip64.so" // ROCM-PATH: ROCm installation search path: [[ROCM_PATH:.*/Inputs/rocm]] // ROCM-PATH: "-mlink-builtin-bitcode" "[[ROCM_PATH]]/amdgcn/bitcode/oclc_isa_version_1010.bc" // ROCM-PATH: "-idirafter" "[[ROCM_PATH]]/include" -// ROCM-PATH: "-L[[ROCM_PATH]]/lib" {{.*}}"-lamdhip64" +// ROCM-PATH: "[[ROCM_PATH]]/lib/libamdhip64.so" // USR: ROCm installation search path: [[ROCM_PATH:.*/usr$]] // USR: "-mlink-builtin-bitcode" "[[ROCM_PATH]]/amdgcn/bitcode/oclc_isa_version_1010.bc" // USR: "-idirafter" "[[ROCM_PATH]]/include" -// USR: "-L[[ROCM_PATH]]/lib" {{.*}}"-lamdhip64" +// USR: "[[ROCM_PATH]]/lib/libamdhip64.so" // ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm // ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm-3.10.0 `````````` </details> https://github.com/llvm/llvm-project/pull/176019 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
