Skip to content

Commit bd788db

Browse files
authored
[AMDGPU] Remove detection of hip runtime for Spack (#133263)
There is special logic to detect the hip runtime when llvm is installed with Spack. It works by matching the install prefix of llvm against `llvm-amdgpu-*` followed by effectively globbing for ``` <llvm dir>/../hip-x.y.z-*/ ``` and checking there is exactly one such directory. I would suggest to remove autodetection for the following reasons: 1. In the Spack ecosystem it's by design that every package lives in its own prefix, and can only know where its dependencies are installed, it has no clue what its dependents are and where they are installed. This heuristic detection breaks that invariant, since `hip` is a dependent of `llvm`, and can be surprising to Spack users. 2. The detection can lead to false positives, since users can be using an llvm installed "upstream" with their own build of hip locally, and they may not realize that clang is picking up upstream hip instead of their local copy. 3. It only works if the directory name is `llvm-amdgpu-*` which happens to be the name of AMD's fork of `llvm`, so it makes no sense that this code lives in the main LLVM repo for which the Spack package name is `llvm`. Feels wrong that LLVM knows about Spack package names, which can change over time. 4. Users can change the install directory structure, meaning that this detection is not robust under config changes in Spack.
1 parent 8a691cc commit bd788db

29 files changed

+1
-118
lines changed

clang/lib/Driver/ToolChains/AMDGPU.cpp

Lines changed: 1 addition & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -32,48 +32,6 @@ using namespace clang::driver::toolchains;
3232
using namespace clang;
3333
using namespace llvm::opt;
3434

35-
// Look for sub-directory starts with PackageName under ROCm candidate path.
36-
// If there is one and only one matching sub-directory found, append the
37-
// sub-directory to Path. If there is no matching sub-directory or there are
38-
// more than one matching sub-directories, diagnose them. Returns the full
39-
// path of the package if there is only one matching sub-directory, otherwise
40-
// returns an empty string.
41-
llvm::SmallString<0>
42-
RocmInstallationDetector::findSPACKPackage(const Candidate &Cand,
43-
StringRef PackageName) {
44-
if (!Cand.isSPACK())
45-
return {};
46-
std::error_code EC;
47-
std::string Prefix = Twine(PackageName + "-" + Cand.SPACKReleaseStr).str();
48-
llvm::SmallVector<llvm::SmallString<0>> SubDirs;
49-
for (llvm::vfs::directory_iterator File = D.getVFS().dir_begin(Cand.Path, EC),
50-
FileEnd;
51-
File != FileEnd && !EC; File.increment(EC)) {
52-
llvm::StringRef FileName = llvm::sys::path::filename(File->path());
53-
if (FileName.starts_with(Prefix)) {
54-
SubDirs.push_back(FileName);
55-
if (SubDirs.size() > 1)
56-
break;
57-
}
58-
}
59-
if (SubDirs.size() == 1) {
60-
auto PackagePath = Cand.Path;
61-
llvm::sys::path::append(PackagePath, SubDirs[0]);
62-
return PackagePath;
63-
}
64-
if (SubDirs.size() == 0 && Verbose) {
65-
llvm::errs() << "SPACK package " << Prefix << " not found at " << Cand.Path
66-
<< '\n';
67-
return {};
68-
}
69-
70-
if (SubDirs.size() > 1 && Verbose) {
71-
llvm::errs() << "Cannot use SPACK package " << Prefix << " at " << Cand.Path
72-
<< " due to multiple installations for the same version\n";
73-
}
74-
return {};
75-
}
76-
7735
void RocmInstallationDetector::scanLibDevicePath(llvm::StringRef Path) {
7836
assert(!Path.empty());
7937

@@ -187,10 +145,7 @@ RocmInstallationDetector::getInstallationPathCandidates() {
187145
auto DoPrintROCmSearchDirs = [&]() {
188146
if (PrintROCmSearchDirs)
189147
for (auto Cand : ROCmSearchDirs) {
190-
llvm::errs() << "ROCm installation search path";
191-
if (Cand.isSPACK())
192-
llvm::errs() << " (Spack " << Cand.SPACKReleaseStr << ")";
193-
llvm::errs() << ": " << Cand.Path << '\n';
148+
llvm::errs() << "ROCm installation search path: " << Cand.Path << '\n';
194149
}
195150
};
196151

@@ -226,22 +181,6 @@ RocmInstallationDetector::getInstallationPathCandidates() {
226181
ParentName = llvm::sys::path::filename(ParentDir);
227182
}
228183

229-
// Detect ROCm packages built with SPACK.
230-
// clang is installed at
231-
// <rocm_root>/llvm-amdgpu-<rocm_release_string>-<hash>/bin directory.
232-
// We only consider the parent directory of llvm-amdgpu package as ROCm
233-
// installation candidate for SPACK.
234-
if (ParentName.starts_with("llvm-amdgpu-")) {
235-
auto SPACKPostfix =
236-
ParentName.drop_front(strlen("llvm-amdgpu-")).split('-');
237-
auto SPACKReleaseStr = SPACKPostfix.first;
238-
if (!SPACKReleaseStr.empty()) {
239-
ParentDir = llvm::sys::path::parent_path(ParentDir);
240-
return Candidate(ParentDir.str(), /*StrictChecking=*/true,
241-
SPACKReleaseStr);
242-
}
243-
}
244-
245184
// Some versions of the rocm llvm package install to /opt/rocm/llvm/bin
246185
// Some versions of the aomp package install to /opt/rocm/aomp/bin
247186
if (ParentName == "llvm" || ParentName.starts_with("aomp"))
@@ -462,10 +401,6 @@ void RocmInstallationDetector::detectHIPRuntime() {
462401
InstallPath = Candidate.Path;
463402
if (InstallPath.empty() || !FS.exists(InstallPath))
464403
continue;
465-
// HIP runtime built by SPACK is installed to
466-
// <rocm_root>/hip-<rocm_release_string>-<hash> directory.
467-
auto SPACKPath = findSPACKPackage(Candidate, "hip");
468-
InstallPath = SPACKPath.empty() ? InstallPath : SPACKPath;
469404

470405
BinPath = InstallPath;
471406
llvm::sys::path::append(BinPath, "bin");

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

Lines changed: 0 additions & 5 deletions
This file was deleted.

clang/test/Driver/Inputs/rocm-spack/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include/hip/hip_runtime.h

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/asanrtl.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/ockl.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_abi_version_400.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_abi_version_500.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_abi_version_600.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_correctly_rounded_sqrt_off.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_correctly_rounded_sqrt_on.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_daz_opt_off.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_daz_opt_on.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_finite_only_off.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_finite_only_on.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1010.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1011.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_1012.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_803.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_900.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_isa_version_908.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_unsafe_math_off.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_unsafe_math_on.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_wavefrontsize64_off.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/oclc_wavefrontsize64_on.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/ocml.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/opencl.bc

Whitespace-only changes.

clang/test/Driver/Inputs/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/.keep

Whitespace-only changes.

clang/test/Driver/rocm-detect.hip

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -94,33 +94,6 @@
9494
// RUN: --print-rocm-search-dirs %s 2>&1 \
9595
// RUN: | FileCheck -check-prefixes=ROCM-REL %s
9696

97-
// Test ROCm installation built by SPACK by invoke clang at %t/rocm-spack/llvm-amdgpu-*
98-
// directory through a soft link.
99-
100-
// RUN: rm -rf %t/rocm-spack
101-
// RUN: cp -r %S/Inputs/rocm-spack %t
102-
// RUN: ln -fs %clang %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang
103-
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -no-canonical-prefixes -v \
104-
// RUN: -resource-dir=%t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/lib/clang \
105-
// RUN: -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 --print-rocm-search-dirs %s 2>&1 \
106-
// RUN: | FileCheck -check-prefixes=SPACK %s
107-
108-
// Test SPACK installation with multiple hip and rocm-device-libs packages of the same
109-
// ROCm release. --hip-path and --rocm-device-lib-path can be used to specify them.
110-
111-
// RUN: cp -r %t/rocm-spack/hip-* %t/rocm-spack/hip-4.0.0-abcd
112-
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -no-canonical-prefixes -v \
113-
// RUN: -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 \
114-
// RUN: --hip-path=%t/rocm-spack/hip-4.0.0-abcd \
115-
// RUN: %s 2>&1 | FileCheck -check-prefixes=SPACK-SET %s
116-
117-
// Test invalid SPACK ROCm installation missing hip and rocm-device-libs packages.
118-
119-
// RUN: rm -rf %t/rocm-spack/hip-*
120-
// RUN: rm -rf %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn
121-
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang --version 2>&1 \
122-
// RUN: | FileCheck -check-prefixes=SPACK-MISS-SILENT %s
123-
12497
// 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
12598

12699
// NODEFAULTLIBS-NOT: error: cannot find
@@ -145,23 +118,3 @@
145118

146119
// ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm
147120
// ROCM-REL: ROCm installation search path: {{.*}}/opt/rocm-3.10.0
148-
149-
// SPACK: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
150-
// SPACK: ROCm installation search path (Spack 4.0.0): [[DIR]]
151-
// SPACK: ROCm installation search path: [[CLANG:.*]]
152-
// SPACK: ROCm installation search path: [[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z
153-
// SPACK: ROCm installation search path: [[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/lib/clang
154-
// SPACK: ROCm installation search path: /opt/rocm
155-
// SPACK: Found HIP installation: [[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5, version 4.0.20214-a2917cd
156-
// SPACK: "-triple" "amdgcn-amd-amdhsa"
157-
// SPACK-SAME: "-mlink-builtin-bitcode" "[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc"
158-
// SPACK-SAME: "-idirafter" "[[DIR]]/hip-4.0.0-5f63slrursbrvfe2txrrjkynbsywsob5/include"
159-
160-
// SPACK-SET: InstalledDir: [[DIR:.*]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin
161-
// SPACK-SET: Found HIP installation: [[DIR]]/hip-4.0.0-abcd, version 4.0.20214-a2917cd
162-
// SPACK-SET: "-triple" "amdgcn-amd-amdhsa"
163-
// SPACK-SET-SAME: "-mlink-builtin-bitcode" "[[DIR]]/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/amdgcn/bitcode/hip.bc"
164-
// SPACK-SET-SAME: "-idirafter" "[[DIR]]/hip-4.0.0-abcd/include"
165-
166-
// SPACK-MISS-SILENT-NOT: SPACK package hip-{{.*}} not found at
167-
// SPACK-MISS-SILENT-NOT: Found HIP installation

0 commit comments

Comments
 (0)