[clang] [offload-arch] Fix HIP DLL discovery and loading on Windows (PR #194063)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 27 08:47:10 PDT 2026
https://github.com/Samiii777 updated https://github.com/llvm/llvm-project/pull/194063
>From 31fe9d5f2a0e3d3eef75e5e14343235948d6a45d Mon Sep 17 00:00:00 2001
From: Samiii777 <58442200+Samiii777 at users.noreply.github.com>
Date: Fri, 24 Apr 2026 16:54:37 -0400
Subject: [PATCH 1/4] [offload-arch] Fix HIP DLL discovery and loading on
Windows
On Windows, offload-arch fails to find or loads the wrong amdhip64
DLL when running from a source-built LLVM/Clang installation where
the executable and HIP runtime are in different subdirectories.
Three fixes:
1. getSearchPaths(): walk parent directories appending /bin to each,
so layouts like <root>/lib/llvm/bin/offload-arch can discover
<root>/bin/amdhip64_*.dll. Capped at 6 levels with root detection.
Case-insensitive dedup for Windows paths.
2. findNewestHIPDLL(): use stable_sort to preserve search-path order
on version ties, so a colocated build DLL wins over a system copy.
3. printGPUsByHIP(): prime the DLL load with LoadLibraryExW and
LOAD_WITH_ALTERED_SEARCH_PATH so transitive dependencies resolve
from the DLL own directory. Uses LLVM convertUTF8ToUTF16String
for path conversion.
Also fixes two pre-existing bugs: inverted current_path error check
(line 138) and missing break in HipApiVersion switch (line 421).
---
clang/tools/offload-arch/AMDGPUArchByHIP.cpp | 77 +++++++++++--
clang/unittests/offload-arch/CMakeLists.txt | 16 +++
.../offload-arch/OffloadArchTest.cpp | 103 ++++++++++++++++++
3 files changed, 188 insertions(+), 8 deletions(-)
create mode 100644 clang/unittests/offload-arch/CMakeLists.txt
create mode 100644 clang/unittests/offload-arch/OffloadArchTest.cpp
diff --git a/clang/tools/offload-arch/AMDGPUArchByHIP.cpp b/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
index d7f6d79b135df..0ec1ee5737fc6 100644
--- a/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
@@ -25,6 +25,7 @@
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <string>
+#include <cassert>
#include <vector>
#ifdef _WIN32
@@ -82,13 +83,42 @@ static cl::opt<HipApiVersion> HipApi(
cl::init(HipApiVersion::Auto), cl::cat(AMDGPUArchByHIPCategory));
#ifdef _WIN32
+// Return candidate bin/ directories by walking parent dirs of ExeDir.
+SmallVector<std::string, 8> getCandidateBinPaths(StringRef ExeDir) {
+ SmallVector<std::string, 8> Paths;
+ Paths.push_back(sys::path::convert_to_slash(ExeDir));
+ // Search parent/bin dirs: <root>/lib/llvm/bin needs depth 2,
+ // <root>/opt/rocm/lib/llvm/bin needs 3. Cap at 6.
+ constexpr int MaxParentLevels = 6;
+ SmallString<256> Parent(sys::path::parent_path(ExeDir));
+ for (int Depth = 0; Depth < MaxParentLevels && !Parent.empty();
+ ++Depth) {
+ SmallString<256> GrandParent(sys::path::parent_path(Parent));
+ SmallString<256> Candidate(Parent);
+ sys::path::append(Candidate, "bin");
+ std::string CandStr = sys::path::convert_to_slash(Candidate);
+ auto IsDup = [&](const std::string &P) {
+ return StringRef(P).equals_insensitive(CandStr);
+ };
+ if (llvm::none_of(Paths, IsDup))
+ Paths.push_back(CandStr);
+ if (StringRef(GrandParent) == StringRef(Parent))
+ break;
+ Parent = GrandParent;
+ }
+ return Paths;
+}
+
static std::vector<std::string> getSearchPaths() {
std::vector<std::string> Paths;
// Get the directory of the current executable
if (auto MainExe = sys::fs::getMainExecutable(nullptr, nullptr);
- !MainExe.empty())
- Paths.push_back(sys::path::parent_path(MainExe).str());
+ !MainExe.empty()) {
+ StringRef ExeDir = sys::path::parent_path(MainExe);
+ auto BinPaths = getCandidateBinPaths(ExeDir);
+ Paths.insert(Paths.end(), BinPaths.begin(), BinPaths.end());
+ }
// Get the system directory
wchar_t SystemDirectory[MAX_PATH];
@@ -112,10 +142,7 @@ static std::vector<std::string> getSearchPaths() {
Paths.push_back(Utf8WindowsDir);
}
- // Get the current working directory
- SmallVector<char, 256> CWD;
- if (sys::fs::current_path(CWD))
- Paths.push_back(std::string(CWD.begin(), CWD.end()));
+ // CWD deliberately excluded — DLL planting risk.
// Get the PATH environment variable
if (std::optional<std::string> PathEnv = sys::Process::GetEnv("PATH")) {
@@ -129,7 +156,8 @@ static std::vector<std::string> getSearchPaths() {
}
// Custom comparison function for dll name
-static bool compareVersions(StringRef A, StringRef B) {
+// Returns true when A's version is greater than B's (descending order).
+bool compareVersions(StringRef A, StringRef B) {
auto ParseVersion = [](StringRef S) -> VersionTuple {
StringRef Filename = sys::path::filename(S);
size_t Pos = Filename.find_last_of('_');
@@ -181,7 +209,9 @@ static std::pair<std::string, bool> findNewestHIPDLL() {
if (DLLNames.empty())
return {"amdhip64.dll", true};
- llvm::sort(DLLNames, compareVersions);
+ // stable_sort preserves the insertion order from getSearchPaths() on
+ // version ties, so a colocated build DLL wins over a system copy.
+ llvm::stable_sort(DLLNames, compareVersions);
return {DLLNames[0], false};
#else
// On Linux, fallback to default shared object
@@ -189,6 +219,29 @@ static std::pair<std::string, bool> findNewestHIPDLL() {
#endif
}
+#ifdef _WIN32
+// Pre-load DLL with LOAD_WITH_ALTERED_SEARCH_PATH so transitive deps
+// resolve from its directory. Pinned so getPermanentLibrary reuses it.
+static void primeLibraryLoad(StringRef Path) {
+ // One DLL primed per process; subsequent calls are no-ops.
+ // Not thread-safe, but offload-arch is single-threaded.
+ static HMODULE PinnedModule = nullptr;
+ if (PinnedModule)
+ return;
+ SmallVector<wchar_t, 256> WPath;
+ assert(sys::path::is_absolute(Path) && "priming requires absolute path");
+ if (!convertUTF8ToUTF16String(Path, WPath))
+ return;
+ WPath.push_back(L'\0'); // ensure null-termination for LoadLibraryExW
+ PinnedModule = LoadLibraryExW(WPath.data(), nullptr,
+ LOAD_WITH_ALTERED_SEARCH_PATH);
+ DWORD Err = GetLastError();
+ if (!PinnedModule && Verbose)
+ errs() << "Note: priming LoadLibraryExW failed for " << Path
+ << " (error " << Err << ")\n";
+}
+#endif
+
int printGPUsByHIP() {
auto [DynamicHIPPath, IsFallback] = findNewestHIPDLL();
@@ -200,6 +253,13 @@ int printGPUsByHIP() {
}
std::string ErrMsg;
+#ifdef _WIN32
+ // Prime DLL load so transitive deps resolve from its directory.
+ if (!IsFallback) {
+ if (sys::path::is_absolute(DynamicHIPPath))
+ primeLibraryLoad(DynamicHIPPath);
+ }
+#endif
auto DynlibHandle = std::make_unique<llvm::sys::DynamicLibrary>(
llvm::sys::DynamicLibrary::getPermanentLibrary(DynamicHIPPath.c_str(),
&ErrMsg));
@@ -356,6 +416,7 @@ int printGPUsByHIP() {
break;
case HipApiVersion::Unversioned:
OK = TryUnversioned(I);
+ break;
}
if (ArchName.empty()) {
diff --git a/clang/unittests/offload-arch/CMakeLists.txt b/clang/unittests/offload-arch/CMakeLists.txt
new file mode 100644
index 0000000000000..d16a9c52ee4fe
--- /dev/null
+++ b/clang/unittests/offload-arch/CMakeLists.txt
@@ -0,0 +1,16 @@
+if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
+ set(LLVM_LINK_COMPONENTS Support)
+
+ add_clang_unittest(OffloadArchTests
+ OffloadArchTest.cpp
+ ${CMAKE_SOURCE_DIR}/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
+ )
+
+ target_include_directories(OffloadArchTests PRIVATE
+ ${CMAKE_SOURCE_DIR}/clang/tools/offload-arch
+ )
+
+ target_link_libraries(OffloadArchTests PRIVATE
+ clangBasic
+ )
+endif()
diff --git a/clang/unittests/offload-arch/OffloadArchTest.cpp b/clang/unittests/offload-arch/OffloadArchTest.cpp
new file mode 100644
index 0000000000000..9f6c1ad2cf65c
--- /dev/null
+++ b/clang/unittests/offload-arch/OffloadArchTest.cpp
@@ -0,0 +1,103 @@
+//===-- OffloadArchTest.cpp - Tests for offload-arch helpers -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include <algorithm>
+#include <string>
+
+// Defined in AMDGPUArchByHIP.cpp (non-static, compiled into this test).
+#ifdef _WIN32
+bool compareVersions(llvm::StringRef A, llvm::StringRef B);
+llvm::SmallVector<std::string, 8> getCandidateBinPaths(llvm::StringRef ExeDir);
+#endif
+
+using namespace llvm;
+
+#ifdef _WIN32
+
+// --- compareVersions ---
+
+TEST(CompareVersions, HigherVersionWins) {
+ EXPECT_TRUE(compareVersions("C:/bin/amdhip64_7.dll", "C:/bin/amdhip64_6.dll"));
+ EXPECT_FALSE(compareVersions("C:/bin/amdhip64_6.dll", "C:/bin/amdhip64_7.dll"));
+}
+
+TEST(CompareVersions, EqualVersionsReturnFalse) {
+ EXPECT_FALSE(compareVersions("C:/a/amdhip64_7.dll", "C:/b/amdhip64_7.dll"));
+}
+
+TEST(CompareVersions, MultiDigitVersions) {
+ EXPECT_TRUE(compareVersions("amdhip64_12.dll", "amdhip64_6.dll"));
+}
+
+TEST(CompareVersions, StableSortPreservesInsertionOrder) {
+ std::vector<std::string> DLLs = {
+ "C:/rocm/bin/amdhip64_7.dll",
+ "C:/Windows/System32/amdhip64_7.dll"
+ };
+ llvm::stable_sort(DLLs, compareVersions);
+ EXPECT_EQ(DLLs[0], "C:/rocm/bin/amdhip64_7.dll");
+}
+
+// --- getCandidateBinPaths ---
+
+TEST(CandidateBinPaths, FindsParentBin) {
+ auto Paths = getCandidateBinPaths("C:/root/lib/llvm/bin");
+ bool Found = false;
+ for (const auto &P : Paths)
+ if (StringRef(P).equals_insensitive("C:/root/bin"))
+ Found = true;
+ EXPECT_TRUE(Found);
+}
+
+TEST(CandidateBinPaths, NoDuplicatesWhenExeInBin) {
+ auto Paths = getCandidateBinPaths("C:/root/bin");
+ int Count = 0;
+ for (const auto &P : Paths)
+ if (StringRef(P).equals_insensitive("C:/root/bin"))
+ Count++;
+ EXPECT_EQ(Count, 1);
+}
+
+TEST(CandidateBinPaths, CaseInsensitiveDedup) {
+ // Paths differing only in case should not both appear.
+ auto Paths = getCandidateBinPaths("C:/Root/Lib/Bin");
+ int Count = 0;
+ for (const auto &P : Paths)
+ if (StringRef(P).equals_insensitive("C:/Root/bin"))
+ Count++;
+ EXPECT_LE(Count, 1);
+}
+
+TEST(CandidateBinPaths, StopsWithinBound) {
+ auto Paths = getCandidateBinPaths("C:/a/b/c/d/e/f/g/h");
+ // MaxParentLevels=6 + self = 7 max entries.
+ EXPECT_LE(Paths.size(), 7u);
+}
+
+TEST(CandidateBinPaths, RootInput) {
+ auto Paths = getCandidateBinPaths("C:/");
+ // Should produce at least 1 entry (self) and not crash.
+ EXPECT_GE(Paths.size(), 1u);
+}
+
+TEST(CandidateBinPaths, NonAsciiPath) {
+ // Paths with non-ASCII characters should not crash.
+ auto Paths = getCandidateBinPaths(u8"C:/üser/äpp/bin");
+ EXPECT_GE(Paths.size(), 1u);
+}
+
+TEST(CandidateBinPaths, UnicodePathDedup) {
+ auto Paths = getCandidateBinPaths(u8"C:/日本語/lib/bin");
+ // Should produce entries without crashing on CJK characters.
+ EXPECT_GE(Paths.size(), 1u);
+}
+
+#endif // _WIN32
>From 323ae7fa3e37a4013b6445e9fdddbe68da50bc9f Mon Sep 17 00:00:00 2001
From: Samiii777 <58442200+Samiii777 at users.noreply.github.com>
Date: Mon, 27 Apr 2026 10:59:34 -0400
Subject: [PATCH 2/4] [offload-arch] Use WithColor::note() for diagnostic
---
clang/tools/offload-arch/AMDGPUArchByHIP.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/tools/offload-arch/AMDGPUArchByHIP.cpp b/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
index 0ec1ee5737fc6..dba5dfb809f23 100644
--- a/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
@@ -23,6 +23,7 @@
#include "llvm/Support/Program.h"
#include "llvm/Support/VersionTuple.h"
#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/WithColor.h"
#include <algorithm>
#include <string>
#include <cassert>
@@ -237,8 +238,8 @@ static void primeLibraryLoad(StringRef Path) {
LOAD_WITH_ALTERED_SEARCH_PATH);
DWORD Err = GetLastError();
if (!PinnedModule && Verbose)
- errs() << "Note: priming LoadLibraryExW failed for " << Path
- << " (error " << Err << ")\n";
+ WithColor::note() << "priming LoadLibraryExW failed for " << Path
+ << " (error " << Err << ")\n";
}
#endif
>From c0330aa709ee1bf55618d5f932984486dd18d2b9 Mon Sep 17 00:00:00 2001
From: Samiii777 <58442200+Samiii777 at users.noreply.github.com>
Date: Mon, 27 Apr 2026 11:46:59 -0400
Subject: [PATCH 3/4] [NFC] clang-format AMDGPUArchByHIP.cpp
---
clang/tools/offload-arch/AMDGPUArchByHIP.cpp | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/clang/tools/offload-arch/AMDGPUArchByHIP.cpp b/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
index dba5dfb809f23..53343d299dca8 100644
--- a/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByHIP.cpp
@@ -22,11 +22,11 @@
#include "llvm/Support/Process.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/VersionTuple.h"
-#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
#include <algorithm>
-#include <string>
#include <cassert>
+#include <string>
#include <vector>
#ifdef _WIN32
@@ -92,8 +92,7 @@ SmallVector<std::string, 8> getCandidateBinPaths(StringRef ExeDir) {
// <root>/opt/rocm/lib/llvm/bin needs 3. Cap at 6.
constexpr int MaxParentLevels = 6;
SmallString<256> Parent(sys::path::parent_path(ExeDir));
- for (int Depth = 0; Depth < MaxParentLevels && !Parent.empty();
- ++Depth) {
+ for (int Depth = 0; Depth < MaxParentLevels && !Parent.empty(); ++Depth) {
SmallString<256> GrandParent(sys::path::parent_path(Parent));
SmallString<256> Candidate(Parent);
sys::path::append(Candidate, "bin");
@@ -234,12 +233,12 @@ static void primeLibraryLoad(StringRef Path) {
if (!convertUTF8ToUTF16String(Path, WPath))
return;
WPath.push_back(L'\0'); // ensure null-termination for LoadLibraryExW
- PinnedModule = LoadLibraryExW(WPath.data(), nullptr,
- LOAD_WITH_ALTERED_SEARCH_PATH);
+ PinnedModule =
+ LoadLibraryExW(WPath.data(), nullptr, LOAD_WITH_ALTERED_SEARCH_PATH);
DWORD Err = GetLastError();
if (!PinnedModule && Verbose)
WithColor::note() << "priming LoadLibraryExW failed for " << Path
- << " (error " << Err << ")\n";
+ << " (error " << Err << ")\n";
}
#endif
@@ -417,7 +416,7 @@ int printGPUsByHIP() {
break;
case HipApiVersion::Unversioned:
OK = TryUnversioned(I);
- break;
+ break;
}
if (ArchName.empty()) {
>From 7f469e428bf0b0304587ade2d56143f238ec4060 Mon Sep 17 00:00:00 2001
From: Samiii777 <58442200+Samiii777 at users.noreply.github.com>
Date: Mon, 27 Apr 2026 11:47:00 -0400
Subject: [PATCH 4/4] [NFC] clang-format OffloadArchTest.cpp
---
clang/unittests/offload-arch/OffloadArchTest.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/unittests/offload-arch/OffloadArchTest.cpp b/clang/unittests/offload-arch/OffloadArchTest.cpp
index 9f6c1ad2cf65c..26a820146144a 100644
--- a/clang/unittests/offload-arch/OffloadArchTest.cpp
+++ b/clang/unittests/offload-arch/OffloadArchTest.cpp
@@ -1,4 +1,4 @@
-//===-- OffloadArchTest.cpp - Tests for offload-arch helpers -----*- C++ -*-===//
+//===-- OffloadArchTest.cpp - Tests for offload-arch helpers ---*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -25,8 +25,10 @@ using namespace llvm;
// --- compareVersions ---
TEST(CompareVersions, HigherVersionWins) {
- EXPECT_TRUE(compareVersions("C:/bin/amdhip64_7.dll", "C:/bin/amdhip64_6.dll"));
- EXPECT_FALSE(compareVersions("C:/bin/amdhip64_6.dll", "C:/bin/amdhip64_7.dll"));
+ EXPECT_TRUE(
+ compareVersions("C:/bin/amdhip64_7.dll", "C:/bin/amdhip64_6.dll"));
+ EXPECT_FALSE(
+ compareVersions("C:/bin/amdhip64_6.dll", "C:/bin/amdhip64_7.dll"));
}
TEST(CompareVersions, EqualVersionsReturnFalse) {
@@ -38,10 +40,8 @@ TEST(CompareVersions, MultiDigitVersions) {
}
TEST(CompareVersions, StableSortPreservesInsertionOrder) {
- std::vector<std::string> DLLs = {
- "C:/rocm/bin/amdhip64_7.dll",
- "C:/Windows/System32/amdhip64_7.dll"
- };
+ std::vector<std::string> DLLs = {"C:/rocm/bin/amdhip64_7.dll",
+ "C:/Windows/System32/amdhip64_7.dll"};
llvm::stable_sort(DLLs, compareVersions);
EXPECT_EQ(DLLs[0], "C:/rocm/bin/amdhip64_7.dll");
}
More information about the cfe-commits
mailing list