[offload-arch] Fix HIP DLL discovery and loading on Windows (#194063)

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).

---------

Co-authored-by: Samiii777 <Samiii777@users.noreply.github.com>

clang/tools/offload-arch/AMDGPUArchByHIP.cpp

@@ -22,6 +22,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/VersionTuple.h"
+#include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <string>
@@ -82,13 +83,40 @@ 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) {
+    if (sys::path::root_path(Parent) == StringRef(Parent))
+      break;
+    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);
+    Parent = SmallString<256>(sys::path::parent_path(Parent));
+  }
+  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];
@@ -129,7 +157,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 +210,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 +220,28 @@ 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 || !sys::path::is_absolute(Path))
+    return;
+  SmallVector<UTF16, 256> WPath;
+  if (!convertUTF8ToUTF16String(Path, WPath))
+    return;
+  WPath.push_back(0);
+  PinnedModule = LoadLibraryExW(reinterpret_cast<LPCWSTR>(WPath.data()),
+                                nullptr, LOAD_WITH_ALTERED_SEARCH_PATH);
+  DWORD Err = GetLastError();
+  if (!PinnedModule && Verbose)
+    WithColor::note() << "priming LoadLibraryExW failed for " << Path
+                      << " (error " << Err << ")\n";
+}
+#endif
+
 int printGPUsByHIP() {
   auto [DynamicHIPPath, IsFallback] = findNewestHIPDLL();
 
@@ -200,6 +253,11 @@ int printGPUsByHIP() {
   }
 
   std::string ErrMsg;
+#ifdef _WIN32
+  // Prime DLL load so transitive deps resolve from its directory.
+  if (!IsFallback)
+    primeLibraryLoad(DynamicHIPPath);
+#endif
   auto DynlibHandle = std::make_unique<llvm::sys::DynamicLibrary>(
       llvm::sys::DynamicLibrary::getPermanentLibrary(DynamicHIPPath.c_str(),
                                                      &ErrMsg));
@@ -356,6 +414,7 @@ int printGPUsByHIP() {
       break;
     case HipApiVersion::Unversioned:
       OK = TryUnversioned(I);
+      break;
     }
 
     if (ArchName.empty()) {

clang/unittests/CMakeLists.txt

@@ -110,6 +110,7 @@ add_subdirectory(Support)
 if (CLANG_ENABLE_CIR)
   add_subdirectory(CIR)
 endif()
+add_subdirectory(offload-arch)
 
 # If we're doing a single merged clang unit test binary, add that target after
 # all the previous subdirectories have been processed.

clang/unittests/offload-arch/CMakeLists.txt

@@ -0,0 +1,10 @@
+if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
+  add_distinct_clang_unittest(OffloadArchTests
+    OffloadArchTest.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../../tools/offload-arch/AMDGPUArchByHIP.cpp
+    CLANG_LIBS
+      clangBasic
+    LLVM_COMPONENTS
+      Support
+    )
+endif()

clang/unittests/offload-arch/OffloadArchTest.cpp

@@ -0,0 +1,114 @@
+//===-- 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 "llvm/Support/CommandLine.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;
+
+cl::opt<bool> Verbose("offload-arch-test-verbose", cl::Hidden, cl::init(false));
+
+#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("C:/\xC3\xBCser/\xC3\xA4pp/bin");
+  EXPECT_GE(Paths.size(), 1u);
+}
+
+TEST(CandidateBinPaths, UnicodePathDedup) {
+  auto Paths =
+      getCandidateBinPaths("C:/\xE6\x97\xA5\xE6\x9C\xAC\xE8\xAA\x9E/lib/bin");
+  // Should produce entries without crashing on CJK characters.
+  EXPECT_GE(Paths.size(), 1u);
+}
+
+TEST(CandidateBinPaths, NoDriveRootBin) {
+  auto Paths = getCandidateBinPaths("C:\\Program Files\\AMD\\HIP\\bin");
+  for (const auto &P : Paths)
+    EXPECT_FALSE(StringRef(P).equals_insensitive("C:/bin"))
+        << "Drive-root bin/ must not appear (DLL planting risk)";
+}
+
+#endif // _WIN32