AMDGPU/clang: Search resource directory for device libraries

[arsenm]

Summary

This should be the preferred way of locating the libraries, and it's a
packaging problem to ensure the libraries are symlinked into the
resource directory.

This takes precedence over searching for the rocm installation, but
check an explicit --hip-device-lib-path first.

[tra]

Could you walk me through how you see this working in practice?

IIUIC, the idea is to have bitcode files located somewhere within clang installation.
If that's the case, will we ship those bitcode libraries with clang, or do they come from ROCm packages?
If we ship them with clang, who/where/how builds them?
If they come from ROCm packages, how would those packages add stuff into *clang* install directory? Resource dir is a rather awkward location if contents may be expected to change routinely.
What if I have multiple ROCm versions installed? Which one should provide the bitcode in the resource dir?

As long as explicitly specified --hip-device-lib-path can still point to the right path, it's probably OK, but it all adds some confusion about who controls which parts of the HIP compilation and how it all is supposed to work in cases that deviate from the default assumptions.
It would help if the requirements would be documented somewhere.

[arsenm]

In D84068#2159391, @tra wrote:

Could you walk me through how you see this working in practice?

IIUIC, the idea is to have bitcode files located somewhere within clang installation.

Yes

If that's the case, will we ship those bitcode libraries with clang, or do they come from ROCm packages?

The rocm package will install symlinks into the clang resource directory. The rocm provided clang package should depend on the bitcode package

If we ship them with clang, who/where/how builds them?
If they come from ROCm packages, how would those packages add stuff into *clang* install directory? Resource dir is a rather awkward location if contents may be expected to change routinely.

Symlinks. I've been building the device libraries as part of LLVM_EXTERNAL_PROJECTS, and think this should be the preferred way to build and package the libraries. This is how compiler-rt is packaged on linux distributions. The compiler-rt binaries are a separate package symlinked into the resource directory locations. I'm not sure what you mean exactly by change routinely, the libraries should be an implementation detail invisible to users, not something they should be directly relying on. Only clang actually knows how to use them correctly and every other user is buggy

What if I have multiple ROCm versions installed? Which one should provide the bitcode in the resource dir?

These should be treated as an integral part of clang, and not something to mix and match. Each rocm version should have its own copy of the device libraries. It only happens to work most of the time if you mismatch these, and this isn't a guaranteed property.

As long as explicitly specified --hip-device-lib-path can still point to the right path, it's probably OK, but it all adds some confusion about who controls which parts of the HIP compilation and how it all is supposed to work in cases that deviate from the default assumptions.

Long term I would rather get rid of --hip-device-lib-path, and only use the standard -resource_dir flags

It would help if the requirements would be documented somewhere.

Documentation would be good, but the problem I always have is deciding where "somewhere" is

[tra]

In D84068#2204713, @arsenm wrote:

If we ship them with clang, who/where/how builds them?
If they come from ROCm packages, how would those packages add stuff into *clang* install directory? Resource dir is a rather awkward location if contents may be expected to change routinely.

Symlinks. I've been building the device libraries as part of LLVM_EXTERNAL_PROJECTS, and think this should be the preferred way to build and package the libraries. This is how compiler-rt is packaged on linux distributions. The compiler-rt binaries are a separate package symlinked into the resource directory locations. I'm not sure what you mean exactly by change routinely, the libraries should be an implementation detail invisible to users, not something they should be directly relying on. Only clang actually knows how to use them correctly and every other user is buggy

What if I have multiple ROCm versions installed? Which one should provide the bitcode in the resource dir?

These should be treated as an integral part of clang, and not something to mix and match. Each rocm version should have its own copy of the device libraries. It only happens to work most of the time if you mismatch these, and this isn't a guaranteed property.

I'm still not sure how that's going to work. We have M clang versions:N ROCm versions relationship here.
If I have one clang version, but want to do two different builds, one with ROCm-X and one with ROCm-Y, how would I do that? It sounds like I'll need to have multiple clang installation variants.

Similarly, if I have multiple clang versions installed, how would ROCm know which of those clang installations must be updated?

What if I install yet another clang version *after* ROCm has been installed, how will ROCm package know that it needs up update yet another clang installation.

This will get rather unmanageable as soon as you get beyond the "I only have one clang and one ROCm version" scenario.

I think it would make much more sense for clang to treat ROCm's bits as an external dependency, similar to CUDA. Be definition clang/llvm does not control anything outside of its own packages. While ROCm is AMD's package, I'm willing to bet that eventually various Linux distros will start shuffling its bits around the same way it happened to CUDA.

As long as explicitly specified --hip-device-lib-path can still point to the right path, it's probably OK, but it all adds some confusion about who controls which parts of the HIP compilation and how it all is supposed to work in cases that deviate from the default assumptions.

Long term I would rather get rid of --hip-device-lib-path, and only use the standard -resource_dir flags

Please, please, please keep explicit path option. There are real use cases where you can not expect ROCm to be installed anywhere 'standard'. Or at all.
Imagine people embedding libclang into their GUI/tools. There's no resource directory. There may be no ROCm installation, or it may not be possible due to lack of privileges.

I short, I think that tightly coupling clang's expectations to a non-clang project is not a good idea.
Summoning @echristo for a second opinion.

It would help if the requirements would be documented somewhere.

Documentation would be good, but the problem I always have is deciding where "somewhere" is

clang/docs ?

[arsenm]

In D84068#2208132, @tra wrote:

In D84068#2204713, @arsenm wrote:

If we ship them with clang, who/where/how builds them?
If they come from ROCm packages, how would those packages add stuff into *clang* install directory? Resource dir is a rather awkward location if contents may be expected to change routinely.

Symlinks. I've been building the device libraries as part of LLVM_EXTERNAL_PROJECTS, and think this should be the preferred way to build and package the libraries. This is how compiler-rt is packaged on linux distributions. The compiler-rt binaries are a separate package symlinked into the resource directory locations. I'm not sure what you mean exactly by change routinely, the libraries should be an implementation detail invisible to users, not something they should be directly relying on. Only clang actually knows how to use them correctly and every other user is buggy

What if I have multiple ROCm versions installed? Which one should provide the bitcode in the resource dir?

These should be treated as an integral part of clang, and not something to mix and match. Each rocm version should have its own copy of the device libraries. It only happens to work most of the time if you mismatch these, and this isn't a guaranteed property.

I'm still not sure how that's going to work. We have M clang versions:N ROCm versions relationship here.
If I have one clang version, but want to do two different builds, one with ROCm-X and one with ROCm-Y, how would I do that? It sounds like I'll need to have multiple clang installation variants.

Similarly, if I have multiple clang versions installed, how would ROCm know which of those clang installations must be updated?

What if I install yet another clang version *after* ROCm has been installed, how will ROCm package know that it needs up update yet another clang installation.

This will get rather unmanageable as soon as you get beyond the "I only have one clang and one ROCm version" scenario.

What I'm aiming for is a 1 clang : 1 device library copy. Every clang should have its own device library build. It's an implementation detail of clang, and not an independent component you can do anything with (correctly at least). clang is also minimally useful without the libraries

I think it would make much more sense for clang to treat ROCm's bits as an external dependency, similar to CUDA. Be definition clang/llvm does not control anything outside of its own packages. While ROCm is AMD's package, I'm willing to bet that eventually various Linux distros will start shuffling its bits around the same way it happened to CUDA.

Long term, I'd rather aim for merging rocm-device-libs into libclc and making it an llvm project. They're largely forks of the same original sources from about 5 years ago, and it's an unfortunate split of effort. I also specifically do not want distros to be shuffling this around, and want it to behave exactly like compiler-rt. As far as I can tell cuda clang does not actually work with the Ubuntu packaged cuda, which arbitrarily moved the nvvm binaries, and I don't really want a repeat of this situation. I also do want the device libs build to support a non-rocm package to install to a standard distro clang package, which is different than the rocm libraries trying to support every clang in the universe. Ideally just a regular clang works without any formal rocm installation.

As long as explicitly specified --hip-device-lib-path can still point to the right path, it's probably OK, but it all adds some confusion about who controls which parts of the HIP compilation and how it all is supposed to work in cases that deviate from the default assumptions.

Long term I would rather get rid of --hip-device-lib-path, and only use the standard -resource_dir flags

Please, please, please keep explicit path option. There are real use cases where you can not expect ROCm to be installed anywhere 'standard'. Or at all.
Imagine people embedding libclang into their GUI/tools. There's no resource directory. There may be no ROCm installation, or it may not be possible due to lack of privileges.

This isn't any different that compiler-rt; I would expect these cases to embed and mount these in a virtual filesystem.

I short, I think that tightly coupling clang's expectations to a non-clang project is not a good idea.
Summoning @echristo for a second opinion.

With the dance of searching for a rocm or cuda installation, it's still a coupling. It's just weirder looking and assumes the existing poor, non-standard packaging practices. What I want is something that's indistinguishable from compiler-rt from a packaging perspective.

[yaxunl]

In D84068#2208394, @arsenm wrote:

Long term, I'd rather aim for merging rocm-device-libs into libclc and making it an llvm project. They're largely forks of the same original sources from about 5 years ago, and it's an unfortunate split of effort. I also specifically do not want distros to be shuffling this around, and want it to behave exactly like compiler-rt. As far as I can tell cuda clang does not actually work with the Ubuntu packaged cuda, which arbitrarily moved the nvvm binaries, and I don't really want a repeat of this situation. I also do want the device libs build to support a non-rocm package to install to a standard distro clang package, which is different than the rocm libraries trying to support every clang in the universe. Ideally just a regular clang works without any formal rocm installation.

I like the idea of making rocm-device-libs part of llvm-projects, but I think rocm-device-libs is not an OpenCL device library. That said, before rocm-device-library become part of llvm-project, we need to support rocm-device-library at other locations.

[echristo]

Hi All,

I'd really like to avoid depending on any program other than clang installing something into clang's working directory. I think this needs to be written from the perspective of specifying another directory to look for things.

-eric

[arsenm]

In D84068#2210872, @echristo wrote:

Hi All,

I'd really like to avoid depending on any program other than clang installing something into clang's working directory. I think this needs to be written from the perspective of specifying another directory to look for things.

-eric

Do you mean "resource directory" by "working directory"? It's not copying to the working directory.

My point is these libraries *are* part of clang

[arsenm]

This was already done

clang/lib/Driver/ToolChains/AMDGPU.cpp

RocmInstallationDetector::getInstallationPathCandidates() {
   Candidates.emplace_back(D.ResourceDir, /*StrictChecking=*/true);
 
   Candidates.emplace_back(D.SysRoot + "/opt/rocm", /*StrictChecking=*/true);
   return Candidates;
 }
 
 RocmInstallationDetector::RocmInstallationDetector(
     const Driver &D, const llvm::Triple &HostTriple,
-    const llvm::opt::ArgList &Args, bool DetectHIPRuntime, bool DetectDeviceLib)
+    const llvm::opt::ArgList &Args, bool DetectHIPRuntime)
     : D(D) {
   RocmPathArg = Args.getLastArgValue(clang::driver::options::OPT_rocm_path_EQ);
   RocmDeviceLibPathArg =
       Args.getAllArgValues(clang::driver::options::OPT_rocm_device_lib_path_EQ);
   if (auto *A = Args.getLastArg(clang::driver::options::OPT_hip_version_EQ)) {
     HIPVersionArg = A->getValue();
     unsigned Major = 0;
     unsigned Minor = 0;
VersionMajorMinor =
         llvm::VersionTuple(DefaultVersionMajor, DefaultVersionMinor);
     DetectedVersion = (Twine(DefaultVersionMajor) + "." +
                        Twine(DefaultVersionMinor) + "." + VersionPatch)
                           .str();
   }
 
   if (DetectHIPRuntime)
     detectHIPRuntime();
-  if (DetectDeviceLib)
-    detectDeviceLibrary();
 }
 
-void RocmInstallationDetector::detectDeviceLibrary() {
+void RocmInstallationDetector::detectDeviceLibrary(llvm::Triple Triple) {
   assert(LibDevicePath.empty());
 
   if (!RocmDeviceLibPathArg.empty())
     LibDevicePath = RocmDeviceLibPathArg[RocmDeviceLibPathArg.size() - 1];
   else if (const char *LibPathEnv = ::getenv("HIP_DEVICE_LIB_PATH"))
     LibDevicePath = LibPathEnv;
 
   auto &FS = D.getVFS();
   if (!LibDevicePath.empty()) {
     // Maintain compatability with HIP flag/envvar pointing directly at the
     // bitcode library directory. This points directly at the library path instead
     // of the rocm root installation.
     if (!FS.exists(LibDevicePath))
       return;
 
     scanLibDevicePath(LibDevicePath);
     HasDeviceLibrary = allGenericLibsValid() && !LibDeviceMap.empty();
     return;
   }
 
+  {
+    // Try the resource directory.
+    // Look in amdhsa/amdgcn subdirectory.
+    SmallString<0> ResourceSubDir(D.ResourceDir);
+
+    // XXX - Should we keep bitcode subdirectory or re-add .amdgcn to the
+    // suffix? The compiler-rt libraries use libclang_rt.{name}-{i386|x86_64}.a
+    // in the same subdirectory.
+    llvm::sys::path::append(ResourceSubDir, "lib",
+                            llvm::Triple::getOSTypeName(Triple.getOS()),
+                            llvm::Triple::getArchTypeName(Triple.getArch()));
+    scanLibDevicePath(ResourceSubDir);
+    HasDeviceLibrary = allGenericLibsValid() && !LibDeviceMap.empty();
+    if (HasDeviceLibrary)
+      return;
+  }
+
   // The install path situation in old versions of ROCm is a real mess, and
   // use a different install layout. Multiple copies of the device libraries
   // exist for each frontend project, and differ depending on which build
   // system produced the packages. Standalone OpenCL builds also have a
   // different directory structure from the ROCm OpenCL package.
   auto Candidates = getInstallationPathCandidates();
   for (const auto &Candidate : Candidates) {
     auto CandidatePath = Candidate.Path;
return !HasWave32 || DriverArgs.hasFlag(
     options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);
 }
 
 
 /// ROCM Toolchain
 ROCMToolChain::ROCMToolChain(const Driver &D, const llvm::Triple &Triple,
                              const ArgList &Args)
     : AMDGPUToolChain(D, Triple, Args) {
-  RocmInstallation.detectDeviceLibrary();
+  RocmInstallation.detectDeviceLibrary(Triple);
 }
 
 void AMDGPUToolChain::addClangTargetOptions(
     const llvm::opt::ArgList &DriverArgs,
     llvm::opt::ArgStringList &CC1Args,
     Action::OffloadKind DeviceOffloadingKind) const {
   // Default to "hidden" visibility, as object level linking will not be
   // supported for the foreseeable future.

clang/lib/Driver/ToolChains/ROCm.h

private:
 
   void scanLibDevicePath(llvm::StringRef Path);
   void ParseHIPVersionFile(llvm::StringRef V);
   SmallVector<Candidate, 4> getInstallationPathCandidates();
 
 public:
   RocmInstallationDetector(const Driver &D, const llvm::Triple &HostTriple,
                            const llvm::opt::ArgList &Args,
-                           bool DetectHIPRuntime = true,
-                           bool DetectDeviceLib = false);
+                           bool DetectHIPRuntime = true);
 
   /// Add arguments needed to link default bitcode libraries.
   void addCommonBitcodeLibCC1Args(const llvm::opt::ArgList &DriverArgs,
                                   llvm::opt::ArgStringList &CC1Args,
                                   StringRef LibDeviceFile, bool Wave64,
                                   bool DAZ, bool FiniteOnly, bool UnsafeMathOpt,
                                   bool FastRelaxedMath, bool CorrectSqrt) const;
 
public:
   /// Get libdevice file for given architecture
   std::string getLibDeviceFile(StringRef Gpu) const {
     return LibDeviceMap.lookup(Gpu);
   }
 
   void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                          llvm::opt::ArgStringList &CC1Args) const;
 
-  void detectDeviceLibrary();
+  void detectDeviceLibrary(llvm::Triple);
   void detectHIPRuntime();
 
   /// Get the values for --rocm-device-lib-path arguments
   std::vector<std::string> getRocmDeviceLibPathArg() const {
     return RocmDeviceLibPathArg;
   }
 
   /// Get the value for --rocm-path argument

clang/test/Driver/hip-device-libs-resource-dir.hip

+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Test if oclc_daz_opt_on or if oclc_daz_opt_off is linked depending on
+// expected denormal mode.
+
+// Test subtarget with flushing on by default.
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:  --cuda-gpu-arch=gfx803 \
+// RUN:  -resource-dir=%S/Inputs/resource_dir   \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,FLUSHD
+
+
+// Test subtarget with flushing off by ddefault.
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:  --cuda-gpu-arch=gfx900 \
+// RUN:  -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,NOFLUSHD
+
+
+// Test explicit flag, opposite of target default.
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   -fcuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,FLUSHD
+
+
+// Test explicit flag, opposite of target default.
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 \
+// RUN:   -fno-cuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,NOFLUSHD
+
+
+// Test explicit flag, same as target default.
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   -fno-cuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,NOFLUSHD
+
+
+// Test explicit flag, same as target default.
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 \
+// RUN:   -fcuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,FLUSHD
+
+
+// Test last flag wins, not flushing
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 \
+// RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,NOFLUSHD
+
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   -fcuda-flush-denormals-to-zero -fno-cuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir   \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,NOFLUSHD
+
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   -fno-cuda-flush-denormals-to-zero -fcuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir   \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,FLUSHD
+
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 \
+// RUN:   -fno-cuda-flush-denormals-to-zero -fcuda-flush-denormals-to-zero \
+// RUN:   -resource-dir=%S/Inputs/resource_dir \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,FLUSHD
+
+
+// Test --hip-device-lib-path flag
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 \
+// RUN:   --hip-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode   \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL,FLUSHD
+
+
+// Test environment variable HIP_DEVICE_LIB_PATH
+// RUN: env HIP_DEVICE_LIB_PATH=%S/Inputs/rocm/amdgcn/bitcode \
+// RUN:   %clang -### -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx900 \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s --check-prefixes=ALL
+
+// ALL: {{"[^"]*clang[^"]*"}}
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.bc"
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}ocml.bc"
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}ockl.bc"
+
+// FLUSHD-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_daz_opt_on.bc"
+// NOFLUSHD-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_daz_opt_off.bc"
+
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_unsafe_math_off.bc"
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_finite_only_off.bc"
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_correctly_rounded_sqrt_on.bc"
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_wavefrontsize64_on.bc"
+// ALL-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_isa_version_{{[0-9]+}}.bc"