[clang] [offload-arch] Report failure when KFD topology cannot be read (PR #217449)
A. Cauble via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 21 03:31:05 PDT 2026
https://github.com/accauble updated https://github.com/llvm/llvm-project/pull/217449
>From b6e2ea160e852ac65646a3e656db5c9bb3d35c9a Mon Sep 17 00:00:00 2001
From: Allyson Cauble-Chantrenne <acaublec at amd.com>
Date: Wed, 19 Aug 2026 15:18:48 -0400
Subject: [PATCH 1/3] [offload-arch] Report failure when KFD topology cannot be
read
printGPUsByKFD() did not check the error code until looping over
a `directory_iterator`. This works fine unless the `directory_iterator`
fails to construct, which is possible if the KFD topology cannot
be read (e.g., in WSL). This meant that printAMD() would get a
success but no GPU, when it should instead have gotten a failure
and fallen back on printGPUsByHIP().
This fixes that by checking the error code after constructing the
`directory_iterator`.
This also adds a number of tests surrounding the printGPUsByKFD()
that check to see if there is still success if there are GPUs or
if there are no GPUs at all.
---
clang/tools/offload-arch/AMDGPUArchByKFD.cpp | 17 +++-
clang/unittests/offload-arch/CMakeLists.txt | 25 +++--
.../offload-arch/OffloadArchTest.cpp | 96 +++++++++++++++++++
3 files changed, 127 insertions(+), 11 deletions(-)
diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
index 94ebf9073e00e..1334529a7daeb 100644
--- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
@@ -29,11 +29,20 @@ constexpr static long getMajor(long Ver) { return (Ver / 10000) % 100; }
constexpr static long getMinor(long Ver) { return (Ver / 100) % 100; }
constexpr static long getStep(long Ver) { return Ver % 100; }
-int printGPUsByKFD() {
+// Enumerate the GPUs described by the KFD topology rooted at \p NodePath.
+// Exposed so unit tests can run against a synthetic topology; it is not
+// declared in a header.
+int printGPUsByKFD(StringRef NodePath) {
SmallVector<std::pair<long, long>> Devices;
std::error_code EC;
- for (sys::fs::directory_iterator Begin(KFD_SYSFS_NODE_PATH, EC), End;
- Begin != End; Begin.increment(EC)) {
+ sys::fs::directory_iterator Begin(NodePath, EC), End;
+
+ // Check if we could construct the directory_iterator, which can fail if
+ // there is no KFD driver (e.g., WSL)
+ if (EC)
+ return 1;
+
+ for (; Begin != End; Begin.increment(EC)) {
if (EC)
return 1;
@@ -75,3 +84,5 @@ int printGPUsByKFD() {
return 0;
}
+
+int printGPUsByKFD() { return printGPUsByKFD(KFD_SYSFS_NODE_PATH); }
diff --git a/clang/unittests/offload-arch/CMakeLists.txt b/clang/unittests/offload-arch/CMakeLists.txt
index db4fa5ceba5ea..8d9cbf5c60205 100644
--- a/clang/unittests/offload-arch/CMakeLists.txt
+++ b/clang/unittests/offload-arch/CMakeLists.txt
@@ -1,10 +1,19 @@
+set(OffloadArchTestSources
+ OffloadArchTest.cpp
+ ${CMAKE_CURRENT_SOURCE_DIR}/../../tools/offload-arch/AMDGPUArchByKFD.cpp
+ )
+
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
- )
+ list(APPEND OffloadArchTestSources
+ ${CMAKE_CURRENT_SOURCE_DIR}/../../tools/offload-arch/AMDGPUArchByHIP.cpp)
endif()
+
+add_distinct_clang_unittest(OffloadArchTests
+ ${OffloadArchTestSources}
+ CLANG_LIBS
+ clangBasic
+ LINK_LIBS
+ LLVMTestingSupport
+ LLVM_COMPONENTS
+ Support
+ )
diff --git a/clang/unittests/offload-arch/OffloadArchTest.cpp b/clang/unittests/offload-arch/OffloadArchTest.cpp
index 4b07af39a918a..a120428bbde2d 100644
--- a/clang/unittests/offload-arch/OffloadArchTest.cpp
+++ b/clang/unittests/offload-arch/OffloadArchTest.cpp
@@ -9,8 +9,13 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
#include "gtest/gtest.h"
#include <algorithm>
+#include <cstdio>
#include <string>
// Defined in AMDGPUArchByHIP.cpp (non-static, compiled into this test).
@@ -19,6 +24,9 @@ bool compareVersions(llvm::StringRef A, llvm::StringRef B);
llvm::SmallVector<std::string, 8> getCandidateBinPaths(llvm::StringRef ExeDir);
#endif
+// Defined in AMDGPUArchByKFD.cpp (non-static, compiled into this test).
+int printGPUsByKFD(llvm::StringRef NodePath);
+
using namespace llvm;
cl::opt<bool> Verbose("offload-arch-test-verbose", cl::Hidden, cl::init(false));
@@ -112,3 +120,91 @@ TEST(CandidateBinPaths, NoDriveRootBin) {
}
#endif // _WIN32
+
+// --- printGPUsByKFD ---
+
+namespace {
+// Write <Dir>/<Node>/properties containing the given lines.
+void addNode(StringRef Dir, unsigned Node, StringRef Properties) {
+ SmallString<128> NodeDir(Dir);
+ sys::path::append(NodeDir, Twine(Node));
+ ASSERT_FALSE(sys::fs::create_directories(NodeDir));
+
+ SmallString<128> PropertiesPath(NodeDir);
+ sys::path::append(PropertiesPath, "properties");
+ std::error_code EC;
+ raw_fd_ostream OS(PropertiesPath, EC);
+ ASSERT_FALSE(EC);
+ OS << Properties;
+}
+
+// Write a node describing a GPU with the given gfx_target_version.
+void addGPUNode(StringRef Dir, unsigned Node, StringRef GFXVersion) {
+ addNode(Dir, Node, ("gfx_target_version " + GFXVersion + "\n").str());
+}
+
+// Run printGPUsByKFD, collecting what it writes to stdout.
+int printGPUsByKFDCapturingStdout(StringRef NodePath, std::string &Output) {
+ testing::internal::CaptureStdout();
+ int Result = printGPUsByKFD(NodePath);
+ std::fflush(stdout);
+ Output = testing::internal::GetCapturedStdout();
+ return Result;
+}
+} // namespace
+
+// A topology directory that cannot be opened must be reported as a failure, so
+// that the caller falls back to enumerating with the HIP runtime.
+TEST(KFDTopology, MissingDirectoryFails) {
+ unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+ std::string Output;
+ EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path("does-not-exist"), Output),
+ 1);
+ EXPECT_EQ(Output, "");
+}
+
+// A readable topology describing no GPUs is not an error, and prints nothing.
+TEST(KFDTopology, CPUOnlyTopologySucceeds) {
+ unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+ addGPUNode(Dir.path(), 0, "0");
+ std::string Output;
+ EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+ EXPECT_EQ(Output, "");
+}
+
+// A node whose properties do not mention gfx_target_version is a CPU too.
+TEST(KFDTopology, NodeWithoutGFXVersionSucceeds) {
+ unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+ addNode(Dir.path(), 0, "cpu_cores_count 16\n");
+ std::string Output;
+ EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+ EXPECT_EQ(Output, "");
+}
+
+TEST(KFDTopology, EmptyTopologySucceeds) {
+ unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+ std::string Output;
+ EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+ EXPECT_EQ(Output, "");
+}
+
+TEST(KFDTopology, GPUNodeIsPrinted) {
+ unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+ addGPUNode(Dir.path(), 0, "0"); // CPU
+ addGPUNode(Dir.path(), 1, "110001"); // gfx1101
+ std::string Output;
+ EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+ EXPECT_EQ(Output, "gfx1101\n");
+}
+
+// Devices are printed in node order, and the step is printed in hex so that
+// e.g. gfx90a renders correctly.
+TEST(KFDTopology, MultipleGPUsArePrintedInNodeOrder) {
+ unittest::TempDir Dir("kfd-topology", /*Unique=*/true);
+ addGPUNode(Dir.path(), 0, "0"); // CPU
+ addGPUNode(Dir.path(), 2, "90010"); // gfx90a
+ addGPUNode(Dir.path(), 1, "110001"); // gfx1101
+ std::string Output;
+ EXPECT_EQ(printGPUsByKFDCapturingStdout(Dir.path(), Output), 0);
+ EXPECT_EQ(Output, "gfx1101\ngfx90a\n");
+}
>From 7d2ef583f886d310fe0846747049f4a08a73b552 Mon Sep 17 00:00:00 2001
From: Allyson Cauble-Chantrenne <acaublec at amd.com>
Date: Thu, 20 Aug 2026 10:33:49 -0400
Subject: [PATCH 2/3] [offload-arch] Moved AMDGPUArchByKFD and tests to llvm
streams
---
clang/tools/offload-arch/AMDGPUArchByKFD.cpp | 11 +++++++----
clang/unittests/offload-arch/OffloadArchTest.cpp | 3 +--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
index 1334529a7daeb..2ac8dd7c75e7f 100644
--- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
@@ -13,9 +13,11 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Format.h"
#include "llvm/Support/LineIterator.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
#include <memory>
using namespace llvm;
@@ -37,8 +39,9 @@ int printGPUsByKFD(StringRef NodePath) {
std::error_code EC;
sys::fs::directory_iterator Begin(NodePath, EC), End;
- // Check if we could construct the directory_iterator, which can fail if
- // there is no KFD driver (e.g., WSL)
+ // Fail if the sysfs topology does not exist so that the caller can fall
+ // back to the HIP runtime. This can happen when the amdgpu kernel module
+ // is not loaded (e.g., WSL).
if (EC)
return 1;
@@ -79,8 +82,8 @@ int printGPUsByKFD(StringRef NodePath) {
// Sort the devices by their node to make sure it prints in order.
llvm::sort(Devices, [](auto &L, auto &R) { return L.first < R.first; });
for (const auto &[Node, GFXVersion] : Devices)
- std::fprintf(stdout, "gfx%ld%ld%lx\n", getMajor(GFXVersion),
- getMinor(GFXVersion), getStep(GFXVersion));
+ outs() << "gfx" << getMajor(GFXVersion) << getMinor(GFXVersion)
+ << format_hex_no_prefix(getStep(GFXVersion), 1) << '\n';
return 0;
}
diff --git a/clang/unittests/offload-arch/OffloadArchTest.cpp b/clang/unittests/offload-arch/OffloadArchTest.cpp
index a120428bbde2d..5f5e49f5c72cc 100644
--- a/clang/unittests/offload-arch/OffloadArchTest.cpp
+++ b/clang/unittests/offload-arch/OffloadArchTest.cpp
@@ -15,7 +15,6 @@
#include "llvm/Testing/Support/SupportHelpers.h"
#include "gtest/gtest.h"
#include <algorithm>
-#include <cstdio>
#include <string>
// Defined in AMDGPUArchByHIP.cpp (non-static, compiled into this test).
@@ -147,7 +146,7 @@ void addGPUNode(StringRef Dir, unsigned Node, StringRef GFXVersion) {
int printGPUsByKFDCapturingStdout(StringRef NodePath, std::string &Output) {
testing::internal::CaptureStdout();
int Result = printGPUsByKFD(NodePath);
- std::fflush(stdout);
+ outs().flush();
Output = testing::internal::GetCapturedStdout();
return Result;
}
>From 3541acd3977a2b9f5b27b4af016aa8e721a166d6 Mon Sep 17 00:00:00 2001
From: Allyson Cauble-Chantrenne <acaublec at amd.com>
Date: Fri, 21 Aug 2026 06:29:33 -0400
Subject: [PATCH 3/3] [offload-arch] Trimmed added comments in printGPUsByFKD
---
clang/tools/offload-arch/AMDGPUArchByKFD.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
index 2ac8dd7c75e7f..8e4284eb5f8de 100644
--- a/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
+++ b/clang/tools/offload-arch/AMDGPUArchByKFD.cpp
@@ -31,17 +31,13 @@ constexpr static long getMajor(long Ver) { return (Ver / 10000) % 100; }
constexpr static long getMinor(long Ver) { return (Ver / 100) % 100; }
constexpr static long getStep(long Ver) { return Ver % 100; }
-// Enumerate the GPUs described by the KFD topology rooted at \p NodePath.
-// Exposed so unit tests can run against a synthetic topology; it is not
-// declared in a header.
+// Exposed for testing
int printGPUsByKFD(StringRef NodePath) {
SmallVector<std::pair<long, long>> Devices;
std::error_code EC;
sys::fs::directory_iterator Begin(NodePath, EC), End;
- // Fail if the sysfs topology does not exist so that the caller can fall
- // back to the HIP runtime. This can happen when the amdgpu kernel module
- // is not loaded (e.g., WSL).
+ // Fail if the sysfs topology does not exist (e.g., WSL)
if (EC)
return 1;
More information about the cfe-commits
mailing list