Skip to content

Commit c56088a

Browse files
authored
[windows] Patch hipcc to use HIP_PATH env var at lower priority. (#1201)
Fixes #1200. Sent upstream for review as well at ROCm/llvm-project#289. The HIP SDK sets the `HIP_PATH` environment variable, which hipcc and hipconfig were using instead of neighboring files. This resulted in errors like this when the HIP SDK was installed: ``` λ .\build\compiler\hipcc\dist\bin\hipcc --version 'C:\Program' is not recognized as an internal or external command, operable program or batch file. The system cannot find the path specified. 'nvcc' is not recognized as an internal or external command, operable program or batch file. Device not supported - Defaulting to AMD HIP version: 6.2.41512-db3292736 The system cannot find the path specified. failed to execute:""C:\Program Files\AMD\ROCm\6.2\lib\llvm\bin\clang.exe" --driver-mode=g++ -O3 -fuse-ld=lld --ld-path="C:\Program Files\AMD\ROCm\6.2\lib\llvm\bin\lld-link.exe" -Llib --hip-link --version" ``` A robust way to handle this would be to stamp all information about related tools into the tools themselves during packaging, rather than use global state like environment variables or heuristics like scanning the file system. I think once we've progressed further towards rolling out the new ROCm "superrepos" and open source CI/CD we can make those deeper changes. For now, I've extended the brittle logic with another special case.
1 parent 2d50ed8 commit c56088a

5 files changed

+78
-21
lines changed

patches/amd-mainline/llvm-project/0001-Enable-MSVC-support-in-comgr.patch

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
From 2fcbc3fc1fe34eae9fd647b79d6b0a15252c06ae Mon Sep 17 00:00:00 2001
1+
From bcdc455a08df8527e8946eb33fee714c28e30836 Mon Sep 17 00:00:00 2001
22
From: Scott <[email protected]>
33
Date: Fri, 14 Feb 2025 15:22:48 -0800
4-
Subject: [PATCH 1/3] Enable MSVC support in comgr.
4+
Subject: [PATCH 1/5] Enable MSVC support in comgr.
55

66
---
77
amd/comgr/CMakeLists.txt | 7 ++++++-
88
1 file changed, 6 insertions(+), 1 deletion(-)
99

1010
diff --git a/amd/comgr/CMakeLists.txt b/amd/comgr/CMakeLists.txt
11-
index ed01d21cea30..e643256f2a89 100644
11+
index 2eeb8c2ac6ac..8b16fb9377e1 100644
1212
--- a/amd/comgr/CMakeLists.txt
1313
+++ b/amd/comgr/CMakeLists.txt
1414
@@ -228,7 +228,12 @@ elseif (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
@@ -26,5 +26,5 @@ index ed01d21cea30..e643256f2a89 100644
2626
endif()
2727

2828
--
29-
2.43.0
29+
2.47.1.windows.2
3030

patches/amd-mainline/llvm-project/0001-hipcc-fix-default-include-path-on-Windows-and-adapt-.patch renamed to patches/amd-mainline/llvm-project/0002-hipcc-fix-default-include-path-on-Windows-and-adapt-.patch

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
From 993fefc7d2295fd34219534c5971c50758526d96 Mon Sep 17 00:00:00 2001
1+
From d70e7d2166d478ead5d9bd3f6e4b1408190ea666 Mon Sep 17 00:00:00 2001
22
From: Scott Tsai <[email protected]>
33
Date: Tue, 15 Apr 2025 09:30:13 +0800
4-
Subject: [PATCH 4/4] hipcc: fix default include path on Windows and adapt to
4+
Subject: [PATCH 2/5] hipcc: fix default include path on Windows and adapt to
55
TheRock rocm layout
66

77
getCppConfig in the orignial code did not add the content of
@@ -18,10 +18,10 @@ separators to make the output obviously correct.
1818
1 file changed, 22 insertions(+), 38 deletions(-)
1919

2020
diff --git a/amd/hipcc/src/hipBin_amd.h b/amd/hipcc/src/hipBin_amd.h
21-
index 550e4b248128..f66852b8352d 100644
21+
index ecea39e071b4..c74b43ff598f 100644
2222
--- a/amd/hipcc/src/hipBin_amd.h
2323
+++ b/amd/hipcc/src/hipBin_amd.h
24-
@@ -202,16 +202,11 @@ void HipBinAmd::constructCompilerPath() {
24+
@@ -201,16 +201,11 @@ void HipBinAmd::constructCompilerPath() {
2525
const EnvVariables& envVariables = getEnvVariables();
2626
if (envVariables.hipClangPathEnv_.empty()) {
2727
fs::path hipClangPath;
@@ -43,7 +43,7 @@ index 550e4b248128..f66852b8352d 100644
4343
compilerPath = hipClangPath.string();
4444
} else {
4545
compilerPath = envVariables.hipClangPathEnv_;
46-
@@ -227,32 +222,17 @@ const string& HipBinAmd::getCompilerPath() const {
46+
@@ -226,32 +221,17 @@ const string& HipBinAmd::getCompilerPath() const {
4747
void HipBinAmd::printCompilerInfo() const {
4848
const string& hipClangPath = getCompilerPath();
4949
const string& hipPath = getHipPath();
@@ -87,7 +87,7 @@ index 550e4b248128..f66852b8352d 100644
8787
}
8888

8989
string HipBinAmd::getCompilerVersion() {
90-
@@ -294,17 +274,19 @@ string HipBinAmd::getCppConfig() {
90+
@@ -293,17 +273,19 @@ string HipBinAmd::getCppConfig() {
9191
hipPathInclude = hipPath;
9292
hipPathInclude /= "include";
9393
if (isWindows()) {
@@ -108,7 +108,7 @@ index 550e4b248128..f66852b8352d 100644
108108
return cppConfig;
109109
}
110110

111-
@@ -877,7 +859,9 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
111+
@@ -874,7 +856,9 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
112112

113113
// to avoid using dk linker or MSVC linker
114114
if (isWindows()) {
@@ -120,5 +120,5 @@ index 550e4b248128..f66852b8352d 100644
120120

121121
if (!compileOnly) {
122122
--
123-
2.49.0.windows.1
123+
2.47.1.windows.2
124124

patches/amd-mainline/llvm-project/0002-HACK-Handle-ROCM-installation-layout-of-lib-llvm-bin.patch renamed to patches/amd-mainline/llvm-project/0003-HACK-Handle-ROCM-installation-layout-of-lib-llvm-bin.patch

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
From 3d9ccab2c606772c26afe5403cecd4a695bbdb72 Mon Sep 17 00:00:00 2001
1+
From 7dab0ed787cbde9d4aaaf99e24b7f33efda8f2a5 Mon Sep 17 00:00:00 2001
22
From: Stella Laurenzo <[email protected]>
33
Date: Thu, 13 Feb 2025 17:58:53 -0800
4-
Subject: [PATCH 2/3] HACK: Handle ROCM installation layout of
4+
Subject: [PATCH 3/5] HACK: Handle ROCM installation layout of
55
lib/llvm/bin/clang++.
66

77
---
88
clang/lib/Driver/ToolChains/AMDGPU.cpp | 11 ++++++++++-
99
1 file changed, 10 insertions(+), 1 deletion(-)
1010

1111
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
12-
index 5e8a949f5861..f4ee2c460481 100644
12+
index 798ea8aad6de..c45f07a6e285 100644
1313
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
1414
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
1515
@@ -244,8 +244,17 @@ RocmInstallationDetector::getInstallationPathCandidates() {
@@ -32,5 +32,5 @@ index 5e8a949f5861..f4ee2c460481 100644
3232
// and it seems ParentDir is already pointing to correct place.
3333
return Candidate(ParentDir.str(), /*StrictChecking=*/true);
3434
--
35-
2.43.0
35+
2.47.1.windows.2
3636

patches/amd-mainline/llvm-project/0003-Disable-hipcc-passing-hip-device-libs.patch renamed to patches/amd-mainline/llvm-project/0004-Disable-hipcc-passing-hip-device-libs.patch

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
From d9f595be0f7142a8509fd4b5dd82b863d4919c21 Mon Sep 17 00:00:00 2001
1+
From 0dd361bc6ae2e9ab9e19b9eeed808d719e58ad71 Mon Sep 17 00:00:00 2001
22
From: Stella Laurenzo <[email protected]>
33
Date: Thu, 13 Feb 2025 19:07:07 -0800
4-
Subject: [PATCH 3/3] Disable hipcc passing hip-device-libs.
4+
Subject: [PATCH 4/5] Disable hipcc passing hip-device-libs.
55

66
In modern times, the clang driver is far more knowledgable about this stuff and can operate without instruction.
77
---
88
amd/hipcc/src/hipBin_amd.h | 19 ++++++++++---------
99
1 file changed, 10 insertions(+), 9 deletions(-)
1010

1111
diff --git a/amd/hipcc/src/hipBin_amd.h b/amd/hipcc/src/hipBin_amd.h
12-
index a37c07836620..fb3942bee5f7 100644
12+
index c74b43ff598f..cb70026f6abf 100644
1313
--- a/amd/hipcc/src/hipBin_amd.h
1414
+++ b/amd/hipcc/src/hipBin_amd.h
15-
@@ -864,15 +864,16 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
15+
@@ -844,15 +844,16 @@ void HipBinAmd::executeHipCCCmd(vector<string> argv) {
1616
}
1717
}
1818

@@ -39,5 +39,5 @@ index a37c07836620..fb3942bee5f7 100644
3939
// to avoid using dk linker or MSVC linker
4040
if (isWindows()) {
4141
--
42-
2.43.0
42+
2.47.1.windows.2
4343

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
From cac004dd1df8e7db1bac39859aca6c2a271e906c Mon Sep 17 00:00:00 2001
2+
From: Scott Todd <[email protected]>
3+
Date: Tue, 5 Aug 2025 12:40:12 -0700
4+
Subject: [PATCH 5/5] Rework constructHipPath so HIP_PATH env var is lower
5+
priority.
6+
7+
---
8+
amd/hipcc/src/hipBin_base.h | 28 +++++++++++++++++++++++-----
9+
1 file changed, 23 insertions(+), 5 deletions(-)
10+
11+
diff --git a/amd/hipcc/src/hipBin_base.h b/amd/hipcc/src/hipBin_base.h
12+
index ea37e6fd12fc..4aa7431fdba6 100644
13+
--- a/amd/hipcc/src/hipBin_base.h
14+
+++ b/amd/hipcc/src/hipBin_base.h
15+
@@ -320,16 +320,34 @@ void HipBinBase::readEnvVariables() {
16+
17+
// constructs the HIP path
18+
void HipBinBase::constructHipPath() {
19+
- // we need to use --hip-path option
20+
+ // The --hip-path argument option takes precedence over all other settings.
21+
string hip_path_name = gethip_pathOption();
22+
if (!hip_path_name.empty()) {
23+
variables_.hipPathEnv_ = hip_path_name;
24+
- } else if (envVariables_.hipPathEnv_.empty()) {
25+
- fs::path full_path(hipcc::utils::getSelfPath());
26+
- variables_.hipPathEnv_ = (full_path.parent_path()).string();
27+
- } else {
28+
+ return;
29+
+ }
30+
+
31+
+ fs::path full_path(hipcc::utils::getSelfPath());
32+
+ fs::path parent_path = full_path.parent_path();
33+
+
34+
+ // Next, check for `../lib/llvm/bin/`, the standard ROCm install structure.
35+
+ fs::path llvm_path = parent_path / "lib" / "llvm" / "bin";
36+
+ if (fs::exists(llvm_path)) {
37+
+ variables_.hipPathEnv_ = parent_path.string();
38+
+ return;
39+
+ }
40+
+
41+
+ // Otherwise, check the HIP_PATH environment variable from the HIP SDK.
42+
+ // Normally an environment variable setting could take precedence over an
43+
+ // implicit path, but this environment variable is set by system-wide installs
44+
+ // and self-contained builds/installs should not be reading that global state.
45+
+ if (!envVariables_.hipPathEnv_.empty()) {
46+
variables_.hipPathEnv_ = envVariables_.hipPathEnv_;
47+
+ return;
48+
}
49+
+
50+
+ // Finally, fallback to the parent path (the standard ROCm install structure).
51+
+ variables_.hipPathEnv_ = parent_path.string();
52+
}
53+
54+
55+
--
56+
2.47.1.windows.2
57+

0 commit comments

Comments
 (0)