[HIP] Support Spack packages

[yaxunl]

Summary

Spack is a package management tool extensively used by HPC community.
As ROCm packages are built by Spack by HPC community, we need to teach
clang driver to detect ROCm installation built by Spack.

[tra]

clang/lib/Driver/ToolChains/AMDGPU.cpp

25–31

void function can not return true, so the comment needs updating.

Also, Using Path as both an input and an output is odd. We should probably return the full path or an empty string and call the function findSPACKPackage.

27–28

How are users expected to handle cases when they do have multiple rocm versions installed with spack (I assume that having multiple packages *is* possible. E.g. package-4.0, package-3.7 should be able to co-exist). They may have legitimate reasons to have more than one rocm version installed *and* they may need to be able to build with a particular one.

Can users use --hip-path=specific/hip/version/in-spack ?

clang/lib/Driver/ToolChains/ROCm.h

63

Nit: using SmallString here and everywhere else in this patch does not buy us anything. std::string would be fine.
My own rule of thumb is to use standard types by default and optimized types when they are needed.

[yaxunl]

clang/lib/Driver/ToolChains/AMDGPU.cpp

25–31

done

27–28

Yes. --hip-path is introduced for that purpose.

clang/lib/Driver/ToolChains/ROCm.h

63

thanks. will use std::string

[yaxunl]

revised by Artem's comments

[tra]

clang/lib/Driver/ToolChains/AMDGPU.cpp

27–28

So, --hip-path is expected to point to the HIP runtime install directory and could presumably be used to point anywhere, including an arbitrary spack package. OK, this part works.

32

Based on how the function is used, perhaps we should make it
static std::string findSPACKPackage(const Driver &D, StringRef Path, StringRef PackageName, StringRef RocmVersion) and fold the prefix construction (and possibly version non-emptiness check) into the function.

383–398

So, normally --hip-path would point to the HIP package installation.
However, is clang installation path contains llvm-amdgpu (and thus CanInfo.SPACKReleaseStr is set to whatever version may follow),
then --hip-path (and other candidate paths) directories will also be treated as an SPACK package directory and, if the hip package is found there, then *that* package directory will be used as the actual path to be used.

I see several potential issues with that.

• if clang path contains llvm-amdgpu, but it is *not* installed as an SPACK package We have no control over user's choice of the name for the installation path.
• what if clang is installed as an SPCAK, but is invoked via a symlink with the name that does not contain llvm-amdgpu
• the fact that --hip-path/--rocm-path behavior may change based on clang's executable path is rather peculiar. User-supplied parameters should not change their meaning depending on what you call the executable.

I'd be OK to derive spack-based paths if user didn't provide the paths explicitly, but I don't think we should add any magic to explicit overrides.

[yaxunl]

clang/lib/Driver/ToolChains/AMDGPU.cpp

32

Will use PackageName and RocmVersion as arguments.

However, we use llvm::sys::path to manipulate paths and llvm::sys::path is using llvm::SmallString. If we return std::string or use StringRef for path argument, we have to convert them back and force.

383–398

Will make SPACKReleaseStr a member of Candidate and allow mixed Spack candidates and non-Spack candidates. If clang path contains llvm-amdgpu but is not a Spack package, clang will still look for non-Spack ROCm candidates.

Also added parent of real path of clang as ROCm candidate. If clang is invoked through a sym link which is not under the real ROCm directory, clang will still be able to detect it. Also added --print-rocm-search-dirs for testing this.

When --hip-path/--rocm-path is set, the specified ROCm/HIP path is used. Spack detection is disabled, since the candidate specified by --rocm-path or --hip-path has empty SPACKReleaseStr.

[yaxunl]

revised by Artem's comments

[tra]

clang/include/clang/Driver/Options.td

3566–3567

Perhaps the existing print-search-dirs option would do the job?

clang/lib/Driver/ToolChains/AMDGPU.cpp

32

It should still work. The use cases below look like this:

  auto SPACKPath = findSPACKPackage(D, Path, "rocm-device-libs",
                                    Candidate.SPACKReleaseStr);
  if (!SPACKPath.empty())
    Path = SPACKPath;
}
for (auto SubDir : SubDirs)
  llvm::sys::path::append(Path, SubDir);

It's the Pack that needs to be a SmallString. Assigning an std::string to it should work -- it has operator=(StringRef) and StringRef can be created from a std::string.

I'll leave it up to you. If std::string is inconvenient to use, the SmallString is fine.

186–188

We call getInstallationPathCandidates() more more than one place, so we may end up with the same path added multiple times.

I think population of the candidate list should be separated from reading it back, and should be done only once.
E.g.

if (!isCandidateListReady) 
   initCandidateList(); // populates ROCmSearchDirs, sets isCandidateListReady
return ROCmSearchDirs;

213

What would be a consequence of mistakingly detecting a non-SPACK installation as SPACK?

I'm still not quite comfortable with using a path name, which can be arbitrarily named by a user, as a way to detect particular installation method.
Do SPACK packages have something *inside* the package directory that we could use to tell that it is indeed an SPACK package? Some sort of metadata or version file, perhaps?

It's probably OK to rely on the path naming scheme, but there are still corner cases it can't handle.
I could have a setup when the path-name based detection would still fail despite considering both the reaolved and unresolved symlink names.
E.g.

/pkg/llvm-amdgpu-hash -> /pkg/some-other-real-dir-name
/usr/local/bin/clang -> /pkg/llvm-amdgpu-hash/bin/clang

If I compile with /usr/local/bin/clang neither the realpath nor the unresolved symlink will have 'llvm-amdgpu'.

Granted, it's a somewhat contrived setup. Using the name as the hint would work for the standard setups, so we can live with that, provided that we can always tell compiler where to find the files it needs, if the user has an unusual install.

383–398

Also added --print-rocm-search-dirs for testing this.

Maybe we could just enable that with -v. We already are printing various locations for GCC installations, etc.

clang/lib/Driver/ToolChains/ROCm.h

48

I'd add a bool isSPACK() method to consolidate the SPACK/non-SPACK determination into a single location.

[yaxunl]

clang/include/clang/Driver/Options.td

3566–3567

The concern is similar to https://reviews.llvm.org/D79210:

There may be tools parsing print-search-dirs output, changing it may break those tools.

clang/lib/Driver/ToolChains/AMDGPU.cpp

186–188

We check whether ROCmSearchDirs is populated on line 166 and only populate it if not.

213

llvm-amdgpu Spack installation does not add extra files. It uses normal llvm cmake files to build and differs from a non-Spack llvm build only by name. However, its name follows a very unusual pattern llvm-amdgpu-<release>-<hash>, where <hash> is a string of 32 alphanumerical characters. I think we could detect it by checking the length of <hash>.

383–398

The situation is somehow similar to https://reviews.llvm.org/D79210

Another reason not to print this with -v is that normally users may not need this information unless clang cannot find ROCm and users want to know why.

clang/lib/Driver/ToolChains/ROCm.h

48

done

[yaxunl]

revised by Artem's comments

[tra]

LGTM modulo couple of nits.

clang/include/clang/Driver/Options.td

3566–3567

OK. New option it is then,

clang/lib/Driver/ToolChains/AMDGPU.cpp

163–164

I'm not quite sure which part of the code is related to the do not do strict check. Perhaps move the comment closer to the code it describes and add the details what we'd do otherwise, so one can understand what's missing.

Also, adding a comment describing what the function does (returns candidate list, populating it once, if necessary) would be great.

186–188

Agreed, the code works. That said, the fact that I didn't notice that is a point in case -- it's not obvious.
It's much easier to keep track of what's going on when one function does one things without changing its behavior from one call to another.
I'd at least add a comment around line 166 saying that just return the list if we've already populated it.

213

I think we're getting beyond the point of diminishing returns here. If we don't have a way to identify an SPACK package directly, detecting it by the name will never be 100% reliable. The current code will be correct most of the time. For those few cases where it may get it wrong, youser should be able to specify the correct path directly. No need to complicate it further -- it's not going to buy us much.

364–370

This could be simplified to

auto MaybeSpackPath = Candidate.isSPACK() ? findSPACKPackage() : "";
auto Path = MaybeSpackPath.empty() ? CandidatePath : MaybeSpackPath;

We could further simplify it if we move the isSpack() check into findSPACKPackage and make it return an empty string if it's not.

383–398

OK.

[yaxunl]

clang/lib/Driver/ToolChains/AMDGPU.cpp

163–164

will do

186–188

will do

213

sure. will not check hash length.

364–370

will move isSPACK() inside findSPACKPackage

[haampie]

Hi @tra and @yaxunl, I'm commenting as a reviewer of the spack pull request for the rocm 4.2.0 ecosystem. First of all: thanks for caring about spack installations, that's highly appreciated.

However, this patch does not seem the right approach to me, for a couple reasons:

1. LLVM should not be aware of spack -- it doesn't make sense. Spack doesn't even have a stable 1.0 release, expect it to break and to be inconsistent between versions.
2. It's incorrect in multiple ways: (a) the directory structure name is configurable in spack, not everybody has this <name>-<version>-<hash> structure, so you shouldn't rely on it, (b) you are still not guaranteed to pick up the correct llvm-amdgpu because it may be installed in a chained repo on a shared HPC system, and it won't be in the same prefix folder at all (c) spack's main selling point is it supports multiple installs of one package (a hash also changes for the same llvm version if a downstream dep such as zlib is updated), this patch just bails when it detect multiple installations

Let's step back a bit. The problem you seem to be solving is the circular dependency between llvm and rocm-device-libs which are separate packages in spack; clang can't know where rocm-device-libs is at compile time because rocm-device-libs depends on llvm.

Clearly the solution is to build llvm together with rocm-device-libs, as explained in the readme of https://github.com/RadeonOpenCompute/ROCm-Device-Libs, we can fix that in spack by adding a resource in the llvm package to pull in more sources, and LLVM can happily live without knowing about spack.

Thanks,

Harmen

[haampie]

I've created a pull request to spack here: https://github.com/spack/spack/pull/23859, hopefully that's enough to revert this patch.

Edit: sorry, I may have accidentally changed the state of this issue.

[yaxunl]

In D97340#2775477, @haampie wrote:

Hi @tra and @yaxunl, I'm commenting as a reviewer of the spack pull request for the rocm 4.2.0 ecosystem. First of all: thanks for caring about spack installations, that's highly appreciated.

However, this patch does not seem the right approach to me, for a couple reasons:

1. LLVM should not be aware of spack -- it doesn't make sense. Spack doesn't even have a stable 1.0 release, expect it to break and to be inconsistent between versions.
2. It's incorrect in multiple ways: (a) the directory structure name is configurable in spack, not everybody has this <name>-<version>-<hash> structure, so you shouldn't rely on it, (b) you are still not guaranteed to pick up the correct llvm-amdgpu because it may be installed in a chained repo on a shared HPC system, and it won't be in the same prefix folder at all (c) spack's main selling point is it supports multiple installs of one package (a hash also changes for the same llvm version if a downstream dep such as zlib is updated), this patch just bails when it detect multiple installations

Let's step back a bit. The problem you seem to be solving is the circular dependency between llvm and rocm-device-libs which are separate packages in spack; clang can't know where rocm-device-libs is at compile time because rocm-device-libs depends on llvm.

Clearly the solution is to build llvm together with rocm-device-libs, as explained in the readme of https://github.com/RadeonOpenCompute/ROCm-Device-Libs, we can fix that in spack by adding a resource in the llvm package to pull in more sources, and LLVM can happily live without knowing about spack.

Thanks,

Harmen

Thanks for your feedback.

Before this patch, clang assumes llvm, HIP and device lib directory follow ROCm directory structure. Since the option -hip-path and environment variable HIP_PATH have not been introduced to clang, there was no way for clang to specify or deduce HIP path if it does not follow ROCm directory structure. At that time, I thought the only way to support Spack package was to let clang know its naming convention and relative directory structure among llvm, HIP and device lib. That's what motivated this patch.

The patch should not cause circular dependency on HIP or device library. There was a bug causing clang to fail if it did not find Spack HIP package or multiple Spack HIP packages. I apologize for the trouble caused by that. That bug was fixed by https://reviews.llvm.org/D102556. With that fix, clang will not fail if no Spack HIP package detected or multiple Spack HIP packages are detected.

With that fix, Spack llvm-amdgpu package does not need to depend on Spack HIP and device-lib packages. llvm-amdgpu package can be built and installed first. Then device-lib package can be built with clang as before, since device-lib is written in OpenCL and the OpenCL code is compiled with -nogpulib, therefore clang should not try to detect device-lib when building device lib. Also clang does not need HIP to compile device-lib.

Now that clang supports -hip-path option and HIP_PATH environment variable, it is possible to specify HIP location for Spack. The detection of Spack HIP and device-lib path become an optional feature for users' convenience. Without auto-detection of HIP and device-lib path, users have to use lengthy options -hip-device-lib-path and -hip-path to specify them, which is painful. That's why we want to detect them automatically, like how clang auto-detect gcc, msvc, CUDA SDK, and ROCm. Such auto-detection is optional. If clang cannot auto-detect HIP or device-lib, it just assumes they are not installed and will not fail. Users are always able to override it by using -hip-device-lib-path and -hip-path. Therefore for users it is not a loss to have auto-detection. Hopefully, by following proper heuristics based on naming and location conventions, we may be able to detect HIP and device-lib location automatically for common users, so that they can use clang out of box to compile HIP programs without specifying HIP and device-lib path explicitly. For cases where clang cannot auto-detect HIP and device-lib locations, users can always specify them with options or environment variables.

[haampie]

Hi Yaxunl,

The patch should not cause circular dependency on HIP or device library.

I'm not saying this patch introduces a circular dependency, I'm saying you are trying to solve an already existing circular dependency (clang needs device libs at runtime, but device libs need llvm to compile).

Let's talk about my PR here: https://github.com/spack/spack/pull/23859. It's building rocm-device-libs as part of llvm-amdgpu by configuring llvm with -DLLVM_EXTERNAL_PROJECTS=device-libs.

If you checkout that pr, run spack install hip@4.2.0, what you get is get is:

$ spack find -p llvm-amdgpu@4.2.0
llvm-amdgpu@4.2.0  /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi

And indeed the bitcode is there:

$ find /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi -iname '*.bc'
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/oclc_isa_version_1033.bc
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/ocml.bc
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/hip.bc
...

Now when I used this --print-rocm-search-dirs flag on clang without other flags, what I see is:

$ /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/bin/clang++ --print-rocm-search-dirs -x hip hi.cc 
ROCm installation search path (Spack 4.2.0): /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0
ROCm installation search path: /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/lib/clang/12.0.0
ROCm installation search path: /opt/rocm
...
clang-12: error: cannot find ROCm device library. Provide its path via --rocm-path or --rocm-device-lib-path, or pass -nogpulib to build without ROCm device library.

Now can you make it such that clang will search the llvm prefix itself?

In fact, if you replace all 3 search paths JUST with CMAKE_INSTALL_PREFIX path, it will find the correct bitcode files independent of whether you're using spack or whether you're bundling the rocm packages the traditional way in with the shared prefix in /opt/rocm... So, what I'd like to see is this:

$ /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/bin/clang++ --print-rocm-search-dirs -x hip hi.cc 
ROCm installation search path: /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/

and

$ /opt/rocm/llvm/bin/clang++ --print-rocm-search-dirs -x hip hi.cc 
ROCm installation search path: /opt/rocm/llvm

Doesn't that make a whole lot more sense than informing llvm about spack?

Edit: it seems like the amdgcn directory in the distributions is in /opt/rocm/amdgcn, but moving them inside /opt/rocm/llvm/amdgcn could hopefully be an option? I mean, the omp bitcode files are already in the llvm prefix...

Finally, it doesn't settle locating hipcc (nor .hipVersion), but that shouldn't be a real issue from the point of view of a spack user, since spack already sets the -I and -L flags for you when you make your package depend on hip. Note that spack already sets many hip-related environment variables anyways when you run spack load hipcc@4.2.0...

Last comment on this: on many HPC systems you find a system install of rocm in /opt/rocm, but the user may have reasons to install rocm themselves with spack (maybe a debug build). So, another thing that searching only CMAKE_INSTALL_PREFIX for bitcode files would solve (unless overriden by env variable/flag), is accidentally falling back to /opt/rocm when the spack detection does not work.

[yaxunl]

In D97340#2778073, @haampie wrote:

Hi Yaxunl,

The patch should not cause circular dependency on HIP or device library.

I'm not saying this patch introduces a circular dependency, I'm saying you are trying to solve an already existing circular dependency (clang needs device libs at runtime, but device libs need llvm to compile).

Let's talk about my PR here: https://github.com/spack/spack/pull/23859. It's building rocm-device-libs as part of llvm-amdgpu by configuring llvm with -DLLVM_EXTERNAL_PROJECTS=device-libs.

If you checkout that pr, run spack install hip@4.2.0, what you get is get is:

$ spack find -p llvm-amdgpu@4.2.0
llvm-amdgpu@4.2.0  /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi

And indeed the bitcode is there:

$ find /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi -iname '*.bc'
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/oclc_isa_version_1033.bc
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/ocml.bc
/spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/amdgcn/bitcode/hip.bc
...

Now when I used this --print-rocm-search-dirs flag on clang without other flags, what I see is:

$ /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/bin/clang++ --print-rocm-search-dirs -x hip hi.cc 
ROCm installation search path (Spack 4.2.0): /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0
ROCm installation search path: /spack/opt/spack/linux-ubuntu20.04-zen2/gcc-10.2.0/llvm-amdgpu-4.2.0-a7jwajhh2cmn7p5djyx42lpcdfluk7wi/lib/clang/12.0.0
ROCm installation search path: /opt/rocm
...
clang-12: error: cannot find ROCm device library. Provide its path via --rocm-path or --rocm-device-lib-path, or pass -nogpulib to build without ROCm device library.

Now can you make it such that clang will search the llvm prefix itself?

Yes I can make clang to search that path for device libs. I will open a review for that and remove the old detection of device libs for spack.

I still think auto detection of HIP path is a desirable feature, especially for interactive use of clang in a shell. This also allows clang to maintain certain level of backward compatibility with old HIP runtime by detecting HIP version.

[yaxunl]

opened https://reviews.llvm.org/D103281 to fix device lib detection for spack

clang/include/clang/Driver/Options.td

 def : Flag<["-"], "fcuda-rdc">, Alias<fgpu_rdc>;
 def : Flag<["-"], "fno-cuda-rdc">, Alias<fno_gpu_rdc>;
 defm cuda_short_ptr : BoolFOption<"cuda-short-ptr",
   TargetOpts<"NVPTXUseShortPointers">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Use 32-bit pointers for accessing const/local/shared address spaces">,
   NegFlag<SetFalse>>;
 def rocm_path_EQ : Joined<["--"], "rocm-path=">, Group<i_Group>,
   HelpText<"ROCm installation path, used for finding and automatically linking required bitcode libraries.">;
+def hip_path_EQ : Joined<["--"], "hip-path=">, Group<i_Group>,
+  HelpText<"HIP runtime installation path, used for finding HIP version and adding HIP include path.">;
 def rocm_device_lib_path_EQ : Joined<["--"], "rocm-device-lib-path=">, Group<Link_Group>,
   HelpText<"ROCm device library path. Alternative to rocm-path.">;
 def : Joined<["--"], "hip-device-lib-path=">, Alias<rocm_device_lib_path_EQ>;
 def hip_device_lib_EQ : Joined<["--"], "hip-device-lib=">, Group<Link_Group>,
   HelpText<"HIP device library">;
 def hip_version_EQ : Joined<["--"], "hip-version=">,
   HelpText<"HIP version in the format of major.minor.patch">;
 def fhip_dump_offload_linker_script : Flag<["-"], "fhip-dump-offload-linker-script">,

 def print_prog_name_EQ : Joined<["-", "--"], "print-prog-name=">,
   HelpText<"Print the full program path of <name>">, MetaVarName<"<name>">;
 def print_resource_dir : Flag<["-", "--"], "print-resource-dir">,
   HelpText<"Print the resource directory pathname">;
 def print_search_dirs : Flag<["-", "--"], "print-search-dirs">,
   HelpText<"Print the paths used for finding libraries and programs">;
 def print_targets : Flag<["-", "--"], "print-targets">,
   HelpText<"Print the registered targets">;
+def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">,
+  HelpText<"Print the paths used for finding ROCm installation">;
 def private__bundle : Flag<["-"], "private_bundle">;
 def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",
   LangOpts<"POSIXThreads">, DefaultFalse,
   PosFlag<SetTrue, [], "Support POSIX threads in generated code">,
   NegFlag<SetFalse>, BothFlags<[CC1Option]>>;
 def p : Flag<["-"], "p">;
 def pie : Flag<["-"], "pie">, Group<Link_Group>;

clang/lib/Driver/ToolChains/AMDGPU.cpp

 #include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang::driver::toolchains;
 using namespace clang;
 using namespace llvm::opt;
 
+// Look for sub-directory starts with PackageName under ROCm candidate path.
+// If there is one and only one matching sub-directory found, append the
+// sub-directory to Path. If there is no matching sub-directory or there are
+// more than one matching sub-directories, diagnose them. Returns the full
+// path of the package if there is only one matching sub-directory, otherwise
+// returns an empty string.
+llvm::SmallString<0>
+RocmInstallationDetector::findSPACKPackage(const Candidate &Cand,
+                                           StringRef PackageName) {
+  if (!Cand.isSPACK())
+    return {};
+  std::error_code EC;
+  std::string Prefix = Twine(PackageName + "-" + Cand.SPACKReleaseStr).str();
+  llvm::SmallVector<llvm::SmallString<0>> SubDirs;
+  for (llvm::vfs::directory_iterator File = D.getVFS().dir_begin(Cand.Path, EC),
+                                     FileEnd;
+       File != FileEnd && !EC; File.increment(EC)) {
+    llvm::StringRef FileName = llvm::sys::path::filename(File->path());
+    if (FileName.startswith(Prefix)) {
+      SubDirs.push_back(FileName);
+      if (SubDirs.size() > 1)
+        break;
+    }
+  }
+  if (SubDirs.size() == 1) {
+    auto PackagePath = Cand.Path;
+    llvm::sys::path::append(PackagePath, SubDirs[0]);
+    return PackagePath;
+  }
+  if (SubDirs.size() == 0) {
+    unsigned DiagID = D.getDiags().getCustomDiagID(
+        DiagnosticsEngine::Error,
+        "Expecting SPACK package %0 at %1 but not found");
+    D.Diag(DiagID) << Prefix << Cand.Path;
+    return {};
+  }
+
+  assert(SubDirs.size() > 1);
+  unsigned DiagID = D.getDiags().getCustomDiagID(
+      DiagnosticsEngine::Error,
+      "Expecting one SPACK package %0 at %1 but found more");
+  D.Diag(DiagID) << Prefix << Cand.Path;
+  return {};
+}
+
 void RocmInstallationDetector::scanLibDevicePath(llvm::StringRef Path) {
   assert(!Path.empty());
 
   const StringRef Suffix(".bc");
   const StringRef Suffix2(".amdgcn.bc");
 
   std::error_code EC;
   for (llvm::vfs::directory_iterator LI = D.getVFS().dir_begin(Path, EC), LE;
bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
   }
   if (Major == ~0U || Minor == ~0U)
     return true;
   VersionMajorMinor = llvm::VersionTuple(Major, Minor);
   DetectedVersion =
       (Twine(Major) + "." + Twine(Minor) + "." + VersionPatch).str();
   return false;
 }
 
-// For candidate specified by --rocm-path we do not do strict check.
-SmallVector<RocmInstallationDetector::Candidate, 4>
+/// \returns a list of candidate directories for ROCm installation, which is
+/// cached and populated only once.
+const SmallVectorImpl<RocmInstallationDetector::Candidate> &
 RocmInstallationDetector::getInstallationPathCandidates() {
-  SmallVector<Candidate, 4> Candidates;
+
+  // Return the cached candidate list if it has already been populated.
+  if (!ROCmSearchDirs.empty())
+    return ROCmSearchDirs;
+
+  auto DoPrintROCmSearchDirs = [&]() {
+    if (PrintROCmSearchDirs)
+      for (auto Cand : ROCmSearchDirs) {
+        llvm::errs() << "ROCm installation search path";
+        if (Cand.isSPACK())
+          llvm::errs() << " (Spack " << Cand.SPACKReleaseStr << ")";
+        llvm::errs() << ": " << Cand.Path << '\n';
+      }
+  };
+
+  // For candidate specified by --rocm-path we do not do strict check, i.e.,
+  // checking existence of HIP version file and device library files.
   if (!RocmPathArg.empty()) {
-    Candidates.emplace_back(RocmPathArg.str());
-    return Candidates;
+    ROCmSearchDirs.emplace_back(RocmPathArg.str());
+    DoPrintROCmSearchDirs();
+    return ROCmSearchDirs;
   }
 
   // Try to find relative to the compiler binary.
   const char *InstallDir = D.getInstalledDir();
 
   // Check both a normal Unix prefix position of the clang binary, as well as
   // the Windows-esque layout the ROCm packages use with the host architecture
   // subdirectory of bin.
-
+  auto DeduceROCmPath = [](StringRef ClangPath) {
     // Strip off directory (usually bin)
-  StringRef ParentDir = llvm::sys::path::parent_path(InstallDir);
+    StringRef ParentDir = llvm::sys::path::parent_path(ClangPath);
     StringRef ParentName = llvm::sys::path::filename(ParentDir);
 
     // Some builds use bin/{host arch}, so go up again.
     if (ParentName == "bin") {
       ParentDir = llvm::sys::path::parent_path(ParentDir);
       ParentName = llvm::sys::path::filename(ParentDir);
     }
 
+    // Detect ROCm packages built with SPACK.
+    // clang is installed at
+    // <rocm_root>/llvm-amdgpu-<rocm_release_string>-<hash>/bin directory.
+    // We only consider the parent directory of llvm-amdgpu package as ROCm
+    // installation candidate for SPACK.
+    if (ParentName.startswith("llvm-amdgpu-")) {
+      auto SPACKPostfix =
+          ParentName.drop_front(strlen("llvm-amdgpu-")).split('-');
+      auto SPACKReleaseStr = SPACKPostfix.first;
+      if (!SPACKReleaseStr.empty()) {
+        ParentDir = llvm::sys::path::parent_path(ParentDir);
+        return Candidate(ParentDir.str(), /*StrictChecking=*/true,
+                         SPACKReleaseStr);
+      }
+    }
+
     // Some versions of the rocm llvm package install to /opt/rocm/llvm/bin
-  if (ParentName == "llvm")
+    // Some versions of the aomp package install to /opt/rocm/aomp/bin
+    if (ParentName == "llvm" || ParentName.startswith("aomp"))
       ParentDir = llvm::sys::path::parent_path(ParentDir);
 
-  Candidates.emplace_back(ParentDir.str(), /*StrictChecking=*/true);
+    return Candidate(ParentDir.str(), /*StrictChecking=*/true);
+  };
+
+  // Deduce ROCm path by the path used to invoke clang. Do not resolve symbolic
+  // link of clang itself.
+  ROCmSearchDirs.emplace_back(DeduceROCmPath(InstallDir));
+
+  // Deduce ROCm path by the real path of the invoked clang, resolving symbolic
+  // link of clang itself.
+  llvm::SmallString<256> RealClangPath;
+  llvm::sys::fs::real_path(D.getClangProgramPath(), RealClangPath);
+  auto ParentPath = llvm::sys::path::parent_path(RealClangPath);
+  if (ParentPath != InstallDir)
+    ROCmSearchDirs.emplace_back(DeduceROCmPath(ParentPath));
 
   // Device library may be installed in clang resource directory.
-  Candidates.emplace_back(D.ResourceDir, /*StrictChecking=*/true);
+  ROCmSearchDirs.emplace_back(D.ResourceDir,
+                              /*StrictChecking=*/true);
 
-  Candidates.emplace_back(D.SysRoot + "/opt/rocm", /*StrictChecking=*/true);
-  return Candidates;
+  ROCmSearchDirs.emplace_back(D.SysRoot + "/opt/rocm",
+                              /*StrictChecking=*/true);
+  DoPrintROCmSearchDirs();
+  return ROCmSearchDirs;
 }
 
 RocmInstallationDetector::RocmInstallationDetector(
     const Driver &D, const llvm::Triple &HostTriple,
     const llvm::opt::ArgList &Args, bool DetectHIPRuntime, bool DetectDeviceLib)
     : D(D) {
   RocmPathArg = Args.getLastArgValue(clang::driver::options::OPT_rocm_path_EQ);
+  PrintROCmSearchDirs =
+      Args.hasArg(clang::driver::options::OPT_print_rocm_search_dirs);
   RocmDeviceLibPathArg =
       Args.getAllArgValues(clang::driver::options::OPT_rocm_device_lib_path_EQ);
+  HIPPathArg = Args.getLastArgValue(clang::driver::options::OPT_hip_path_EQ);
   if (auto *A = Args.getLastArg(clang::driver::options::OPT_hip_version_EQ)) {
     HIPVersionArg = A->getValue();
     unsigned Major = 0;
     unsigned Minor = 0;
     SmallVector<StringRef, 3> Parts;
     HIPVersionArg.split(Parts, '.');
     if (Parts.size())
       Parts[0].getAsInteger(0, Major);
if (!LibDevicePath.empty()) {
     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 &ROCmDirs = getInstallationPathCandidates();
+  for (const auto &Candidate : ROCmDirs) {
     auto CandidatePath = Candidate.Path;
 
     // Check device library exists at the given path.
     auto CheckDeviceLib = [&](StringRef Path) {
       bool CheckLibDevice = (!NoBuiltinLibs || Candidate.StrictChecking);
       if (CheckLibDevice && !FS.exists(Path))
         return false;
 
for (const auto &Candidate : ROCmDirs) {
     static constexpr std::array<const char *, 2> SubDirsList[] = {
         {"amdgcn", "bitcode"},
         {"lib", ""},
         {"lib", "bitcode"},
     };
 
     // Make a path by appending sub-directories to InstallPath.
     auto MakePath = [&](const llvm::ArrayRef<const char *> &SubDirs) {
-      auto Path = CandidatePath;
+      // Device library built by SPACK is installed to
+      // <rocm_root>/rocm-device-libs-<rocm_release_string>-<hash> directory.
+      auto SPACKPath = findSPACKPackage(Candidate, "rocm-device-libs");
+      auto Path = SPACKPath.empty() ? CandidatePath : SPACKPath;
       for (auto SubDir : SubDirs)
         llvm::sys::path::append(Path, SubDir);
       return Path;
     };
 
     for (auto SubDirs : SubDirsList) {
       LibDevicePath = MakePath(SubDirs);
       HasDeviceLibrary = CheckDeviceLib(LibDevicePath);
       if (HasDeviceLibrary)
         return;
     }
   }
 }
 
 void RocmInstallationDetector::detectHIPRuntime() {
-  auto Candidates = getInstallationPathCandidates();
+  SmallVector<Candidate, 4> HIPSearchDirs;
+  if (!HIPPathArg.empty())
+    HIPSearchDirs.emplace_back(HIPPathArg.str(), /*StrictChecking=*/true);
+  else
+    HIPSearchDirs.append(getInstallationPathCandidates());
   auto &FS = D.getVFS();
 
-  for (const auto &Candidate : Candidates) {
+  for (const auto &Candidate : HIPSearchDirs) {
     InstallPath = Candidate.Path;
     if (InstallPath.empty() || !FS.exists(InstallPath))
       continue;
+    // HIP runtime built by SPACK is installed to
+    // <rocm_root>/hip-<rocm_release_string>-<hash> directory.
+    auto SPACKPath = findSPACKPackage(Candidate, "hip");
+    InstallPath = SPACKPath.empty() ? InstallPath : SPACKPath;
 
     BinPath = InstallPath;
     llvm::sys::path::append(BinPath, "bin");
     IncludePath = InstallPath;
     llvm::sys::path::append(IncludePath, "include");
     LibPath = InstallPath;
     llvm::sys::path::append(LibPath, "lib");
 
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> VersionFile =

clang/lib/Driver/ToolChains/ROCm.h

StringRef get(bool Enabled) const {
       return Enabled ? On : Off;
     }
   };
 
   // Installation path candidate.
   struct Candidate {
     llvm::SmallString<0> Path;
     bool StrictChecking;
-
-    Candidate(std::string Path, bool StrictChecking = false)
-        : Path(Path), StrictChecking(StrictChecking) {}
+    // Release string for ROCm packages built with SPACK if not empty. The
+    // installation directories of ROCm packages built with SPACK follow the
+    // convention <package_name>-<rocm_release_string>-<hash>.
+    std::string SPACKReleaseStr;
+
+    bool isSPACK() const { return !SPACKReleaseStr.empty(); }
+    Candidate(std::string Path, bool StrictChecking = false,
+              StringRef SPACKReleaseStr = {})
+        : Path(Path), StrictChecking(StrictChecking),
+          SPACKReleaseStr(SPACKReleaseStr.str()) {}
   };
 
   const Driver &D;
   bool HasHIPRuntime = false;
   bool HasDeviceLibrary = false;
 
   // Default version if not detected or specified.
   const unsigned DefaultVersionMajor = 3;
   const unsigned DefaultVersionMinor = 5;
   const char *DefaultVersionPatch = "0";
 
   // The version string in Major.Minor.Patch format.
   std::string DetectedVersion;
   // Version containing major and minor.
   llvm::VersionTuple VersionMajorMinor;
   // Version containing patch.
   std::string VersionPatch;
 
   // ROCm path specified by --rocm-path.
   StringRef RocmPathArg;
   // ROCm device library paths specified by --rocm-device-lib-path.
   std::vector<std::string> RocmDeviceLibPathArg;
+  // HIP runtime path specified by --hip-path.
+  StringRef HIPPathArg;
   // HIP version specified by --hip-version.
   StringRef HIPVersionArg;
   // Wheter -nogpulib is specified.
   bool NoBuiltinLibs = false;
 
   // Paths
   SmallString<0> InstallPath;
   SmallString<0> BinPath;
private:
 
   // Libraries swapped based on compile flags.
   ConditionalLibrary WavefrontSize64;
   ConditionalLibrary FiniteOnly;
   ConditionalLibrary UnsafeMath;
   ConditionalLibrary DenormalsAreZero;
   ConditionalLibrary CorrectlyRoundedSqrt;
 
+  // Cache ROCm installation search paths.
+  SmallVector<Candidate, 4> ROCmSearchDirs;
+  bool PrintROCmSearchDirs;
+
   bool allGenericLibsValid() const {
     return !OCML.empty() && !OCKL.empty() && !OpenCL.empty() && !HIP.empty() &&
            WavefrontSize64.isValid() && FiniteOnly.isValid() &&
            UnsafeMath.isValid() && DenormalsAreZero.isValid() &&
            CorrectlyRoundedSqrt.isValid();
   }
 
   void scanLibDevicePath(llvm::StringRef Path);
   bool parseHIPVersionFile(llvm::StringRef V);
-  SmallVector<Candidate, 4> getInstallationPathCandidates();
+  const SmallVectorImpl<Candidate> &getInstallationPathCandidates();
+
+  /// Find the path to a SPACK package under the ROCm candidate installation
+  /// directory if the candidate is a SPACK ROCm candidate. \returns empty
+  /// string if the candidate is not SPACK ROCm candidate or the requested
+  /// package is not found.
+  llvm::SmallString<0> findSPACKPackage(const Candidate &Cand,
+                                        StringRef PackageName);
 
 public:
   RocmInstallationDetector(const Driver &D, const llvm::Triple &HostTriple,
                            const llvm::opt::ArgList &Args,
                            bool DetectHIPRuntime = true,
                            bool DetectDeviceLib = false);
 
   /// Get file paths of default bitcode libraries common to AMDGPU based

clang/test/Driver/Inputs/rocm-spack/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/bin/.hipVersion

+# NOTE: The trailing whitespace is added on purpose to verify that these
+# whitespaces are trimmed before paring.
+HIP_VERSION_MAJOR=4 
+HIP_VERSION_MINOR=0
+HIP_VERSION_PATCH=20214-a2917cd

clang/test/Driver/rocm-detect.hip

 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
+// UNSUPPORTED: system-windows
 
 // Make sure the appropriate device specific library is available.
 
 // We don't include every target in the test directory, so just pick a valid
 // target not included in the test.
 
 // RUN: %clang -### -v -target x86_64-linux-gnu --cuda-gpu-arch=gfx902 \
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,GFX902-DEFAULTLIBS %s
 
 // Should not interpret -nostdlib as disabling offload libraries.
 // RUN: %clang -### -v -target x86_64-linux-gnu --cuda-gpu-arch=gfx902 -nostdlib \
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,GFX902-DEFAULTLIBS %s
 
-
 // RUN: %clang -### -v -target x86_64-linux-gnu --cuda-gpu-arch=gfx902 -nogpulib \
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,NODEFAULTLIBS %s
 
+// Test ROCm installation built by SPACK by invoke clang at %T/rocm-spack/llvm-amdgpu-*
+// directory through a soft link.
+
+// RUN: rm -rf %T/rocm-spack
+// RUN: cp -r %S/Inputs/rocm-spack %T
+// RUN: ln -fs %clang %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 --print-rocm-search-dirs %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=SPACK %s
+
+// Test SPACK installation with multiple hip and rocm-device-libs packages of the same
+// ROCm release. Clang cannot determine which one to use and emit diagnostics. --hip-path
+// and --rocm-device-lib-path can be used to specify them.
+
+// RUN: cp -r %T/rocm-spack/hip-* %T/rocm-spack/hip-4.0.0-abcd
+// RUN: cp -r %T/rocm-spack/rocm-device-libs-* %T/rocm-spack/rocm-device-libs-4.0.0-efgh
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=SPACK-MULT %s
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 \
+// RUN:   --hip-path=%T/rocm-spack/hip-4.0.0-abcd \
+// RUN:   --rocm-device-lib-path=%T/rocm-spack/rocm-device-libs-4.0.0-efgh/amdgcn/bitcode \
+// RUN:    %s 2>&1 | FileCheck -check-prefixes=SPACK-SET %s
+
+// Test invalid SPACK ROCm installation missing hip and rocm-device-libs packages.
+
+// RUN: rm -rf %T/rocm-spack/hip-*
+// RUN: rm -rf %T/rocm-spack/rocm-device-libs-*
+// RUN: %T/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
+// RUN:   -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 %s 2>&1 \
+// RUN:   | FileCheck -check-prefixes=SPACK-MISS %s
 
 // GFX902-DEFAULTLIBS: error: cannot find ROCm device library for gfx902. Provide its path via --rocm-path or --rocm-device-lib-path, or pass -nogpulib to build without ROCm device library
 
 // NODEFAULTLIBS-NOT: error: cannot find
 
 // COMMON: "-triple" "amdgcn-amd-amdhsa"
+
+// SPACK: ROCm installation search path (Spack 4.0.0): [[DIR:.*]]
+// SPACK: ROCm installation search path: [[CLANG:.*]]
+// SPACK: ROCm installation search path: [[CLANG]]/lib/clang/{{[0-9.]+}}
+// SPACK: ROCm installation search path: /opt/rocm
+// SPACK: InstalledDir: [[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
+// SPACK: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
+// SPACK: "-triple" "amdgcn-amd-amdhsa"
+// SPACK-SAME: "-mlink-builtin-bitcode" "[[DIR]]/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc"
+// SPACK-SAME: "-internal-isystem" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"
+
+// SPACK-MULT: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
+// SPACK-MULT-DAG: Expecting one SPACK package hip-4.0.0 at [[DIR]] but found more
+// SPACK-MULT-DAG: Expecting one SPACK package rocm-device-libs-4.0.0 at [[DIR]] but found more
+// SPACK-MULT-NOT: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
+// SPACK-MULT-NOT: "-mlink-builtin-bitcode" "[[DIR]]/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc"
+// SPACK-MULT-NOT: "-internal-isystem" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"
+
+// SPACK-SET: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
+// SPACK-SET: Found HIP installation: [[DIR]]/hip-4.0.0-abcd, version 4.0.20214-a2917cd
+// SPACK-SET: "-triple" "amdgcn-amd-amdhsa"
+// SPACK-SET-SAME: "-mlink-builtin-bitcode" "[[DIR]]/rocm-device-libs-4.0.0-efgh/amdgcn/bitcode/hip.bc"
+// SPACK-SET-SAME: "-internal-isystem" "[[DIR]]/hip-4.0.0-abcd/include"
+
+// SPACK-MISS: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
+// SPACK-MISS-DAG: Expecting SPACK package hip-4.0.0 at [[DIR]] but not found
+// SPACK-MISS-DAG: Expecting SPACK package rocm-device-libs-4.0.0 at [[DIR]] but not found
+// SPACK-MISS-NOT: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
+// SPACK-MISS-NOT: "-mlink-builtin-bitcode" "[[DIR]]/rocm-device-libs-4.0.0-6wnyzz4hgl3hr7uswasnagt7j2adctbs/amdgcn/bitcode/hip.bc"
+// SPACK-MISS-NOT: "-internal-isystem" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"