From 1eb23a388a453fdf0b0653810e05da3626de73e1 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Wed, 29 Jul 2026 20:39:24 +0000 Subject: [PATCH 01/12] fix(rccl): drop false "fabric needs AMD SMI" warning on the sysfs path The warning claimed fabric support required RCCL_USE_AMD_SMI_LIB=1, which stopped being true once ARSMI_get_fabric_info() added sysfs-based UALoE/UALLink discovery. Since both the env var and HIP_FABRIC_API default to on, it printed on every rank of every job and sent readers chasing a non-existent config problem. Replace it with an INFO noting which discovery path is in use. Co-authored-by: Cursor --- projects/rccl/src/misc/amdsmi_wrap.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/projects/rccl/src/misc/amdsmi_wrap.cc b/projects/rccl/src/misc/amdsmi_wrap.cc index 368b483fa020..2ffcadc70ff8 100644 --- a/projects/rccl/src/misc/amdsmi_wrap.cc +++ b/projects/rccl/src/misc/amdsmi_wrap.cc @@ -141,7 +141,7 @@ std::atomic amdSmiInitCalled{false}; // Track if amd_smi_init has been cal ************************************************************************/ static ncclResult_t getProcessorHandle(uint32_t deviceIndex, amdsmi_processor_handle* procHandle) { if (!rcclParamUseAmdSmiLib()) { - return ncclSystemError; // Fabric only supported with amd_smi_lib + return ncclSystemError; // processor handles are amd_smi_lib-only } uint32_t socket_count = 0; @@ -241,13 +241,13 @@ ncclResult_t amd_smi_init() { AMDSMITRY(amdsmi_get_lib_version, &version); INFO(NCCL_INIT, "amdsmi_lib: version %d.%d.%d.%s", version.major, version.minor, version.release, version.build); } else { -#ifdef HIP_FABRIC_API - WARN("RCCL_USE_AMD_SMI_LIB not set, but HIP_FABRIC_API is defined. Fabric support is only available through AMD " - "SMI. Rerun with RCCL_USE_AMD_SMI_LIB=1 to enable AMD SMI and UALoE fabric support."); -#endif // initialize alternate rsmi ARSMICHECK(ARSMI_init()); - INFO(NCCL_INIT, "initialized internal alternative rsmi functionality"); + // RCCL_USE_AMD_SMI_LIB only selects who performs fabric *discovery*: amd_smi_lib, or the + // ualink sysfs nodes via ARSMI_get_fabric_info(). Both populate amdsmiFabricDevices + // identically, so UALoE/UALLink works on this path. + INFO(NCCL_INIT, "initialized internal alternative rsmi functionality; UALoE/UALLink fabric discovery uses sysfs " + "(set RCCL_USE_AMD_SMI_LIB=1 to use amd_smi_lib instead)"); } return ncclSuccess; } From 9c8a5b1bef2efdc078920dd701a24fd6ca403e87 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Wed, 29 Jul 2026 21:00:25 +0000 Subject: [PATCH 02/12] fix(rccl): accept fabric info when amd_smi reports no version amdsmi_get_gpu_fabric_info() returns SUCCESS with every fabric field correctly populated, but never writes fabric_info.version: fields it cannot source from sysfs keep an all-ones sentinel. The equality check against AMDSMI_FABRIC_INFO_CURRENT_VERSION therefore never passed against libamd_smi.so.26, so RCCL_USE_AMD_SMI_LIB=1 discarded valid UALoE data and marked every device unsupported while the sysfs path worked fine. Treat the sentinel as "unreported" and assume v1, still rejecting a version the library actually claims. Also stop printing amdsmi's full version string as a fourth dotted component, which rendered as "version 26.5.0.26.5.0". Co-authored-by: Cursor --- projects/rccl/src/misc/amdsmi_wrap.cc | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/projects/rccl/src/misc/amdsmi_wrap.cc b/projects/rccl/src/misc/amdsmi_wrap.cc index 2ffcadc70ff8..f97c77ee78f1 100644 --- a/projects/rccl/src/misc/amdsmi_wrap.cc +++ b/projects/rccl/src/misc/amdsmi_wrap.cc @@ -128,6 +128,8 @@ int amdsmiFabricDeviceCount = 0; struct amdsmiFabricDeviceInfo amdsmiFabricDevices[amdsmiFabricMaxDevices]; namespace { +// Sentinel amd_smi leaves in numeric fabric fields it could not read from sysfs +constexpr uint32_t kAmdSmiFabricVersionUnreported = 0xFFFFFFFFu; std::mutex fabricLock; // Thread safety for fabric operations bool fabricInitialized = false; thread_local bool threadFabricInitialized = false; @@ -239,7 +241,8 @@ ncclResult_t amd_smi_init() { // get amd-smi version amdsmi_version_t version; AMDSMITRY(amdsmi_get_lib_version, &version); - INFO(NCCL_INIT, "amdsmi_lib: version %d.%d.%d.%s", version.major, version.minor, version.release, version.build); + INFO(NCCL_INIT, "amdsmi_lib: version %u.%u.%u (build %s)", version.major, version.minor, version.release, + version.build); } else { // initialize alternate rsmi ARSMICHECK(ARSMI_init()); @@ -640,9 +643,12 @@ ncclResult_t amd_smi_ensureFabricInitialized() { devInfo->fabricSupported = false; continue; } - if (fabricInfo.fabric_version != AMDSMI_FABRIC_INFO_CURRENT_VERSION) { - WARN("AMD SMI fabric: unexpected fabric info version %u for device %u, expected %u", - fabricInfo.fabric_version, d, AMDSMI_FABRIC_INFO_CURRENT_VERSION); + // amd_smi does not populate version: fields it cannot read keep a sentinel of all-ones, + // so treat that as "unreported" and assume v1 rather than discarding valid fabric data. + const uint32_t fabricVersion = fabricInfo.fabric_version; + if (fabricVersion != kAmdSmiFabricVersionUnreported && fabricVersion != AMDSMI_FABRIC_INFO_CURRENT_VERSION) { + WARN("AMD SMI fabric: unexpected fabric info version %u for device %u, expected %u", fabricVersion, d, + AMDSMI_FABRIC_INFO_CURRENT_VERSION); devInfo->fabricSupported = false; continue; } From 35ff80bba7abf316ca8e61d090d7736163c4c6fe Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Wed, 29 Jul 2026 21:00:33 +0000 Subject: [PATCH 03/12] fix(rccl): pin the amdsmi fabric struct layout The compat declaration of amdsmi_fabric_info_v1_t still sized local_accelerators for 8 GPUs while the shipped library uses 16, making our struct 32 bytes short. Since amdsmi_get_gpu_fabric_info() is reached through dlopen, the library writes its own layout: that overflows the caller's object and shifts addr_mode/accel_state out from under us, with no compiler diagnostic either way. Bump the bound to 16 (matching the change in #7844 so the hunks merge) and add static_asserts on the struct sizes and payload offset so future ABI drift fails the build instead of corrupting memory. The pinned values are measured against amdsmi 26.5.0 and were chosen to be stable across the #7844 flatten, which leaves both sizes and the offset of reserved unchanged. Co-authored-by: Cursor --- projects/rccl/src/include/amdsmi_wrap.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/projects/rccl/src/include/amdsmi_wrap.h b/projects/rccl/src/include/amdsmi_wrap.h index 83319f69ef9a..3a3ef263f7b9 100644 --- a/projects/rccl/src/include/amdsmi_wrap.h +++ b/projects/rccl/src/include/amdsmi_wrap.h @@ -3,6 +3,7 @@ #ifndef AMDSMI_WRAP_H_ #define AMDSMI_WRAP_H_ +#include #include #include @@ -462,6 +463,20 @@ amdsmi_status_t amdsmi_fabric_telem_id_to_string(uint64_t telem_id, const char** /** @} End rcclFabricCompat */ #endif // !AMDSMI_FABRIC_DIRECT +/************************************************************************* + * Fabric ABI guard + * + * amdsmi_get_gpu_fabric_info() is reached through dlopen, so it writes this + * struct using the shipped library's layout, not ours. If our declaration is + * smaller the library writes past the end of the caller's object, and any + * shift moves the fields we read. Neither shows up as a compiler diagnostic, + * so pin the layout here: a mismatch must be a deliberate update, not a + * silent memory bug. Values measured against amdsmi 26.5.0 (ROCm 7.15). + ************************************************************************/ +static_assert(sizeof(amdsmi_fabric_info_v1_t) == 244, "amdsmi fabric v1 payload layout changed"); +static_assert(sizeof(amdsmi_fabric_info_t) == 320, "amdsmi fabric info struct size changed"); +static_assert(offsetof(amdsmi_fabric_info_t, reserved) == 256, "amdsmi fabric payload region moved"); + /************************************************************************* * AMD SMI Fabric Info Cache * From a4f56467d81628dbe14f740c64311ed92e9df2aa Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Wed, 29 Jul 2026 22:05:21 +0000 Subject: [PATCH 04/12] test(rccl): cover the amdsmi fabric checks and fold in the wrap tests The version-sentinel fix and the struct layout guard had no regression coverage, and the logic they corrected sat inline in amd_smi_ensureFabricInitialized(), which needs real amd_smi and GPUs to reach. Extract the two decisions into header-inline predicates so they can be exercised directly. Adds rccl-UnitTestsAmdSmi. AmdSmiFabricTests covers version acceptance (including the all-ones sentinel that was disabling fabric), the type/state eligibility matrix, and the field offsets a short local_accelerators[] would shift. Keeping the predicates inline means it needs no librccl symbols, so it runs in Release as well as Debug. test/amdsmi_wrap_test/ was a Makefile project outside the build that reached amdsmi_wrap through hand-written core.h/utils.h stubs, reported results by printing, and was never run by CTest. Its coverage moves into AmdSmiWrapTests as gtest cases that link librccl rather than stub it, so they exercise the code the library ships; each skips instead of failing when the devices, amd_smi library, or fabric it needs is absent. That linkage is also what confines the file to Debug, the only configuration where the internal amd_smi_* symbols are exported. AmdSmiWrapFabricPaths is the case that would have caught the original bug: it asserts amdsmi_get_gpu_fabric_info() and the ualink sysfs nodes agree about device 0, which they did not while the unwritten version field was grounds for rejecting the payload. It runs process-isolated because RCCL_PARAM caches RCCL_USE_AMD_SMI_LIB on first read. Co-authored-by: Cursor --- projects/rccl/src/include/amdsmi_wrap.h | 18 + projects/rccl/src/misc/amdsmi_wrap.cc | 11 +- projects/rccl/test/AmdSmiFabricTests.cpp | 135 +++++++ projects/rccl/test/AmdSmiWrapTests.cpp | 224 ++++++++++++ projects/rccl/test/CMakeLists.txt | 53 +++ projects/rccl/test/amdsmi_wrap_test/Makefile | 62 ---- projects/rccl/test/amdsmi_wrap_test/README.md | 69 ---- .../amdsmi_wrap_test/amdsmi_wrap_test.cpp | 339 ------------------ projects/rccl/test/amdsmi_wrap_test/debug.h | 23 -- projects/rccl/test/amdsmi_wrap_test/nccl.h | 24 -- .../rccl/test/amdsmi_wrap_test/test_stubs.h | 83 ----- .../rccl/test/amdsmi_wrap_test/test_utils.cc | 44 --- .../rccl/test/test_categories_amdsmi.yaml | 63 ++++ 13 files changed, 495 insertions(+), 653 deletions(-) create mode 100644 projects/rccl/test/AmdSmiFabricTests.cpp create mode 100644 projects/rccl/test/AmdSmiWrapTests.cpp delete mode 100644 projects/rccl/test/amdsmi_wrap_test/Makefile delete mode 100644 projects/rccl/test/amdsmi_wrap_test/README.md delete mode 100644 projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp delete mode 100644 projects/rccl/test/amdsmi_wrap_test/debug.h delete mode 100644 projects/rccl/test/amdsmi_wrap_test/nccl.h delete mode 100644 projects/rccl/test/amdsmi_wrap_test/test_stubs.h delete mode 100644 projects/rccl/test/amdsmi_wrap_test/test_utils.cc create mode 100644 projects/rccl/test/test_categories_amdsmi.yaml diff --git a/projects/rccl/src/include/amdsmi_wrap.h b/projects/rccl/src/include/amdsmi_wrap.h index 3a3ef263f7b9..9fd4ba41fab4 100644 --- a/projects/rccl/src/include/amdsmi_wrap.h +++ b/projects/rccl/src/include/amdsmi_wrap.h @@ -487,6 +487,24 @@ static_assert(offsetof(amdsmi_fabric_info_t, reserved) == 256, "amdsmi fabric pa #define AMDSMI_FABRIC_INFO_VERSION_1 1 #define AMDSMI_FABRIC_INFO_CURRENT_VERSION AMDSMI_FABRIC_INFO_VERSION_1 +// Sentinel amd_smi leaves in numeric fabric fields it could not source from sysfs +constexpr uint32_t kAmdSmiFabricVersionUnreported = 0xFFFFFFFFu; + +// amd_smi never populates amdsmi_fabric_info_t's version field, so an unreported value +// has to be read as v1 rather than as a reason to discard an otherwise valid payload. +// A version the library does claim, but that we cannot interpret, is still refused. +inline bool amdSmiFabricVersionUsable(uint32_t version) { + return version == kAmdSmiFabricVersionUnreported || version == AMDSMI_FABRIC_INFO_CURRENT_VERSION; +} + +// Fabric is usable only over a link type we drive, and only once the accelerator has +// reached a state where its vPoD can carry traffic. +inline bool amdSmiFabricStateUsable(amdsmi_fabric_type_t type, amdsmi_fabric_accelerator_vpod_state_t state) { + return (type == AMDSMI_FABRIC_TYPE_UALOE || type == AMDSMI_FABRIC_TYPE_UALLINK) && + (state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ACTIVE || + state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY); +} + struct amdsmiFabricDeviceInfo { bool fabricSupported; //!< Whether UALoE fabric is available amdsmi_fabric_type_t fabricType; //!< Fabric type (UALOE or UALLINK) diff --git a/projects/rccl/src/misc/amdsmi_wrap.cc b/projects/rccl/src/misc/amdsmi_wrap.cc index f97c77ee78f1..3ec853ed21d4 100644 --- a/projects/rccl/src/misc/amdsmi_wrap.cc +++ b/projects/rccl/src/misc/amdsmi_wrap.cc @@ -128,8 +128,6 @@ int amdsmiFabricDeviceCount = 0; struct amdsmiFabricDeviceInfo amdsmiFabricDevices[amdsmiFabricMaxDevices]; namespace { -// Sentinel amd_smi leaves in numeric fabric fields it could not read from sysfs -constexpr uint32_t kAmdSmiFabricVersionUnreported = 0xFFFFFFFFu; std::mutex fabricLock; // Thread safety for fabric operations bool fabricInitialized = false; thread_local bool threadFabricInitialized = false; @@ -643,20 +641,15 @@ ncclResult_t amd_smi_ensureFabricInitialized() { devInfo->fabricSupported = false; continue; } - // amd_smi does not populate version: fields it cannot read keep a sentinel of all-ones, - // so treat that as "unreported" and assume v1 rather than discarding valid fabric data. const uint32_t fabricVersion = fabricInfo.fabric_version; - if (fabricVersion != kAmdSmiFabricVersionUnreported && fabricVersion != AMDSMI_FABRIC_INFO_CURRENT_VERSION) { + if (!amdSmiFabricVersionUsable(fabricVersion)) { WARN("AMD SMI fabric: unexpected fabric info version %u for device %u, expected %u", fabricVersion, d, AMDSMI_FABRIC_INFO_CURRENT_VERSION); devInfo->fabricSupported = false; continue; } const amdsmi_fabric_info_v1_t* v1 = &fabricInfo.fabric_info.v1; - devInfo->fabricSupported = - ((v1->fabric_type == AMDSMI_FABRIC_TYPE_UALOE || v1->fabric_type == AMDSMI_FABRIC_TYPE_UALLINK) && - (v1->accel_state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ACTIVE || - v1->accel_state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY)); + devInfo->fabricSupported = amdSmiFabricStateUsable(v1->fabric_type, v1->accel_state); devInfo->fabricType = v1->fabric_type; devInfo->state = v1->accel_state; devInfo->acceleratorId = v1->accelerator_id; diff --git a/projects/rccl/test/AmdSmiFabricTests.cpp b/projects/rccl/test/AmdSmiFabricTests.cpp new file mode 100644 index 000000000000..f905756f05da --- /dev/null +++ b/projects/rccl/test/AmdSmiFabricTests.cpp @@ -0,0 +1,135 @@ +/************************************************************************* + * Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved. + * + * See LICENSE.txt for license information + ************************************************************************/ + +#include "amdsmi_wrap.h" + +#include +#include + +namespace RcclUnitTesting +{ + +// --------------------------------------------------------------------------- +// Fabric info version acceptance +// +// amdsmi_get_gpu_fabric_info() returns SUCCESS with a fully populated payload +// but never writes the version field, leaving it at the sentinel amd_smi uses +// for anything it could not source from sysfs. Rejecting that sentinel made +// RCCL_USE_AMD_SMI_LIB=1 discard working UALoE fabric on every device. +// --------------------------------------------------------------------------- + +TEST(AmdSmiFabricVersion, AcceptsUnreportedSentinel) +{ + EXPECT_TRUE(amdSmiFabricVersionUsable(kAmdSmiFabricVersionUnreported)); +} + +TEST(AmdSmiFabricVersion, AcceptsCurrentVersion) +{ + EXPECT_TRUE(amdSmiFabricVersionUsable(AMDSMI_FABRIC_INFO_CURRENT_VERSION)); +} + +// A version the library actually claims, but whose union layout we do not +// implement, must still be refused rather than misparsed as v1. +TEST(AmdSmiFabricVersion, RejectsUnimplementedFutureVersion) +{ + EXPECT_FALSE(amdSmiFabricVersionUsable(AMDSMI_FABRIC_INFO_CURRENT_VERSION + 1)); +} + +// Distinct from the sentinel: zero means the library returned without writing +// the field at all, which contradicts a SUCCESS status and is worth surfacing. +TEST(AmdSmiFabricVersion, RejectsZero) +{ + EXPECT_FALSE(amdSmiFabricVersionUsable(0)); +} + +// --------------------------------------------------------------------------- +// Fabric type / accelerator state eligibility +// +// This is the decision the version check was gating: only a link type we drive +// combined with a state whose vPoD can carry traffic counts as usable. +// --------------------------------------------------------------------------- + +TEST(AmdSmiFabricState, UaloeReadyIsUsable) +{ + EXPECT_TRUE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UALOE, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY)); +} + +TEST(AmdSmiFabricState, UaloeActiveIsUsable) +{ + EXPECT_TRUE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UALOE, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ACTIVE)); +} + +TEST(AmdSmiFabricState, UalinkReadyIsUsable) +{ + EXPECT_TRUE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UALLINK, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY)); +} + +TEST(AmdSmiFabricState, ConfiguredButNotReadyIsNotUsable) +{ + EXPECT_FALSE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UALOE, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_CONFIGURED)); +} + +TEST(AmdSmiFabricState, UnconfiguredIsNotUsable) +{ + EXPECT_FALSE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UALOE, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_UNCONFIGURED)); +} + +TEST(AmdSmiFabricState, ErrorStateIsNotUsable) +{ + EXPECT_FALSE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UALOE, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ERROR)); +} + +TEST(AmdSmiFabricState, UnknownLinkTypeIsNotUsable) +{ + EXPECT_FALSE(amdSmiFabricStateUsable(AMDSMI_FABRIC_TYPE_UNKNOWN, + AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ACTIVE)); +} + +// The combination this node actually reports, and that the version check used +// to throw away: UALoE + READY reached through the full struct. +TEST(AmdSmiFabricState, PayloadFromSysfsBackedDeviceIsUsable) +{ + amdsmi_fabric_info_v1_t v1{}; + v1.fabric_type = AMDSMI_FABRIC_TYPE_UALOE; + v1.accel_state = AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY; + v1.accelerator_id = 3; + v1.ppod_size = 8; + v1.vpod_id = 1; + v1.vpod_size = 4; + + EXPECT_TRUE(amdSmiFabricVersionUsable(kAmdSmiFabricVersionUnreported)); + EXPECT_TRUE(amdSmiFabricStateUsable(v1.fabric_type, v1.accel_state)); +} + +// --------------------------------------------------------------------------- +// Struct layout +// +// The library writes these structs through dlopen using its own layout, so a +// declaration that disagrees overflows the caller's object and shifts the +// fields we read. amdsmi_wrap.h pins this at compile time; assert the field +// offsets too, since those are what a short local_accelerators[] would move. +// --------------------------------------------------------------------------- + +TEST(AmdSmiFabricLayout, MatchesShippedLibraryAbi) +{ + EXPECT_EQ(sizeof(amdsmi_fabric_info_v1_t), 244u); + EXPECT_EQ(sizeof(amdsmi_fabric_info_t), 320u); + EXPECT_EQ(offsetof(amdsmi_fabric_info_t, reserved), 256u); +} + +TEST(AmdSmiFabricLayout, TrailingFieldsAreNotShifted) +{ + EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, addr_mode), 236u); + EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, accel_state), 240u); +} + +} // namespace RcclUnitTesting diff --git a/projects/rccl/test/AmdSmiWrapTests.cpp b/projects/rccl/test/AmdSmiWrapTests.cpp new file mode 100644 index 000000000000..2e81882f5f53 --- /dev/null +++ b/projects/rccl/test/AmdSmiWrapTests.cpp @@ -0,0 +1,224 @@ +/************************************************************************* + * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. + * + * See LICENSE.txt for license information + ************************************************************************/ + +// Exercises the amdsmi_wrap API against the machine's real GPUs. +// +// These call into librccl rather than recompiling the wrapper, so they only +// link in Debug: src/CMakeLists.txt exports the internal amd_smi_* symbols +// under -fvisibility=default there and hides them everywhere else. +// +// Every case skips rather than fails when what it needs is absent, so the +// suite stays meaningful on a machine with no GPUs, no amd_smi library, or no +// fabric. + +#include "amdsmi_wrap.h" +#include "alt_rsmi.h" +#include "common/ProcessIsolatedTestRunner.hpp" + +#include +#include +#include +#include + +namespace RcclUnitTesting +{ + +namespace +{ + +// amd_smi_init() succeeds on the sysfs fallback path too, so a failure here +// means neither backend is usable and there is nothing to assert against. +class AmdSmiWrapTest : public ::testing::Test +{ +protected: + void SetUp() override + { + if(amd_smi_init() != ncclSuccess) + GTEST_SKIP() << "amd_smi_init() failed: no usable SMI backend"; + initialized_ = true; + + if(amd_smi_getNumDevice(&numDevices_) != ncclSuccess) + GTEST_SKIP() << "amd_smi_getNumDevice() failed"; + } + + void TearDown() override + { + if(initialized_) + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); + } + + void requireDevices(uint32_t count) + { + if(numDevices_ < count) + GTEST_SKIP() << "needs " << count << " device(s), found " << numDevices_; + } + + uint32_t numDevices_ = 0; + bool initialized_ = false; +}; + +} // namespace + +TEST(AmdSmiWrapLifecycle, InitAndShutdownSucceed) +{ + if(amd_smi_init() != ncclSuccess) + GTEST_SKIP() << "amd_smi_init() failed: no usable SMI backend"; + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); +} + +// Re-initializing after a shutdown has to work: RCCL brings the wrapper up per +// communicator, so a one-shot init would break every rank after the first. +TEST(AmdSmiWrapLifecycle, InitIsRepeatable) +{ + if(amd_smi_init() != ncclSuccess) + GTEST_SKIP() << "amd_smi_init() failed: no usable SMI backend"; + ASSERT_EQ(amd_smi_shutdown(), ncclSuccess); + + EXPECT_EQ(amd_smi_init(), ncclSuccess); + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); +} + +TEST_F(AmdSmiWrapTest, PciBusIdIsPopulatedForEveryDevice) +{ + requireDevices(1); + + for(uint32_t i = 0; i < numDevices_; i++) + { + char busId[32] = {0}; + ASSERT_EQ(amd_smi_getDevicePciBusIdString(i, busId, sizeof(busId)), ncclSuccess) + << "device " << i; + EXPECT_GT(strlen(busId), 0u) << "device " << i << " reported an empty bus ID"; + } +} + +// The topology code identifies GPUs by bus ID, so the mapping has to be a +// bijection: every device's own bus ID must resolve back to that same index. +TEST_F(AmdSmiWrapTest, PciBusIdRoundTripsToDeviceIndex) +{ + requireDevices(1); + + for(uint32_t i = 0; i < numDevices_; i++) + { + char busId[32] = {0}; + ASSERT_EQ(amd_smi_getDevicePciBusIdString(i, busId, sizeof(busId)), ncclSuccess); + + uint32_t deviceIndex = 0xFFFFFFFFu; + ASSERT_EQ(amd_smi_getDeviceIndexByPciBusId(busId, &deviceIndex), ncclSuccess) + << "bus ID " << busId; + EXPECT_EQ(deviceIndex, i) << "bus ID " << busId << " resolved to the wrong device"; + } +} + +TEST_F(AmdSmiWrapTest, LinkInfoIsReportedBetweenTwoDevices) +{ + requireDevices(2); + + amdsmi_link_type_t linkType; + int hops = -1; + int count = -1; + ASSERT_EQ(amd_smi_getLinkInfo(0, 1, &linkType, &hops, &count), ncclSuccess); + + EXPECT_GE(hops, 0); + EXPECT_GE(count, 0); +} + +TEST_F(AmdSmiWrapTest, FirmwareVersionQuerySucceeds) +{ + requireDevices(1); + + // Reported as 0 when the backend cannot source it; only the call contract + // is under test here. + uint64_t fwVersion = 0; + EXPECT_EQ(amd_smi_getFirmwareVersion(0, &fwVersion), ncclSuccess); +} + +// amd_smi_isFabricSupported() and amd_smi_getFabricBandwidth() both read the +// cache that amd_smi_ensureFabricInitialized() fills, so they must agree with +// the cached record rather than each deriving their own answer. +TEST_F(AmdSmiWrapTest, FabricViewsAgreeWithCachedDeviceInfo) +{ + requireDevices(1); + ASSERT_EQ(amd_smi_ensureFabricInitialized(), ncclSuccess); + + for(uint32_t i = 0; i < numDevices_; i++) + { + struct amdsmiFabricDeviceInfo devInfo; + memset(&devInfo, 0, sizeof(devInfo)); + ASSERT_EQ(amd_smi_getFabricDeviceInfo(i, &devInfo), ncclSuccess) << "device " << i; + + bool supported = false; + ASSERT_EQ(amd_smi_isFabricSupported(i, &supported), ncclSuccess) << "device " << i; + EXPECT_EQ(supported, devInfo.fabricSupported) << "device " << i; + + uint32_t bandwidth = 0; + ASSERT_EQ(amd_smi_getFabricBandwidth(i, &bandwidth), ncclSuccess) << "device " << i; + + if(!supported) + { + // Documented contract: 0 tells the caller to fall back to + // arch-based defaults instead of trusting a fabric number. + EXPECT_EQ(bandwidth, 0u) << "device " << i << " reported bandwidth without fabric"; + continue; + } + + EXPECT_EQ(bandwidth, devInfo.bandwidth) << "device " << i; + // A device is only marked supported when it passed this predicate, so + // the cached type/state must still satisfy it. + EXPECT_TRUE(amdSmiFabricStateUsable(devInfo.fabricType, devInfo.state)) + << "device " << i << " marked supported with type " << devInfo.fabricType + << " state " << devInfo.state; + EXPECT_GT(devInfo.vpodSize, 0u) << "device " << i; + } +} + +// The regression this suite exists for: RCCL_USE_AMD_SMI_LIB selects between +// amdsmi_get_gpu_fabric_info() and the ualink sysfs nodes, and the two are +// meant to be interchangeable views of one fabric. They silently diverged when +// the library returned SUCCESS without ever writing fabric_info.version and +// RCCL rejected the payload on that basis, so the library path reported no +// fabric on hardware where sysfs plainly showed it. +// +// Runs isolated because RCCL_PARAM caches the env var in a static on first +// read, so the value has to be set before this process starts. +TEST(AmdSmiWrapFabricPaths, LibraryPathMatchesSysfs) +{ + RUN_ISOLATED_TEST_WITH_ENV( + "LibraryPathMatchesSysfs", + []() { + if(amd_smi_init() != ncclSuccess) + GTEST_SKIP() << "amd_smi_init() failed: no usable SMI backend"; + + // Read sysfs first: it decides whether this machine has fabric at + // all, and so whether there is anything to compare against. + ASSERT_EQ(ARSMI_init(), 0); + struct ARSMI_fabricInfo sysfsInfo; + memset(&sysfsInfo, 0, sizeof(sysfsInfo)); + const int sysfsResult = ARSMI_get_fabric_info(0, &sysfsInfo); + if(sysfsResult == ENODEV) + GTEST_SKIP() << "device 0 has no ualink sysfs node: no fabric on this machine"; + ASSERT_EQ(sysfsResult, 0); + + ASSERT_EQ(amd_smi_ensureFabricInitialized(), ncclSuccess); + struct amdsmiFabricDeviceInfo libInfo; + memset(&libInfo, 0, sizeof(libInfo)); + ASSERT_EQ(amd_smi_getFabricDeviceInfo(0, &libInfo), ncclSuccess); + + EXPECT_EQ(libInfo.fabricSupported, sysfsInfo.supported == 1) + << "amd_smi and sysfs disagree on whether device 0 has fabric"; + EXPECT_EQ(static_cast(libInfo.fabricType), static_cast(sysfsInfo.fabric_type)); + EXPECT_EQ(static_cast(libInfo.state), static_cast(sysfsInfo.accel_state)); + EXPECT_EQ(libInfo.acceleratorId, sysfsInfo.accel_id); + EXPECT_EQ(libInfo.ppodSize, sysfsInfo.ppod_size); + EXPECT_EQ(libInfo.cliqueId, sysfsInfo.vpod_id); + EXPECT_EQ(libInfo.vpodSize, sysfsInfo.vpod_size); + + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); + }, + {{"RCCL_USE_AMD_SMI_LIB", "1"}} + ); +} + +} // namespace RcclUnitTesting diff --git a/projects/rccl/test/CMakeLists.txt b/projects/rccl/test/CMakeLists.txt index cd79faaeba78..d6de02e7f74e 100644 --- a/projects/rccl/test/CMakeLists.txt +++ b/projects/rccl/test/CMakeLists.txt @@ -428,6 +428,46 @@ if(BUILD_TESTS) endif() endif() + # rccl-UnitTestsAmdSmi: tests for the amdsmi_wrap layer. + # + # AmdSmiFabricTests.cpp covers the fabric predicates and struct layout pinned in + # amdsmi_wrap.h. Those are header-inline, so they need no librccl.so symbols and + # run in both Release and Debug. + # + # AmdSmiWrapTests.cpp drives the amd_smi_* API itself, so it needs symbols that + # src/CMakeLists.txt only exports under -fvisibility=default in Debug. Adding it + # in Release would fail to link, so it joins the target only where they exist. + if (ROCM_VERSION VERSION_GREATER_EQUAL "60400") + list(APPEND RCCL_TEST_EXECUTABLES rccl-UnitTestsAmdSmi) + + # main_altrsmi.cpp is a plain InitGoogleTest/RUN_ALL_TESTS entry point, reused here. + set(TEST_AMDSMI_SOURCE_FILES + AmdSmiFabricTests.cpp + common/main_altrsmi.cpp + ) + + if(CMAKE_BUILD_TYPE MATCHES "Debug") + list(APPEND TEST_AMDSMI_SOURCE_FILES + AmdSmiWrapTests.cpp + common/ProcessIsolatedTestRunner.cpp + ) + endif() + + add_executable(rccl-UnitTestsAmdSmi ${TEST_AMDSMI_SOURCE_FILES}) + + # amdsmi_wrap.h needs both halves of this pairing: the include path so its + # __has_include finds amdsmi, and AMDSMI_FABRIC_DIRECT, which is PRIVATE to the + # rccl target and so does not reach test TUs. With the headers reachable but the + # define absent, the header's fabric compat block compiles on top of the real + # declarations and every fabric type collides. + if(DEFINED SMI_INCLUDE_DIR) + target_include_directories(rccl-UnitTestsAmdSmi PRIVATE ${SMI_INCLUDE_DIR}) + endif() + if(AMDSMI_FABRIC_API) + target_compile_definitions(rccl-UnitTestsAmdSmi PRIVATE AMDSMI_FABRIC_DIRECT) + endif() + endif() + # Create separate MPI test binary if MPI tests are enabled if(ENABLE_MPI_TESTS) set(MPI_TEST_SOURCE_FILES @@ -617,6 +657,19 @@ if(BUILD_TESTS) else() message(WARNING "Skipping test categories for rccl-UnitTestsAltRsmi: missing ${_rccl_test_categories_altrsmi_yaml}") endif() + + set(_rccl_test_categories_amdsmi_yaml "${CMAKE_CURRENT_SOURCE_DIR}/test_categories_amdsmi.yaml") + if(EXISTS "${_rccl_test_categories_amdsmi_yaml}") + message(STATUS "Applying test categories for rccl-UnitTestsAmdSmi") + apply_test_category_labels( + rccl-UnitTestsAmdSmi + "${_rccl_test_categories_amdsmi_yaml}" + "${PROJECT_BINARY_DIR}" + "${INSTALL_TEST_FILE}" + ) + else() + message(WARNING "Skipping test categories for rccl-UnitTestsAmdSmi: missing ${_rccl_test_categories_amdsmi_yaml}") + endif() endif() if(ENABLE_MPI_TESTS) diff --git a/projects/rccl/test/amdsmi_wrap_test/Makefile b/projects/rccl/test/amdsmi_wrap_test/Makefile deleted file mode 100644 index 954280376c27..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/Makefile +++ /dev/null @@ -1,62 +0,0 @@ -# Makefile for amdsmi_wrap standalone test -ROCM_PATH ?= /opt/rocm -CXX := $(shell command -v hipcc 2>/dev/null || echo g++) - -RCCL_ROOT := ../.. -RCCL_SRC := $(RCCL_ROOT)/src -HIPIFY_DIR := ./hipify - -INCLUDES := -I. -I$(HIPIFY_DIR) -I$(RCCL_SRC)/include -I$(RCCL_SRC) -I$(ROCM_PATH)/include -I$(ROCM_PATH)/include/rocm_smi -I$(ROCM_PATH)/include/amd_smi -CXXFLAGS := -std=c++17 -Wall -Wextra -Wno-unused-parameter -D__HIP_PLATFORM_AMD__ -DRCCL_STANDALONE_TEST $(INCLUDES) -LDFLAGS := -L$(ROCM_PATH)/lib -L$(ROCM_PATH)/lib64 -LIBS := -lrocm_smi64 -lpthread -ldl -lamdhip64 - -TARGET := amdsmi_wrap_test -OBJS := amdsmi_wrap_test.o amdsmi_wrap.o alt_rsmi.o test_utils.o - -all: core.h $(TARGET) - -core.h: test_stubs.h - ln -sf test_stubs.h core.h - ln -sf test_stubs.h utils.h - -$(HIPIFY_DIR): - mkdir -p $(HIPIFY_DIR) - -$(HIPIFY_DIR)/amdsmi_wrap.cc: $(RCCL_SRC)/misc/amdsmi_wrap.cc | $(HIPIFY_DIR) - @echo "Hipifying amdsmi_wrap.cc..." - hipify-perl -quiet-warnings $< -o $@ - -$(HIPIFY_DIR)/alt_rsmi.cc: $(RCCL_SRC)/misc/alt_rsmi.cc | $(HIPIFY_DIR) - @echo "Hipifying alt_rsmi.cc..." - hipify-perl -quiet-warnings $< -o $@ - -$(TARGET): $(OBJS) - $(CXX) $(CXXFLAGS) -o $@ $^ $(LDFLAGS) $(LIBS) - -amdsmi_wrap_test.o: amdsmi_wrap_test.cpp test_stubs.h - $(CXX) $(CXXFLAGS) -c -o $@ $< - -amdsmi_wrap.o: $(HIPIFY_DIR)/amdsmi_wrap.cc core.h - $(CXX) $(CXXFLAGS) -c -o $@ $< - -alt_rsmi.o: $(HIPIFY_DIR)/alt_rsmi.cc - $(CXX) $(CXXFLAGS) -DARSMI_TEST_BUILD -c -o $@ $< - -test_utils.o: test_utils.cc test_stubs.h - $(CXX) $(CXXFLAGS) -c -o $@ $< - -run: $(TARGET) - ./$(TARGET) - -run-amdsmi: $(TARGET) - RCCL_USE_AMD_SMI_LIB=1 ./$(TARGET) - -clean: - rm -f $(TARGET) *.o core.h utils.h - rm -rf $(HIPIFY_DIR) - -info: - @echo "Compiler: $(CXX)" - -.PHONY: all clean run run-amdsmi info diff --git a/projects/rccl/test/amdsmi_wrap_test/README.md b/projects/rccl/test/amdsmi_wrap_test/README.md deleted file mode 100644 index c5618169e20d..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/README.md +++ /dev/null @@ -1,69 +0,0 @@ -# AMD SMI Wrapper Standalone Test - -This directory contains a standalone test for the `amdsmi_wrap` module in RCCL. - -## Overview - -The `amdsmi_wrap` module provides a unified wrapper around AMD's system management interfaces: - -- **AMD SMI Library** (`libamdsmi.so`) - Modern AMD System Management Interface -- **Internal ARSMI** - Lightweight fallback when AMD SMI is not available - -## Prerequisites - -- ROCm installation (tested with ROCm 6.0+) -- HIP compiler (`hipcc`) -- AMD GPUs (for runtime testing) - -## Building - -```bash -# Default build (uses /opt/rocm) -make - -# Custom ROCm path -ROCM_PATH=/path/to/rocm make - -# Show build configuration -make info -``` - -## Running - -```bash -# Run with internal ARSMI fallback (default) -make run - -# Run with AMD SMI library (requires libamdsmi.so) -make run-amdsmi -``` - -## Test Coverage - -The test exercises these `amdsmi_wrap` functions: - -- `amd_smi_init()` / `amd_smi_shutdown()` -- `amd_smi_getNumDevice()` -- `amd_smi_getDevicePciBusIdString()` -- `amd_smi_getDeviceIndexByPciBusId()` -- `amd_smi_getLinkInfo()` -- `amd_smi_getFirmwareVersion()` -- `amd_smi_ensureFabricInitialized()` -- `amd_smi_isFabricSupported()` -- `amd_smi_getFabricBandwidth()` -- `amd_smi_getFabricDeviceInfo()` -- `amd_smi_canUseScaleUpFabric()` - -## Files - -- `amdsmi_wrap_test.cpp` - Main test driver -- `test_stubs.h` - Stub definitions for RCCL dependencies -- `test_utils.cc` - Stub implementations -- `Makefile` - Build configuration - -## Environment Variables - -| Variable | Description | -|----------|-------------| -| `RCCL_USE_AMD_SMI_LIB` | Set to `1` to use AMD SMI library | -| `ROCM_PATH` | Path to ROCm (default: `/opt/rocm`) | diff --git a/projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp b/projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp deleted file mode 100644 index e95f266c4876..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp +++ /dev/null @@ -1,339 +0,0 @@ -/************************************************************************* - * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. - * - * See LICENSE.txt for license information - ************************************************************************/ - -/** - * @file amdsmi_wrap_test.cpp - * @brief Standalone test for amdsmi_wrap wrapper functions - * - * This test exercises the amdsmi_wrap API without requiring the full RCCL - * build infrastructure. It tests both the AMD SMI library path and the - * fallback path. - * - * Build: make - * Run: ./amdsmi_wrap_test - * RCCL_USE_AMD_SMI_LIB=1 ./amdsmi_wrap_test # to test AMD SMI path - */ - -#include -#include -#include -#include -#include - -// Include test stubs first (provides ncclResult_t, macros, etc.) -#include "test_stubs.h" - -// Include the amdsmi_wrap header -#include "amdsmi_wrap.h" - -// Test result tracking -static int tests_passed = 0; -static int tests_failed = 0; - -#define TEST_ASSERT(cond, msg) do { \ - if (!(cond)) { \ - std::cerr << "[FAIL] " << msg << " (line " << __LINE__ << ")" << std::endl; \ - tests_failed++; \ - return false; \ - } \ -} while(0) - -#define TEST_PASS(msg) do { \ - std::cout << "[PASS] " << msg << std::endl; \ - tests_passed++; \ -} while(0) - -// Helper to print test section headers -static void printSection(const char* section) { - std::cout << "\n" << std::string(60, '=') << std::endl; - std::cout << "Testing: " << section << std::endl; - std::cout << std::string(60, '=') << std::endl; -} - -// ============================================================================ -// Test: amd_smi_init and amd_smi_shutdown -// ============================================================================ -bool test_init_shutdown() { - printSection("amd_smi_init / amd_smi_shutdown"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should return ncclSuccess"); - TEST_PASS("amd_smi_init() succeeded"); - - res = amd_smi_shutdown(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_shutdown() should return ncclSuccess"); - TEST_PASS("amd_smi_shutdown() succeeded"); - - return true; -} - -// ============================================================================ -// Test: amd_smi_getNumDevice -// ============================================================================ -bool test_getNumDevice() { - printSection("amd_smi_getNumDevice"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); - - uint32_t numDevs = 0; - res = amd_smi_getNumDevice(&numDevs); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should return ncclSuccess"); - - std::cout << " Number of devices detected: " << numDevs << std::endl; - TEST_PASS("amd_smi_getNumDevice() succeeded"); - - amd_smi_shutdown(); - return true; -} - -// ============================================================================ -// Test: amd_smi_getDevicePciBusIdString -// ============================================================================ -bool test_getDevicePciBusIdString() { - printSection("amd_smi_getDevicePciBusIdString"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); - - uint32_t numDevs = 0; - res = amd_smi_getNumDevice(&numDevs); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); - - if (numDevs == 0) { - std::cout << " No devices found, skipping PCI bus ID test" << std::endl; - amd_smi_shutdown(); - return true; - } - - for (uint32_t i = 0; i < numDevs; i++) { - char busId[32] = {0}; - res = amd_smi_getDevicePciBusIdString(i, busId, sizeof(busId)); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getDevicePciBusIdString() should return ncclSuccess"); - - std::cout << " Device " << i << " PCI Bus ID: " << busId << std::endl; - TEST_ASSERT(strlen(busId) > 0, "PCI Bus ID should not be empty"); - } - TEST_PASS("amd_smi_getDevicePciBusIdString() succeeded for all devices"); - - amd_smi_shutdown(); - return true; -} - -// ============================================================================ -// Test: amd_smi_getDeviceIndexByPciBusId -// ============================================================================ -bool test_getDeviceIndexByPciBusId() { - printSection("amd_smi_getDeviceIndexByPciBusId"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); - - uint32_t numDevs = 0; - res = amd_smi_getNumDevice(&numDevs); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); - - if (numDevs == 0) { - std::cout << " No devices found, skipping test" << std::endl; - amd_smi_shutdown(); - return true; - } - - // Test for all devices - for (uint32_t i = 0; i < numDevs; i++) { - // Get bus ID for device i - char busId[32] = {0}; - res = amd_smi_getDevicePciBusIdString(i, busId, sizeof(busId)); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getDevicePciBusIdString() should succeed"); - - // Look up device index by bus ID - uint32_t deviceIndex = 0xFFFFFFFF; - res = amd_smi_getDeviceIndexByPciBusId(busId, &deviceIndex); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getDeviceIndexByPciBusId() should return ncclSuccess"); - - std::cout << " Bus ID " << busId << " -> Device Index: " << deviceIndex << std::endl; - TEST_ASSERT(deviceIndex == i, "Device index should match the original device"); - } - TEST_PASS("amd_smi_getDeviceIndexByPciBusId() succeeded for all devices"); - - amd_smi_shutdown(); - return true; -} - -// ============================================================================ -// Test: amd_smi_getLinkInfo -// ============================================================================ -bool test_getLinkInfo() { - printSection("amd_smi_getLinkInfo"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); - - uint32_t numDevs = 0; - res = amd_smi_getNumDevice(&numDevs); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); - - if (numDevs < 2) { - std::cout << " Less than 2 devices found (" << numDevs << "), skipping link info test" << std::endl; - amd_smi_shutdown(); - return true; - } - - amdsmi_link_type_t linkType; - int hops = 0; - int count = 0; - - res = amd_smi_getLinkInfo(0, 1, &linkType, &hops, &count); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getLinkInfo() should return ncclSuccess"); - - const char* linkTypeStr = "UNKNOWN"; - switch (linkType) { - case AMDSMI_LINK_TYPE_PCIE: linkTypeStr = "PCIE"; break; - case AMDSMI_LINK_TYPE_XGMI: linkTypeStr = "XGMI"; break; - default: break; - } - - std::cout << " Link 0->1: Type=" << linkTypeStr << ", Hops=" << hops << ", Count=" << count << std::endl; - TEST_PASS("amd_smi_getLinkInfo() succeeded"); - - amd_smi_shutdown(); - return true; -} - -// ============================================================================ -// Test: amd_smi_getFirmwareVersion -// ============================================================================ -bool test_getFirmwareVersion() { - printSection("amd_smi_getFirmwareVersion"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); - - uint32_t numDevs = 0; - res = amd_smi_getNumDevice(&numDevs); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); - - if (numDevs == 0) { - std::cout << " No devices found, skipping firmware version test" << std::endl; - amd_smi_shutdown(); - return true; - } - - uint64_t fwVersion = 0; - res = amd_smi_getFirmwareVersion(0, &fwVersion); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getFirmwareVersion() should return ncclSuccess"); - - std::cout << " Device 0 Firmware Version: " << fwVersion; - if (fwVersion == 0) { - std::cout << " (unavailable - RCCL_USE_AMD_SMI_LIB may not be set)"; - } - std::cout << std::endl; - - TEST_PASS("amd_smi_getFirmwareVersion() succeeded"); - - amd_smi_shutdown(); - return true; -} - -// ============================================================================ -// Test: amd_smi_ensureFabricInitialized and fabric functions -// ============================================================================ -bool test_fabric_functions() { - printSection("Fabric Functions"); - - ncclResult_t res = amd_smi_init(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); - - // Test fabric initialization - res = amd_smi_ensureFabricInitialized(); - TEST_ASSERT(res == ncclSuccess, "amd_smi_ensureFabricInitialized() should return ncclSuccess"); - TEST_PASS("amd_smi_ensureFabricInitialized() succeeded"); - - uint32_t numDevs = 0; - res = amd_smi_getNumDevice(&numDevs); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); - - if (numDevs == 0) { - std::cout << " No devices found, skipping detailed fabric tests" << std::endl; - amd_smi_shutdown(); - return true; - } - - // Test isFabricSupported - bool supported = false; - res = amd_smi_isFabricSupported(0, &supported); - TEST_ASSERT(res == ncclSuccess, "amd_smi_isFabricSupported() should return ncclSuccess"); - std::cout << " Device 0 Fabric Supported: " << (supported ? "Yes" : "No") << std::endl; - TEST_PASS("amd_smi_isFabricSupported() succeeded"); - - // Test getFabricBandwidth - uint32_t bandwidth = 0; - res = amd_smi_getFabricBandwidth(0, &bandwidth); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getFabricBandwidth() should return ncclSuccess"); - std::cout << " Device 0 Fabric Bandwidth: " << bandwidth << " Mb/s" << std::endl; - TEST_PASS("amd_smi_getFabricBandwidth() succeeded"); - - // Test getFabricDeviceInfo - struct amdsmiFabricDeviceInfo devInfo; - memset(&devInfo, 0, sizeof(devInfo)); - res = amd_smi_getFabricDeviceInfo(0, &devInfo); - TEST_ASSERT(res == ncclSuccess, "amd_smi_getFabricDeviceInfo() should return ncclSuccess"); - - std::cout << " Device 0 Fabric Info:" << std::endl; - std::cout << " Fabric Supported: " << (devInfo.fabricSupported ? "Yes" : "No") << std::endl; - std::cout << " Fabric Type: " << devInfo.fabricType << std::endl; - std::cout << " State: " << devInfo.state << std::endl; - std::cout << " Accelerator ID: " << devInfo.acceleratorId << std::endl; - std::cout << " Bandwidth: " << devInfo.bandwidth << " Mb/s" << std::endl; - std::cout << " Latency: " << devInfo.latency << " ns" << std::endl; - std::cout << " UUID: " << devInfo.clusterUuid << std::endl; - std::cout << " pPOD Size: " << devInfo.ppodSize << std::endl; - std::cout << " vPOD ID: " << devInfo.cliqueId << std::endl; - std::cout << " vPOD Size: " << devInfo.vpodSize << std::endl; - TEST_PASS("amd_smi_getFabricDeviceInfo() succeeded"); - - amd_smi_shutdown(); - return true; -} - -// ============================================================================ -// Main entry point -// ============================================================================ -int main(int argc, char* argv[]) { - std::cout << "============================================================" << std::endl; - std::cout << " AMD SMI Wrapper (amdsmi_wrap) Standalone Tests " << std::endl; - std::cout << "============================================================" << std::endl; - - // Check environment - const char* useAmdSmiLib = getenv("RCCL_USE_AMD_SMI_LIB"); - std::cout << "\nEnvironment:" << std::endl; - std::cout << " RCCL_USE_AMD_SMI_LIB: " << (useAmdSmiLib ? useAmdSmiLib : "not set") << std::endl; - - // Run all tests - test_init_shutdown(); - test_getNumDevice(); - test_getDevicePciBusIdString(); - test_getDeviceIndexByPciBusId(); - test_getLinkInfo(); - test_getFirmwareVersion(); - test_fabric_functions(); - - // Print summary - std::cout << "\n" << std::string(60, '=') << std::endl; - std::cout << " Test Summary " << std::endl; - std::cout << std::string(60, '=') << std::endl; - std::cout << " Tests Passed: " << tests_passed << std::endl; - std::cout << " Tests Failed: " << tests_failed << std::endl; - std::cout << std::string(60, '=') << std::endl; - - if (tests_failed > 0) { - std::cout << "\n*** SOME TESTS FAILED ***\n" << std::endl; - return 1; - } - - std::cout << "\n*** ALL TESTS PASSED ***\n" << std::endl; - return 0; -} diff --git a/projects/rccl/test/amdsmi_wrap_test/debug.h b/projects/rccl/test/amdsmi_wrap_test/debug.h deleted file mode 100644 index f8088f599b0c..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/debug.h +++ /dev/null @@ -1,23 +0,0 @@ -#ifndef DEBUG_H -#define DEBUG_H -#include "nccl.h" -#include "core.h" - -static inline ncclResult_t rcclCudaErrorHandler(hipError_t err) { - - // Print the cuda error - ERROR("HIP failure: '%s'", hipGetErrorString(err)); - - // Special error message here: - switch (err) { - case hipErrorStreamCaptureInvalidated: - ERROR("Application is trying to use an invalidated stream to launch RCCL kernel. " - "This operation is invalid. RCCL is exiting."); - break; - default: - break; - } - return ncclUnhandledCudaError; - } - - #endif \ No newline at end of file diff --git a/projects/rccl/test/amdsmi_wrap_test/nccl.h b/projects/rccl/test/amdsmi_wrap_test/nccl.h deleted file mode 100644 index fb569119273a..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/nccl.h +++ /dev/null @@ -1,24 +0,0 @@ -/************************************************************************* - * Stub nccl.h for standalone amdsmi_wrap testing - * Provides minimal NCCL type definitions required by amdsmi_wrap.h - ************************************************************************/ - -#ifndef NCCL_H_ -#define NCCL_H_ - -#include - -// ncclResult_t enum (matches RCCL definition) -typedef enum { - ncclSuccess = 0, - ncclUnhandledCudaError = 1, - ncclSystemError = 2, - ncclInternalError = 3, - ncclInvalidArgument = 4, - ncclInvalidUsage = 5, - ncclRemoteError = 6, - ncclInProgress = 7, - ncclNumResults = 8 -} ncclResult_t; - -#endif // NCCL_H_ diff --git a/projects/rccl/test/amdsmi_wrap_test/test_stubs.h b/projects/rccl/test/amdsmi_wrap_test/test_stubs.h deleted file mode 100644 index 279b9194bcbe..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/test_stubs.h +++ /dev/null @@ -1,83 +0,0 @@ -/************************************************************************* - * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. - * - * See LICENSE.txt for license information - ************************************************************************/ - -/** - * @file test_stubs.h - * @brief Stub definitions for standalone testing of amdsmi_wrap - */ - -#ifndef TEST_STUBS_H_ -#define TEST_STUBS_H_ - -#include -#include -#include - -// Include nccl.h for ncclResult_t -#include "nccl.h" - -// Prevent including the real core.h -#define NCCL_CORE_H_ - -// Debug logging stubs -#define NCCL_INIT 1 -#define NCCL_NET 2 - -#define INFO(subsys, fmt, ...) do { \ - fprintf(stderr, "[INFO] " fmt "\n", ##__VA_ARGS__); \ -} while(0) - -#define WARN(fmt, ...) do { \ - fprintf(stderr, "[WARN] " fmt "\n", ##__VA_ARGS__); \ -} while(0) - -#define ERROR(fmt, ...) do { \ - fprintf(stderr, "[ERROR] " fmt "\n", ##__VA_ARGS__); \ -} while(0) - -// NCCLCHECK macro -#define NCCLCHECK(cmd) do { \ - ncclResult_t __res = (cmd); \ - if (__res != ncclSuccess) return __res; \ -} while(0) - -// CUDACHECK macro (uses HIP for AMD) -// Stub out HIP types when hip_runtime.h is not available -#if __has_include() -#include -#define CUDACHECK(cmd) do { \ - hipError_t __err = (cmd); \ - if (__err != hipSuccess) { \ - ERROR("HIP error %d: %s", __err, hipGetErrorString(__err)); \ - return ncclUnhandledCudaError; \ - } \ -} while(0) -#else -// Stub HIP types for environments without ROCm -typedef int hipError_t; -#define hipSuccess 0 -#define hipDeviceGetByPCIBusId(dev, bus) (0) -#define hipDeviceGetPCIBusId(bus, len, dev) (0) -#define hipGetDeviceCount(count) (*(count) = 0, 0) -#define hipGetErrorString(err) "HIP not available" -#define CUDACHECK(cmd) do { (void)(cmd); } while(0) -#endif - -// RCCL_PARAM macro for runtime parameters -#define RCCL_PARAM(name, env, default_val) \ - static int64_t rcclParam##name() { \ - static int64_t value = -1; \ - if (value == -1) { \ - const char* str = getenv("RCCL_" env); \ - value = str ? atoll(str) : (default_val); \ - } \ - return value; \ - } - -// busIdToInt64 declaration (implemented in test_utils.cc) -ncclResult_t busIdToInt64(const char* busId, int64_t* id); - -#endif // TEST_STUBS_H_ diff --git a/projects/rccl/test/amdsmi_wrap_test/test_utils.cc b/projects/rccl/test/amdsmi_wrap_test/test_utils.cc deleted file mode 100644 index 7f62ba96310c..000000000000 --- a/projects/rccl/test/amdsmi_wrap_test/test_utils.cc +++ /dev/null @@ -1,44 +0,0 @@ -/************************************************************************* - * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. - * - * See LICENSE.txt for license information - ************************************************************************/ - -/** - * @file test_utils.cc - * @brief Stub implementations of utility functions for standalone testing - */ - -#include "test_stubs.h" -#include -#include - -// Implementation of busIdToInt64 for parsing PCI bus IDs -// Format: DDDD:BB:DD.F (domain:bus:device.function) -ncclResult_t busIdToInt64(const char* busId, int64_t* id) { - if (busId == nullptr || id == nullptr) { - return ncclInvalidArgument; - } - - // Parse format: DDDD:BB:DD.F - unsigned int domain, bus, device, function; - int parsed = sscanf(busId, "%x:%x:%x.%x", &domain, &bus, &device, &function); - - if (parsed != 4) { - // Try alternative format without domain - parsed = sscanf(busId, "%x:%x.%x", &bus, &device, &function); - if (parsed != 3) { - return ncclInvalidArgument; - } - domain = 0; - } - - // Pack into int64_t in RCCL format: - // Bits [63:20] = domain - // Bits [19:12] = bus - // Bits [11:4] = device - // Bits [3:0] = function - *id = ((int64_t)domain << 20) | ((int64_t)bus << 12) | ((int64_t)device << 4) | function; - - return ncclSuccess; -} diff --git a/projects/rccl/test/test_categories_amdsmi.yaml b/projects/rccl/test/test_categories_amdsmi.yaml new file mode 100644 index 000000000000..9ecb182d7d2c --- /dev/null +++ b/projects/rccl/test/test_categories_amdsmi.yaml @@ -0,0 +1,63 @@ +# rccl-UnitTestsAmdSmi — shared/ctest schema (ROCm 6.4+, Debug build). +# RCCL does not support Windows: exclude_windows matches all tests so generated suites are empty on Windows. +# +# Suites: +# AmdSmiFabric* — pure predicate and struct layout checks, no hardware needed. +# AmdSmiWrap* — drives the amd_smi_* API against real GPUs; each case skips +# when the devices, amd_smi library, or fabric it needs is absent. +# +# Tiers: +# quick — canonical anchor; NCCL_DEBUG=INFO for library logging. +# standard — merge quick; override labels. +# comprehensive — merge quick; override labels. +# full — merge quick; override labels. + +test_categories: + quick: &amdsmi_quick + description: "Smoke — amdsmi_wrap fabric predicates, struct layout, and wrapper API, short timeout." + test_patterns: + - "AmdSmiFabric*.*" + - "AmdSmiWrap*.*" + exclude_windows: + - "*" + labels: + - "rccl" + - "quick" + - "smoke" + - "precheck" + - "amdsmi" + - "debug" + env_variables: + - "NCCL_DEBUG=INFO" + standard: + <<: *amdsmi_quick + labels: + - "rccl" + - "standard" + - "regression" + - "amdsmi" + - "debug" + comprehensive: + <<: *amdsmi_quick + labels: + - "rccl" + - "comprehensive" + - "comprehensive_regression" + - "amdsmi" + - "debug" + full: + <<: *amdsmi_quick + labels: + - "rccl" + - "full" + - "full_regression" + - "amdsmi" + - "debug" + +execution_settings: + timeout_multiplier: 1 + category_timeouts: + quick: 600 + standard: 600 + comprehensive: 600 + full: 600 From 0f646aae791b7a428fe7c49d74d96f255de5dab1 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Wed, 29 Jul 2026 22:13:04 +0000 Subject: [PATCH 05/12] test(rccl): register the amdsmi suite in Release builds too The CTest registration for rccl-UnitTestsAmdSmi went in next to the FixturesDebug and AltRsmi ones, which sit behind CMAKE_BUILD_TYPE MATCHES "Debug". Those targets are Debug-only, but this one is not: its fabric predicate and layout cases are header-inline and need no librccl symbols, so a Release build compiled and installed them and then never ran them. Move the registration up into the ROCM_VERSION-only block and drop the "debug" label the tiers inherited. Only AmdSmiWrapTests.cpp is Debug-gated, for the exported amd_smi_* symbols it calls, so the yaml's AmdSmiWrap* patterns simply match nothing in a Release binary. Co-authored-by: Cursor --- projects/rccl/test/CMakeLists.txt | 30 +++++++++++-------- .../rccl/test/test_categories_amdsmi.yaml | 9 +++--- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/projects/rccl/test/CMakeLists.txt b/projects/rccl/test/CMakeLists.txt index d6de02e7f74e..dcbb83aec4d0 100644 --- a/projects/rccl/test/CMakeLists.txt +++ b/projects/rccl/test/CMakeLists.txt @@ -629,6 +629,23 @@ if(BUILD_TESTS) else() message(WARNING "Skipping test categories for rccl-UnitTestsFixtures: missing ${_rccl_test_categories_fixtures_yaml}") endif() + + # Registered outside the Debug-only block below: this target's fabric predicate + # and layout cases need no librccl symbols and so run in Release too. The + # AmdSmiWrap* patterns in the yaml match nothing in a Release binary, where + # that file is left out for lack of exported amd_smi_* symbols. + set(_rccl_test_categories_amdsmi_yaml "${CMAKE_CURRENT_SOURCE_DIR}/test_categories_amdsmi.yaml") + if(EXISTS "${_rccl_test_categories_amdsmi_yaml}") + message(STATUS "Applying test categories for rccl-UnitTestsAmdSmi") + apply_test_category_labels( + rccl-UnitTestsAmdSmi + "${_rccl_test_categories_amdsmi_yaml}" + "${PROJECT_BINARY_DIR}" + "${INSTALL_TEST_FILE}" + ) + else() + message(WARNING "Skipping test categories for rccl-UnitTestsAmdSmi: missing ${_rccl_test_categories_amdsmi_yaml}") + endif() endif() if(ROCM_VERSION VERSION_GREATER_EQUAL "60400" AND CMAKE_BUILD_TYPE MATCHES "Debug") @@ -657,19 +674,6 @@ if(BUILD_TESTS) else() message(WARNING "Skipping test categories for rccl-UnitTestsAltRsmi: missing ${_rccl_test_categories_altrsmi_yaml}") endif() - - set(_rccl_test_categories_amdsmi_yaml "${CMAKE_CURRENT_SOURCE_DIR}/test_categories_amdsmi.yaml") - if(EXISTS "${_rccl_test_categories_amdsmi_yaml}") - message(STATUS "Applying test categories for rccl-UnitTestsAmdSmi") - apply_test_category_labels( - rccl-UnitTestsAmdSmi - "${_rccl_test_categories_amdsmi_yaml}" - "${PROJECT_BINARY_DIR}" - "${INSTALL_TEST_FILE}" - ) - else() - message(WARNING "Skipping test categories for rccl-UnitTestsAmdSmi: missing ${_rccl_test_categories_amdsmi_yaml}") - endif() endif() if(ENABLE_MPI_TESTS) diff --git a/projects/rccl/test/test_categories_amdsmi.yaml b/projects/rccl/test/test_categories_amdsmi.yaml index 9ecb182d7d2c..3cdaadcde4ea 100644 --- a/projects/rccl/test/test_categories_amdsmi.yaml +++ b/projects/rccl/test/test_categories_amdsmi.yaml @@ -1,10 +1,13 @@ -# rccl-UnitTestsAmdSmi — shared/ctest schema (ROCm 6.4+, Debug build). +# rccl-UnitTestsAmdSmi — shared/ctest schema (ROCm 6.4+, Release and Debug). # RCCL does not support Windows: exclude_windows matches all tests so generated suites are empty on Windows. # # Suites: # AmdSmiFabric* — pure predicate and struct layout checks, no hardware needed. +# Present in every build type. # AmdSmiWrap* — drives the amd_smi_* API against real GPUs; each case skips # when the devices, amd_smi library, or fabric it needs is absent. +# Debug builds only: the internal amd_smi_* symbols are hidden in +# Release, so these patterns match nothing there. # # Tiers: # quick — canonical anchor; NCCL_DEBUG=INFO for library logging. @@ -26,7 +29,6 @@ test_categories: - "smoke" - "precheck" - "amdsmi" - - "debug" env_variables: - "NCCL_DEBUG=INFO" standard: @@ -36,7 +38,6 @@ test_categories: - "standard" - "regression" - "amdsmi" - - "debug" comprehensive: <<: *amdsmi_quick labels: @@ -44,7 +45,6 @@ test_categories: - "comprehensive" - "comprehensive_regression" - "amdsmi" - - "debug" full: <<: *amdsmi_quick labels: @@ -52,7 +52,6 @@ test_categories: - "full" - "full_regression" - "amdsmi" - - "debug" execution_settings: timeout_multiplier: 1 From 2c8f2830f7f3a60d07781b5558eefdb4c033fd20 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Thu, 30 Jul 2026 11:52:48 +0000 Subject: [PATCH 06/12] fix(rccl): keep amdsmi fabric access working on pre-27 amd_smi amd_smi 27.0 flattened amdsmi_fabric_info_t and changed how amdsmi_fabric_telem_id_to_string returns a name. RCCL followed suit, which left it unable to build against, or safely call into, the older amd_smi that current ROCm installs still ship. Read the struct through accessors that select on its shape rather than on a version macro, so a header carrying the change under any version resolves correctly. The rename moved no fields, so the existing ABI asserts cover both layouts and no runtime branch is needed. Keep the deprecated AMDSMI_FABRIC_TYPE_UALLINK spelling, the only one that exists in every supported header. The telemetry call is not so lucky: the two signatures are not call-compatible and we reach the symbol through dlopen, so retain the major version the loaded runtime reports and call it accordingly. An unknown version has to assume the newer convention, since calling a 27+ implementation the old way leaves it writing through an uninitialized pointer. Mock both struct shapes in the tests, which is the only way to compile and exercise the branch the installed header does not select. Co-authored-by: Cursor --- projects/rccl/src/include/amdsmi_wrap.h | 65 ++++++++++- projects/rccl/src/misc/amdsmi_wrap.cc | 20 +++- projects/rccl/test/AmdSmiFabricTests.cpp | 142 +++++++++++++++++++++++ 3 files changed, 223 insertions(+), 4 deletions(-) diff --git a/projects/rccl/src/include/amdsmi_wrap.h b/projects/rccl/src/include/amdsmi_wrap.h index 9fd4ba41fab4..7e4fb0561eeb 100644 --- a/projects/rccl/src/include/amdsmi_wrap.h +++ b/projects/rccl/src/include/amdsmi_wrap.h @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include "nccl.h" #include "device.h" @@ -499,10 +501,69 @@ inline bool amdSmiFabricVersionUsable(uint32_t version) { // Fabric is usable only over a link type we drive, and only once the accelerator has // reached a state where its vPoD can carry traffic. +// +// AMDSMI_FABRIC_TYPE_UALLINK is deprecated in favour of the correctly spelled +// AMDSMI_FABRIC_TYPE_UALINK, but only the former exists in amd_smi before 27.0 and +// the latter is an alias for it after, so the old name is the one that compiles +// against every header we support. inline bool amdSmiFabricStateUsable(amdsmi_fabric_type_t type, amdsmi_fabric_accelerator_vpod_state_t state) { return (type == AMDSMI_FABRIC_TYPE_UALOE || type == AMDSMI_FABRIC_TYPE_UALLINK) && - (state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ACTIVE || - state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY); + (state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_ACTIVE || state == AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY); +} + +/************************************************************************* + * amdsmi_fabric_info_t layout compatibility + * + * amd_smi 27.0 flattened this struct. The version moved from a nested + * fabric_info.version to a top-level fabric_version, and the payload from + * fabric_info.fabric_version.v1 to fabric_info.v1 -- note that fabric_version + * names different things either side of the change. Field offsets did not move + * (version at byte 8, v1 at 12), so this is a source-level rename only and the + * ABI guard above holds for both. + * + * Detect the shape rather than test a version macro, so a header that carries + * the change under any version still resolves correctly. Only the flattened + * layout has a top-level integral fabric_version; in the nested layout that name + * exists solely inside fabric_info, as a union. + ************************************************************************/ +template +struct amdSmiFabricInfoIsFlat : std::false_type {}; + +template +struct amdSmiFabricInfoIsFlat().fabric_version)>> + : std::is_integral().fabric_version)>> {}; + +// Templates so that if constexpr discards the branch that does not compile against +// the header in use; in a non-template function both branches are type-checked. +template +inline uint32_t amdSmiFabricInfoVersion(const FabricInfoT& info) { + if constexpr (amdSmiFabricInfoIsFlat::value) { + return info.fabric_version; + } else { + return info.fabric_info.version; + } +} + +template +inline const amdsmi_fabric_info_v1_t* amdSmiFabricInfoV1(const FabricInfoT& info) { + if constexpr (amdSmiFabricInfoIsFlat::value) { + return &info.fabric_info.v1; + } else { + return &info.fabric_info.fabric_version.v1; + } +} + +// amd_smi 27.0 also changed amdsmi_fabric_telem_id_to_string from returning the name +// to writing it through an out-parameter. The two are not call-compatible and we reach +// it through dlopen, so the choice has to follow the version the loaded runtime +// reports. An unknown version must assume the out-parameter form: calling a 27+ +// implementation the old way leaves it writing through an uninitialized pointer, +// whereas calling an older one the new way merely leaves the name unwritten. +constexpr uint32_t kAmdSmiLibVersionUnknown = 0; +constexpr uint32_t kAmdSmiTelemOutParamMajor = 27; + +inline bool amdSmiTelemIdUsesOutParam(uint32_t libMajor) { + return libMajor == kAmdSmiLibVersionUnknown || libMajor >= kAmdSmiTelemOutParamMajor; } struct amdsmiFabricDeviceInfo { diff --git a/projects/rccl/src/misc/amdsmi_wrap.cc b/projects/rccl/src/misc/amdsmi_wrap.cc index 3ec853ed21d4..5e5646810550 100644 --- a/projects/rccl/src/misc/amdsmi_wrap.cc +++ b/projects/rccl/src/misc/amdsmi_wrap.cc @@ -134,6 +134,13 @@ thread_local bool threadFabricInitialized = false; ncclResult_t fabricInitResult = ncclSuccess; ncclResult_t amdSmiInitResult = ncclSuccess; std::atomic amdSmiInitCalled{false}; // Track if amd_smi_init has been called + +// Major version of the loaded amd_smi, as it reports itself. Selects the telemetry +// name call convention; see amdSmiTelemIdUsesOutParam(). +std::atomic amdSmiLibMajor{kAmdSmiLibVersionUnknown}; + +// Signature amdsmi_fabric_telem_id_to_string had before amd_smi 27.0 +using AmdSmiTelemIdToStringLegacyFn = const char* (*)(uint64_t); } // namespace /************************************************************************* @@ -241,6 +248,8 @@ ncclResult_t amd_smi_init() { AMDSMITRY(amdsmi_get_lib_version, &version); INFO(NCCL_INIT, "amdsmi_lib: version %u.%u.%u (build %s)", version.major, version.minor, version.release, version.build); + // Retained because the telemetry name call convention depends on it + amdSmiLibMajor.store(version.major, std::memory_order_release); } else { // initialize alternate rsmi ARSMICHECK(ARSMI_init()); @@ -641,14 +650,14 @@ ncclResult_t amd_smi_ensureFabricInitialized() { devInfo->fabricSupported = false; continue; } - const uint32_t fabricVersion = fabricInfo.fabric_version; + const uint32_t fabricVersion = amdSmiFabricInfoVersion(fabricInfo); if (!amdSmiFabricVersionUsable(fabricVersion)) { WARN("AMD SMI fabric: unexpected fabric info version %u for device %u, expected %u", fabricVersion, d, AMDSMI_FABRIC_INFO_CURRENT_VERSION); devInfo->fabricSupported = false; continue; } - const amdsmi_fabric_info_v1_t* v1 = &fabricInfo.fabric_info.v1; + const amdsmi_fabric_info_v1_t* v1 = amdSmiFabricInfoV1(fabricInfo); devInfo->fabricSupported = amdSmiFabricStateUsable(v1->fabric_type, v1->accel_state); devInfo->fabricType = v1->fabric_type; devInfo->state = v1->accel_state; @@ -761,6 +770,13 @@ const char* amd_smi_fabricTelemIdToString(uint64_t telemId) { if (pfn_amdsmi_fabric_telem_id_to_string == nullptr) { return ARSMI_fabric_telem_id_to_string(telemId); } + if (!amdSmiTelemIdUsesOutParam(amdSmiLibMajor.load(std::memory_order_acquire))) { + // dlsym gave us an address, not a prototype: on a pre-27 runtime the symbol has + // to be called through its own signature or the extra argument is read as one. + auto legacyFn = reinterpret_cast(pfn_amdsmi_fabric_telem_id_to_string); + const char* legacyName = legacyFn(telemId); + return legacyName == nullptr ? "UNKNOWN" : legacyName; + } const char* telemName = nullptr; amdsmi_status_t ret = pfn_amdsmi_fabric_telem_id_to_string(telemId, &telemName); if (ret != AMDSMI_STATUS_SUCCESS || telemName == nullptr) { diff --git a/projects/rccl/test/AmdSmiFabricTests.cpp b/projects/rccl/test/AmdSmiFabricTests.cpp index f905756f05da..7c789384ae58 100644 --- a/projects/rccl/test/AmdSmiFabricTests.cpp +++ b/projects/rccl/test/AmdSmiFabricTests.cpp @@ -132,4 +132,146 @@ TEST(AmdSmiFabricLayout, TrailingFieldsAreNotShifted) EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, accel_state), 240u); } +// --------------------------------------------------------------------------- +// Both amdsmi_fabric_info_t shapes +// +// amd_smi 27.0 flattened the struct, so RCCL has to read a version and a v1 +// payload out of either shape. Only one of them exists in any given build, so +// stand both up as mocks here: that is the only way to compile and exercise the +// branch the installed header does not select. +// --------------------------------------------------------------------------- + +namespace +{ +union MockFabricPayload +{ + amdsmi_fabric_info_v1_t v1; +}; + +// Pre-27: version and payload sit inside fabric_info, where the union carries +// the fabric_version name. +struct MockNestedFabricVer +{ + uint32_t version; + MockFabricPayload fabric_version; +}; + +struct MockNestedFabricInfo +{ + amdsmi_bdf_t bdf; + MockNestedFabricVer fabric_info; + uint32_t reserved[15]; +}; + +// 27.0 and later: version hoisted to the top level, payload reachable directly. +struct MockFlatFabricInfo +{ + amdsmi_bdf_t bdf; + uint32_t fabric_version; + MockFabricPayload fabric_info; + uint32_t reserved[15]; +}; + +// A top-level fabric_version that is not the version number, which is what the +// name means in the nested shape. Detection keys on the type, not the name. +struct MockHoistedUnionFabricInfo +{ + amdsmi_bdf_t bdf; + MockFabricPayload fabric_version; +}; +} // namespace + +TEST(AmdSmiFabricLayoutCompat, DetectsEachShape) +{ + EXPECT_FALSE(amdSmiFabricInfoIsFlat::value); + EXPECT_TRUE(amdSmiFabricInfoIsFlat::value); + EXPECT_FALSE(amdSmiFabricInfoIsFlat::value); +} + +TEST(AmdSmiFabricLayoutCompat, ReadsVersionFromNestedShape) +{ + MockNestedFabricInfo info{}; + info.fabric_info.version = kAmdSmiFabricVersionUnreported; + + EXPECT_EQ(amdSmiFabricInfoVersion(info), kAmdSmiFabricVersionUnreported); + EXPECT_TRUE(amdSmiFabricVersionUsable(amdSmiFabricInfoVersion(info))); +} + +TEST(AmdSmiFabricLayoutCompat, ReadsVersionFromFlatShape) +{ + MockFlatFabricInfo info{}; + info.fabric_version = kAmdSmiFabricVersionUnreported; + + EXPECT_EQ(amdSmiFabricInfoVersion(info), kAmdSmiFabricVersionUnreported); + EXPECT_TRUE(amdSmiFabricVersionUsable(amdSmiFabricInfoVersion(info))); +} + +TEST(AmdSmiFabricLayoutCompat, ReadsPayloadFromNestedShape) +{ + MockNestedFabricInfo info{}; + info.fabric_info.fabric_version.v1.fabric_type = AMDSMI_FABRIC_TYPE_UALOE; + info.fabric_info.fabric_version.v1.accel_state = AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY; + info.fabric_info.fabric_version.v1.accelerator_id = 3; + + const amdsmi_fabric_info_v1_t* v1 = amdSmiFabricInfoV1(info); + ASSERT_NE(v1, nullptr); + EXPECT_EQ(v1->accelerator_id, 3u); + EXPECT_TRUE(amdSmiFabricStateUsable(v1->fabric_type, v1->accel_state)); +} + +TEST(AmdSmiFabricLayoutCompat, ReadsPayloadFromFlatShape) +{ + MockFlatFabricInfo info{}; + info.fabric_info.v1.fabric_type = AMDSMI_FABRIC_TYPE_UALOE; + info.fabric_info.v1.accel_state = AMDSMI_FABRIC_ACCELERATOR_VPOD_STATE_READY; + info.fabric_info.v1.accelerator_id = 3; + + const amdsmi_fabric_info_v1_t* v1 = amdSmiFabricInfoV1(info); + ASSERT_NE(v1, nullptr); + EXPECT_EQ(v1->accelerator_id, 3u); + EXPECT_TRUE(amdSmiFabricStateUsable(v1->fabric_type, v1->accel_state)); +} + +// The rename did not move anything, which is why one set of ABI asserts covers +// both headers. If a future layout change breaks this, the accessors are no +// longer enough on their own. +TEST(AmdSmiFabricLayoutCompat, BothShapesShareOneAbi) +{ + EXPECT_EQ(sizeof(MockNestedFabricInfo), sizeof(MockFlatFabricInfo)); + EXPECT_EQ(sizeof(MockFlatFabricInfo), sizeof(amdsmi_fabric_info_t)); + + EXPECT_EQ(offsetof(MockNestedFabricInfo, fabric_info) + offsetof(MockNestedFabricVer, version), + offsetof(MockFlatFabricInfo, fabric_version)); + EXPECT_EQ(offsetof(MockNestedFabricInfo, fabric_info) + offsetof(MockNestedFabricVer, fabric_version), + offsetof(MockFlatFabricInfo, fabric_info)); + EXPECT_EQ(offsetof(MockNestedFabricInfo, reserved), offsetof(MockFlatFabricInfo, reserved)); +} + +// --------------------------------------------------------------------------- +// Telemetry name call convention +// +// amdsmi_fabric_telem_id_to_string swapped its return value for an out-parameter +// in 27.0, and we reach it through dlopen, so the version the loaded runtime +// reports decides how it may be called. +// --------------------------------------------------------------------------- + +TEST(AmdSmiFabricTelemAbi, PreflattenRuntimeUsesReturnValue) +{ + EXPECT_FALSE(amdSmiTelemIdUsesOutParam(26)); +} + +TEST(AmdSmiFabricTelemAbi, FlattenedRuntimeUsesOutParam) +{ + EXPECT_TRUE(amdSmiTelemIdUsesOutParam(kAmdSmiTelemOutParamMajor)); + EXPECT_TRUE(amdSmiTelemIdUsesOutParam(kAmdSmiTelemOutParamMajor + 1)); +} + +// Guessing wrong is not symmetric: the out-parameter form called on an older +// runtime just leaves the name unwritten, while the older form called on a newer +// runtime hands it an uninitialized pointer to write through. +TEST(AmdSmiFabricTelemAbi, UnknownVersionTakesTheSaferConvention) +{ + EXPECT_TRUE(amdSmiTelemIdUsesOutParam(kAmdSmiLibVersionUnknown)); +} + } // namespace RcclUnitTesting From b1a944c7c2cb73a3327fa0d06cd8bb489c311cb7 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Thu, 30 Jul 2026 15:09:32 +0000 Subject: [PATCH 07/12] test(rccl): keep the standalone amdsmi_wrap harness Folding the hardware checks into rccl-UnitTestsAmdSmi does not have to cost the standalone harness, which builds amdsmi_wrap.cc on its own against stub headers and so stays useful outside a full RCCL build. Restore it unchanged and keep both. Note it does not currently build against develop: device.h now reaches for nccl_tuner.h and rccl_ptr.h, which are outside the harness include paths, and its minimal nccl.h stub no longer satisfies nccl_common.h or device.h. That bit-rot predates this branch and reproduces with develop's own amdsmi_wrap sources. Co-authored-by: Cursor --- projects/rccl/test/amdsmi_wrap_test/Makefile | 62 ++++ projects/rccl/test/amdsmi_wrap_test/README.md | 69 ++++ .../amdsmi_wrap_test/amdsmi_wrap_test.cpp | 339 ++++++++++++++++++ projects/rccl/test/amdsmi_wrap_test/debug.h | 23 ++ projects/rccl/test/amdsmi_wrap_test/nccl.h | 24 ++ .../rccl/test/amdsmi_wrap_test/test_stubs.h | 83 +++++ .../rccl/test/amdsmi_wrap_test/test_utils.cc | 44 +++ 7 files changed, 644 insertions(+) create mode 100644 projects/rccl/test/amdsmi_wrap_test/Makefile create mode 100644 projects/rccl/test/amdsmi_wrap_test/README.md create mode 100644 projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp create mode 100644 projects/rccl/test/amdsmi_wrap_test/debug.h create mode 100644 projects/rccl/test/amdsmi_wrap_test/nccl.h create mode 100644 projects/rccl/test/amdsmi_wrap_test/test_stubs.h create mode 100644 projects/rccl/test/amdsmi_wrap_test/test_utils.cc diff --git a/projects/rccl/test/amdsmi_wrap_test/Makefile b/projects/rccl/test/amdsmi_wrap_test/Makefile new file mode 100644 index 000000000000..954280376c27 --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/Makefile @@ -0,0 +1,62 @@ +# Makefile for amdsmi_wrap standalone test +ROCM_PATH ?= /opt/rocm +CXX := $(shell command -v hipcc 2>/dev/null || echo g++) + +RCCL_ROOT := ../.. +RCCL_SRC := $(RCCL_ROOT)/src +HIPIFY_DIR := ./hipify + +INCLUDES := -I. -I$(HIPIFY_DIR) -I$(RCCL_SRC)/include -I$(RCCL_SRC) -I$(ROCM_PATH)/include -I$(ROCM_PATH)/include/rocm_smi -I$(ROCM_PATH)/include/amd_smi +CXXFLAGS := -std=c++17 -Wall -Wextra -Wno-unused-parameter -D__HIP_PLATFORM_AMD__ -DRCCL_STANDALONE_TEST $(INCLUDES) +LDFLAGS := -L$(ROCM_PATH)/lib -L$(ROCM_PATH)/lib64 +LIBS := -lrocm_smi64 -lpthread -ldl -lamdhip64 + +TARGET := amdsmi_wrap_test +OBJS := amdsmi_wrap_test.o amdsmi_wrap.o alt_rsmi.o test_utils.o + +all: core.h $(TARGET) + +core.h: test_stubs.h + ln -sf test_stubs.h core.h + ln -sf test_stubs.h utils.h + +$(HIPIFY_DIR): + mkdir -p $(HIPIFY_DIR) + +$(HIPIFY_DIR)/amdsmi_wrap.cc: $(RCCL_SRC)/misc/amdsmi_wrap.cc | $(HIPIFY_DIR) + @echo "Hipifying amdsmi_wrap.cc..." + hipify-perl -quiet-warnings $< -o $@ + +$(HIPIFY_DIR)/alt_rsmi.cc: $(RCCL_SRC)/misc/alt_rsmi.cc | $(HIPIFY_DIR) + @echo "Hipifying alt_rsmi.cc..." + hipify-perl -quiet-warnings $< -o $@ + +$(TARGET): $(OBJS) + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDFLAGS) $(LIBS) + +amdsmi_wrap_test.o: amdsmi_wrap_test.cpp test_stubs.h + $(CXX) $(CXXFLAGS) -c -o $@ $< + +amdsmi_wrap.o: $(HIPIFY_DIR)/amdsmi_wrap.cc core.h + $(CXX) $(CXXFLAGS) -c -o $@ $< + +alt_rsmi.o: $(HIPIFY_DIR)/alt_rsmi.cc + $(CXX) $(CXXFLAGS) -DARSMI_TEST_BUILD -c -o $@ $< + +test_utils.o: test_utils.cc test_stubs.h + $(CXX) $(CXXFLAGS) -c -o $@ $< + +run: $(TARGET) + ./$(TARGET) + +run-amdsmi: $(TARGET) + RCCL_USE_AMD_SMI_LIB=1 ./$(TARGET) + +clean: + rm -f $(TARGET) *.o core.h utils.h + rm -rf $(HIPIFY_DIR) + +info: + @echo "Compiler: $(CXX)" + +.PHONY: all clean run run-amdsmi info diff --git a/projects/rccl/test/amdsmi_wrap_test/README.md b/projects/rccl/test/amdsmi_wrap_test/README.md new file mode 100644 index 000000000000..c5618169e20d --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/README.md @@ -0,0 +1,69 @@ +# AMD SMI Wrapper Standalone Test + +This directory contains a standalone test for the `amdsmi_wrap` module in RCCL. + +## Overview + +The `amdsmi_wrap` module provides a unified wrapper around AMD's system management interfaces: + +- **AMD SMI Library** (`libamdsmi.so`) - Modern AMD System Management Interface +- **Internal ARSMI** - Lightweight fallback when AMD SMI is not available + +## Prerequisites + +- ROCm installation (tested with ROCm 6.0+) +- HIP compiler (`hipcc`) +- AMD GPUs (for runtime testing) + +## Building + +```bash +# Default build (uses /opt/rocm) +make + +# Custom ROCm path +ROCM_PATH=/path/to/rocm make + +# Show build configuration +make info +``` + +## Running + +```bash +# Run with internal ARSMI fallback (default) +make run + +# Run with AMD SMI library (requires libamdsmi.so) +make run-amdsmi +``` + +## Test Coverage + +The test exercises these `amdsmi_wrap` functions: + +- `amd_smi_init()` / `amd_smi_shutdown()` +- `amd_smi_getNumDevice()` +- `amd_smi_getDevicePciBusIdString()` +- `amd_smi_getDeviceIndexByPciBusId()` +- `amd_smi_getLinkInfo()` +- `amd_smi_getFirmwareVersion()` +- `amd_smi_ensureFabricInitialized()` +- `amd_smi_isFabricSupported()` +- `amd_smi_getFabricBandwidth()` +- `amd_smi_getFabricDeviceInfo()` +- `amd_smi_canUseScaleUpFabric()` + +## Files + +- `amdsmi_wrap_test.cpp` - Main test driver +- `test_stubs.h` - Stub definitions for RCCL dependencies +- `test_utils.cc` - Stub implementations +- `Makefile` - Build configuration + +## Environment Variables + +| Variable | Description | +|----------|-------------| +| `RCCL_USE_AMD_SMI_LIB` | Set to `1` to use AMD SMI library | +| `ROCM_PATH` | Path to ROCm (default: `/opt/rocm`) | diff --git a/projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp b/projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp new file mode 100644 index 000000000000..e95f266c4876 --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/amdsmi_wrap_test.cpp @@ -0,0 +1,339 @@ +/************************************************************************* + * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. + * + * See LICENSE.txt for license information + ************************************************************************/ + +/** + * @file amdsmi_wrap_test.cpp + * @brief Standalone test for amdsmi_wrap wrapper functions + * + * This test exercises the amdsmi_wrap API without requiring the full RCCL + * build infrastructure. It tests both the AMD SMI library path and the + * fallback path. + * + * Build: make + * Run: ./amdsmi_wrap_test + * RCCL_USE_AMD_SMI_LIB=1 ./amdsmi_wrap_test # to test AMD SMI path + */ + +#include +#include +#include +#include +#include + +// Include test stubs first (provides ncclResult_t, macros, etc.) +#include "test_stubs.h" + +// Include the amdsmi_wrap header +#include "amdsmi_wrap.h" + +// Test result tracking +static int tests_passed = 0; +static int tests_failed = 0; + +#define TEST_ASSERT(cond, msg) do { \ + if (!(cond)) { \ + std::cerr << "[FAIL] " << msg << " (line " << __LINE__ << ")" << std::endl; \ + tests_failed++; \ + return false; \ + } \ +} while(0) + +#define TEST_PASS(msg) do { \ + std::cout << "[PASS] " << msg << std::endl; \ + tests_passed++; \ +} while(0) + +// Helper to print test section headers +static void printSection(const char* section) { + std::cout << "\n" << std::string(60, '=') << std::endl; + std::cout << "Testing: " << section << std::endl; + std::cout << std::string(60, '=') << std::endl; +} + +// ============================================================================ +// Test: amd_smi_init and amd_smi_shutdown +// ============================================================================ +bool test_init_shutdown() { + printSection("amd_smi_init / amd_smi_shutdown"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should return ncclSuccess"); + TEST_PASS("amd_smi_init() succeeded"); + + res = amd_smi_shutdown(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_shutdown() should return ncclSuccess"); + TEST_PASS("amd_smi_shutdown() succeeded"); + + return true; +} + +// ============================================================================ +// Test: amd_smi_getNumDevice +// ============================================================================ +bool test_getNumDevice() { + printSection("amd_smi_getNumDevice"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); + + uint32_t numDevs = 0; + res = amd_smi_getNumDevice(&numDevs); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should return ncclSuccess"); + + std::cout << " Number of devices detected: " << numDevs << std::endl; + TEST_PASS("amd_smi_getNumDevice() succeeded"); + + amd_smi_shutdown(); + return true; +} + +// ============================================================================ +// Test: amd_smi_getDevicePciBusIdString +// ============================================================================ +bool test_getDevicePciBusIdString() { + printSection("amd_smi_getDevicePciBusIdString"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); + + uint32_t numDevs = 0; + res = amd_smi_getNumDevice(&numDevs); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); + + if (numDevs == 0) { + std::cout << " No devices found, skipping PCI bus ID test" << std::endl; + amd_smi_shutdown(); + return true; + } + + for (uint32_t i = 0; i < numDevs; i++) { + char busId[32] = {0}; + res = amd_smi_getDevicePciBusIdString(i, busId, sizeof(busId)); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getDevicePciBusIdString() should return ncclSuccess"); + + std::cout << " Device " << i << " PCI Bus ID: " << busId << std::endl; + TEST_ASSERT(strlen(busId) > 0, "PCI Bus ID should not be empty"); + } + TEST_PASS("amd_smi_getDevicePciBusIdString() succeeded for all devices"); + + amd_smi_shutdown(); + return true; +} + +// ============================================================================ +// Test: amd_smi_getDeviceIndexByPciBusId +// ============================================================================ +bool test_getDeviceIndexByPciBusId() { + printSection("amd_smi_getDeviceIndexByPciBusId"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); + + uint32_t numDevs = 0; + res = amd_smi_getNumDevice(&numDevs); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); + + if (numDevs == 0) { + std::cout << " No devices found, skipping test" << std::endl; + amd_smi_shutdown(); + return true; + } + + // Test for all devices + for (uint32_t i = 0; i < numDevs; i++) { + // Get bus ID for device i + char busId[32] = {0}; + res = amd_smi_getDevicePciBusIdString(i, busId, sizeof(busId)); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getDevicePciBusIdString() should succeed"); + + // Look up device index by bus ID + uint32_t deviceIndex = 0xFFFFFFFF; + res = amd_smi_getDeviceIndexByPciBusId(busId, &deviceIndex); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getDeviceIndexByPciBusId() should return ncclSuccess"); + + std::cout << " Bus ID " << busId << " -> Device Index: " << deviceIndex << std::endl; + TEST_ASSERT(deviceIndex == i, "Device index should match the original device"); + } + TEST_PASS("amd_smi_getDeviceIndexByPciBusId() succeeded for all devices"); + + amd_smi_shutdown(); + return true; +} + +// ============================================================================ +// Test: amd_smi_getLinkInfo +// ============================================================================ +bool test_getLinkInfo() { + printSection("amd_smi_getLinkInfo"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); + + uint32_t numDevs = 0; + res = amd_smi_getNumDevice(&numDevs); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); + + if (numDevs < 2) { + std::cout << " Less than 2 devices found (" << numDevs << "), skipping link info test" << std::endl; + amd_smi_shutdown(); + return true; + } + + amdsmi_link_type_t linkType; + int hops = 0; + int count = 0; + + res = amd_smi_getLinkInfo(0, 1, &linkType, &hops, &count); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getLinkInfo() should return ncclSuccess"); + + const char* linkTypeStr = "UNKNOWN"; + switch (linkType) { + case AMDSMI_LINK_TYPE_PCIE: linkTypeStr = "PCIE"; break; + case AMDSMI_LINK_TYPE_XGMI: linkTypeStr = "XGMI"; break; + default: break; + } + + std::cout << " Link 0->1: Type=" << linkTypeStr << ", Hops=" << hops << ", Count=" << count << std::endl; + TEST_PASS("amd_smi_getLinkInfo() succeeded"); + + amd_smi_shutdown(); + return true; +} + +// ============================================================================ +// Test: amd_smi_getFirmwareVersion +// ============================================================================ +bool test_getFirmwareVersion() { + printSection("amd_smi_getFirmwareVersion"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); + + uint32_t numDevs = 0; + res = amd_smi_getNumDevice(&numDevs); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); + + if (numDevs == 0) { + std::cout << " No devices found, skipping firmware version test" << std::endl; + amd_smi_shutdown(); + return true; + } + + uint64_t fwVersion = 0; + res = amd_smi_getFirmwareVersion(0, &fwVersion); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getFirmwareVersion() should return ncclSuccess"); + + std::cout << " Device 0 Firmware Version: " << fwVersion; + if (fwVersion == 0) { + std::cout << " (unavailable - RCCL_USE_AMD_SMI_LIB may not be set)"; + } + std::cout << std::endl; + + TEST_PASS("amd_smi_getFirmwareVersion() succeeded"); + + amd_smi_shutdown(); + return true; +} + +// ============================================================================ +// Test: amd_smi_ensureFabricInitialized and fabric functions +// ============================================================================ +bool test_fabric_functions() { + printSection("Fabric Functions"); + + ncclResult_t res = amd_smi_init(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_init() should succeed"); + + // Test fabric initialization + res = amd_smi_ensureFabricInitialized(); + TEST_ASSERT(res == ncclSuccess, "amd_smi_ensureFabricInitialized() should return ncclSuccess"); + TEST_PASS("amd_smi_ensureFabricInitialized() succeeded"); + + uint32_t numDevs = 0; + res = amd_smi_getNumDevice(&numDevs); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getNumDevice() should succeed"); + + if (numDevs == 0) { + std::cout << " No devices found, skipping detailed fabric tests" << std::endl; + amd_smi_shutdown(); + return true; + } + + // Test isFabricSupported + bool supported = false; + res = amd_smi_isFabricSupported(0, &supported); + TEST_ASSERT(res == ncclSuccess, "amd_smi_isFabricSupported() should return ncclSuccess"); + std::cout << " Device 0 Fabric Supported: " << (supported ? "Yes" : "No") << std::endl; + TEST_PASS("amd_smi_isFabricSupported() succeeded"); + + // Test getFabricBandwidth + uint32_t bandwidth = 0; + res = amd_smi_getFabricBandwidth(0, &bandwidth); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getFabricBandwidth() should return ncclSuccess"); + std::cout << " Device 0 Fabric Bandwidth: " << bandwidth << " Mb/s" << std::endl; + TEST_PASS("amd_smi_getFabricBandwidth() succeeded"); + + // Test getFabricDeviceInfo + struct amdsmiFabricDeviceInfo devInfo; + memset(&devInfo, 0, sizeof(devInfo)); + res = amd_smi_getFabricDeviceInfo(0, &devInfo); + TEST_ASSERT(res == ncclSuccess, "amd_smi_getFabricDeviceInfo() should return ncclSuccess"); + + std::cout << " Device 0 Fabric Info:" << std::endl; + std::cout << " Fabric Supported: " << (devInfo.fabricSupported ? "Yes" : "No") << std::endl; + std::cout << " Fabric Type: " << devInfo.fabricType << std::endl; + std::cout << " State: " << devInfo.state << std::endl; + std::cout << " Accelerator ID: " << devInfo.acceleratorId << std::endl; + std::cout << " Bandwidth: " << devInfo.bandwidth << " Mb/s" << std::endl; + std::cout << " Latency: " << devInfo.latency << " ns" << std::endl; + std::cout << " UUID: " << devInfo.clusterUuid << std::endl; + std::cout << " pPOD Size: " << devInfo.ppodSize << std::endl; + std::cout << " vPOD ID: " << devInfo.cliqueId << std::endl; + std::cout << " vPOD Size: " << devInfo.vpodSize << std::endl; + TEST_PASS("amd_smi_getFabricDeviceInfo() succeeded"); + + amd_smi_shutdown(); + return true; +} + +// ============================================================================ +// Main entry point +// ============================================================================ +int main(int argc, char* argv[]) { + std::cout << "============================================================" << std::endl; + std::cout << " AMD SMI Wrapper (amdsmi_wrap) Standalone Tests " << std::endl; + std::cout << "============================================================" << std::endl; + + // Check environment + const char* useAmdSmiLib = getenv("RCCL_USE_AMD_SMI_LIB"); + std::cout << "\nEnvironment:" << std::endl; + std::cout << " RCCL_USE_AMD_SMI_LIB: " << (useAmdSmiLib ? useAmdSmiLib : "not set") << std::endl; + + // Run all tests + test_init_shutdown(); + test_getNumDevice(); + test_getDevicePciBusIdString(); + test_getDeviceIndexByPciBusId(); + test_getLinkInfo(); + test_getFirmwareVersion(); + test_fabric_functions(); + + // Print summary + std::cout << "\n" << std::string(60, '=') << std::endl; + std::cout << " Test Summary " << std::endl; + std::cout << std::string(60, '=') << std::endl; + std::cout << " Tests Passed: " << tests_passed << std::endl; + std::cout << " Tests Failed: " << tests_failed << std::endl; + std::cout << std::string(60, '=') << std::endl; + + if (tests_failed > 0) { + std::cout << "\n*** SOME TESTS FAILED ***\n" << std::endl; + return 1; + } + + std::cout << "\n*** ALL TESTS PASSED ***\n" << std::endl; + return 0; +} diff --git a/projects/rccl/test/amdsmi_wrap_test/debug.h b/projects/rccl/test/amdsmi_wrap_test/debug.h new file mode 100644 index 000000000000..f8088f599b0c --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/debug.h @@ -0,0 +1,23 @@ +#ifndef DEBUG_H +#define DEBUG_H +#include "nccl.h" +#include "core.h" + +static inline ncclResult_t rcclCudaErrorHandler(hipError_t err) { + + // Print the cuda error + ERROR("HIP failure: '%s'", hipGetErrorString(err)); + + // Special error message here: + switch (err) { + case hipErrorStreamCaptureInvalidated: + ERROR("Application is trying to use an invalidated stream to launch RCCL kernel. " + "This operation is invalid. RCCL is exiting."); + break; + default: + break; + } + return ncclUnhandledCudaError; + } + + #endif \ No newline at end of file diff --git a/projects/rccl/test/amdsmi_wrap_test/nccl.h b/projects/rccl/test/amdsmi_wrap_test/nccl.h new file mode 100644 index 000000000000..fb569119273a --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/nccl.h @@ -0,0 +1,24 @@ +/************************************************************************* + * Stub nccl.h for standalone amdsmi_wrap testing + * Provides minimal NCCL type definitions required by amdsmi_wrap.h + ************************************************************************/ + +#ifndef NCCL_H_ +#define NCCL_H_ + +#include + +// ncclResult_t enum (matches RCCL definition) +typedef enum { + ncclSuccess = 0, + ncclUnhandledCudaError = 1, + ncclSystemError = 2, + ncclInternalError = 3, + ncclInvalidArgument = 4, + ncclInvalidUsage = 5, + ncclRemoteError = 6, + ncclInProgress = 7, + ncclNumResults = 8 +} ncclResult_t; + +#endif // NCCL_H_ diff --git a/projects/rccl/test/amdsmi_wrap_test/test_stubs.h b/projects/rccl/test/amdsmi_wrap_test/test_stubs.h new file mode 100644 index 000000000000..279b9194bcbe --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/test_stubs.h @@ -0,0 +1,83 @@ +/************************************************************************* + * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. + * + * See LICENSE.txt for license information + ************************************************************************/ + +/** + * @file test_stubs.h + * @brief Stub definitions for standalone testing of amdsmi_wrap + */ + +#ifndef TEST_STUBS_H_ +#define TEST_STUBS_H_ + +#include +#include +#include + +// Include nccl.h for ncclResult_t +#include "nccl.h" + +// Prevent including the real core.h +#define NCCL_CORE_H_ + +// Debug logging stubs +#define NCCL_INIT 1 +#define NCCL_NET 2 + +#define INFO(subsys, fmt, ...) do { \ + fprintf(stderr, "[INFO] " fmt "\n", ##__VA_ARGS__); \ +} while(0) + +#define WARN(fmt, ...) do { \ + fprintf(stderr, "[WARN] " fmt "\n", ##__VA_ARGS__); \ +} while(0) + +#define ERROR(fmt, ...) do { \ + fprintf(stderr, "[ERROR] " fmt "\n", ##__VA_ARGS__); \ +} while(0) + +// NCCLCHECK macro +#define NCCLCHECK(cmd) do { \ + ncclResult_t __res = (cmd); \ + if (__res != ncclSuccess) return __res; \ +} while(0) + +// CUDACHECK macro (uses HIP for AMD) +// Stub out HIP types when hip_runtime.h is not available +#if __has_include() +#include +#define CUDACHECK(cmd) do { \ + hipError_t __err = (cmd); \ + if (__err != hipSuccess) { \ + ERROR("HIP error %d: %s", __err, hipGetErrorString(__err)); \ + return ncclUnhandledCudaError; \ + } \ +} while(0) +#else +// Stub HIP types for environments without ROCm +typedef int hipError_t; +#define hipSuccess 0 +#define hipDeviceGetByPCIBusId(dev, bus) (0) +#define hipDeviceGetPCIBusId(bus, len, dev) (0) +#define hipGetDeviceCount(count) (*(count) = 0, 0) +#define hipGetErrorString(err) "HIP not available" +#define CUDACHECK(cmd) do { (void)(cmd); } while(0) +#endif + +// RCCL_PARAM macro for runtime parameters +#define RCCL_PARAM(name, env, default_val) \ + static int64_t rcclParam##name() { \ + static int64_t value = -1; \ + if (value == -1) { \ + const char* str = getenv("RCCL_" env); \ + value = str ? atoll(str) : (default_val); \ + } \ + return value; \ + } + +// busIdToInt64 declaration (implemented in test_utils.cc) +ncclResult_t busIdToInt64(const char* busId, int64_t* id); + +#endif // TEST_STUBS_H_ diff --git a/projects/rccl/test/amdsmi_wrap_test/test_utils.cc b/projects/rccl/test/amdsmi_wrap_test/test_utils.cc new file mode 100644 index 000000000000..7f62ba96310c --- /dev/null +++ b/projects/rccl/test/amdsmi_wrap_test/test_utils.cc @@ -0,0 +1,44 @@ +/************************************************************************* + * Copyright (c) 2024-2026 Advanced Micro Devices, Inc. All rights reserved. + * + * See LICENSE.txt for license information + ************************************************************************/ + +/** + * @file test_utils.cc + * @brief Stub implementations of utility functions for standalone testing + */ + +#include "test_stubs.h" +#include +#include + +// Implementation of busIdToInt64 for parsing PCI bus IDs +// Format: DDDD:BB:DD.F (domain:bus:device.function) +ncclResult_t busIdToInt64(const char* busId, int64_t* id) { + if (busId == nullptr || id == nullptr) { + return ncclInvalidArgument; + } + + // Parse format: DDDD:BB:DD.F + unsigned int domain, bus, device, function; + int parsed = sscanf(busId, "%x:%x:%x.%x", &domain, &bus, &device, &function); + + if (parsed != 4) { + // Try alternative format without domain + parsed = sscanf(busId, "%x:%x.%x", &bus, &device, &function); + if (parsed != 3) { + return ncclInvalidArgument; + } + domain = 0; + } + + // Pack into int64_t in RCCL format: + // Bits [63:20] = domain + // Bits [19:12] = bus + // Bits [11:4] = device + // Bits [3:0] = function + *id = ((int64_t)domain << 20) | ((int64_t)bus << 12) | ((int64_t)device << 4) | function; + + return ncclSuccess; +} From 00140ae01f77dbeb8dcbd44d306c18e3d14bb6ea Mon Sep 17 00:00:00 2001 From: Maisam Arif Date: Thu, 30 Jul 2026 12:57:47 -0500 Subject: [PATCH 08/12] test(rccl): cover the fabric telem-id dispatch contract amd_smi_fabricTelemIdToString dispatches on the loaded runtime's major version between the pre-27 return-value signature and the 27+ out-param signature. Add a Debug case that drives the real dispatch against whichever runtime is loaded and pins the contract its callers rely on: for any id it returns a printable string, never nullptr. A regression in either call-convention path would otherwise only surface as a crash or garbage log line on hardware. Skips when fabric or amd_smi is unavailable, like the rest of the suite. Signed-off-by: Maisam Arif --- projects/rccl/test/AmdSmiWrapTests.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/projects/rccl/test/AmdSmiWrapTests.cpp b/projects/rccl/test/AmdSmiWrapTests.cpp index 2e81882f5f53..23780b30f436 100644 --- a/projects/rccl/test/AmdSmiWrapTests.cpp +++ b/projects/rccl/test/AmdSmiWrapTests.cpp @@ -174,6 +174,29 @@ TEST_F(AmdSmiWrapTest, FabricViewsAgreeWithCachedDeviceInfo) } } +// amd_smi_fabricTelemIdToString dispatches on the loaded runtime's major +// version between the pre-27 return-value signature and the 27+ out-param +// signature (see amdSmiTelemIdUsesOutParam in amdsmi_wrap.h). The predicate +// itself is unit-tested in AmdSmiFabricTests; this drives the actual dispatch +// against whichever runtime is really loaded and pins the contract its callers +// depend on: for any id it returns a printable string, never nullptr. A +// regression in the reinterpret_cast legacy path (or the out-param path) would +// otherwise only surface as a crash or garbage log line on real hardware. +TEST_F(AmdSmiWrapTest, FabricTelemIdToStringNeverReturnsNull) +{ + requireDevices(1); + if(amd_smi_ensureFabricInitialized() != ncclSuccess) + GTEST_SKIP() << "fabric not available on this machine"; + + // An id the runtime does not map still has to come back as a printable + // string, because callers log it unconditionally. Whichever call + // convention matched the loaded library, both paths collapse a missing + // name to "UNKNOWN" rather than nullptr. + const char* unknownName = amd_smi_fabricTelemIdToString(~static_cast(0)); + ASSERT_NE(unknownName, nullptr); + EXPECT_GT(std::strlen(unknownName), 0u); +} + // The regression this suite exists for: RCCL_USE_AMD_SMI_LIB selects between // amdsmi_get_gpu_fabric_info() and the ualink sysfs nodes, and the two are // meant to be interchangeable views of one fabric. They silently diverged when From e8037ca0bfd1b30ab895ec5b8509ff0aaf9fb515 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Thu, 30 Jul 2026 23:30:10 +0000 Subject: [PATCH 09/12] fix(rccl): probe fabric APIs in topo_expl topo_expl bypasses RCCL's CMake configuration but imports headers whose compat declarations depend on HIP_FABRIC_API and AMDSMI_FABRIC_DIRECT. Probe the same APIs before compiling so current ROCm headers select their native types instead of redefining them. Co-authored-by: Cursor --- projects/rccl/tools/topo_expl/Makefile | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/projects/rccl/tools/topo_expl/Makefile b/projects/rccl/tools/topo_expl/Makefile index 7ac28afb0435..d15c634fdbef 100644 --- a/projects/rccl/tools/topo_expl/Makefile +++ b/projects/rccl/tools/topo_expl/Makefile @@ -28,14 +28,34 @@ else NCCL_OS_DEFINE := -DNCCL_OS_WINDOWS endif +# topo_expl bypasses RCCL's CMake configuration, so reproduce the two +# compile-time feature probes needed by headers it imports. +HAVE_HIP_FABRIC_API := $(shell printf '%s\n' \ + '#include ' \ + 'int main() { hipMemFabricHandle_t h{}; (void)h; auto fn = &hipMemImportFromShareableHandle; (void)fn; }' | \ + $(HIPCC) -x c++ -std=c++17 -fsyntax-only -D__HIP_PLATFORM_AMD__ -I$(ROCM_PATH)/include - \ + >/dev/null 2>&1 && echo yes || echo no) +ifeq ($(HAVE_HIP_FABRIC_API),yes) +FEATURE_DEFINES += -DHIP_FABRIC_API +endif + +HAVE_AMDSMI_FABRIC_API := $(shell printf '%s\n' \ + '#include ' \ + 'int main() { amdsmi_fabric_info_t info{}; (void)info; }' | \ + $(HIPCC) -x c++ -std=c++17 -fsyntax-only -I$(ROCM_PATH)/include - \ + >/dev/null 2>&1 && echo yes || echo no) +ifeq ($(HAVE_AMDSMI_FABRIC_API),yes) +FEATURE_DEFINES += -DAMDSMI_FABRIC_DIRECT +endif + CXXFLAGS = -g -ffunction-sections -fdata-sections -DROCM_VERSION=$(ROCM_VERSION) \ -std=c++17 \ -Wl,--gc-sections -fgpu-rdc \ -Iinclude -Itest -Ihipify_rccl/include -Ihipify_rccl/include/plugin \ -Ihipify_rccl/include/nccl_device \ - -Ihipify_rccl/src/device/include -Ihipify_rccl/graph -I/opt/rocm/include/ \ + -Ihipify_rccl/src/device/include -Ihipify_rccl/graph -I$(ROCM_PATH)/include/ \ -DTOPO_EXPL -DENABLE_TRACE -DENABLE_LL128 -DNVTX_NO_IMPL -DRCCL_EXPOSE_STATIC \ - $(NCCL_OS_DEFINE) \ + $(NCCL_OS_DEFINE) $(FEATURE_DEFINES) \ -lpthread TOPO_EXPL_FILES = src/$(EXE).cpp src/topo_expl_api.cpp src/model.cpp src/topo_expl_impl.cpp src/utils.cpp src/stubs.cc test/topo_expl_tests.cpp From 13febdb7ae0cfb05b90c932f852945746c389ea2 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Fri, 31 Jul 2026 01:25:51 +0000 Subject: [PATCH 10/12] fix(rccl): serialize amd_smi initialization amd_smi_init published its once-only flag before initialization completed, allowing concurrent callers to observe the default success result. Macro returns also bypassed the cached result assignment when initialization failed. Serialize initialization and shutdown, and cache every result before marking the attempt complete. The mutex is process-local and only contended by concurrent communicator initialization, so it adds no cross-rank scaling cost. Track backend initialization separately so shutdown cleans up when the later version query fails and resets the runtime ABI selector. Add isolated tests with a version-matched amd_smi stub to cover concurrent failure caching, partial initialization cleanup, and retry after shutdown. Co-authored-by: Cursor --- projects/rccl/src/misc/amdsmi_wrap.cc | 38 ++++++--- projects/rccl/test/AmdSmiLifecycleStub.cpp | 85 +++++++++++++++++++ projects/rccl/test/AmdSmiWrapTests.cpp | 96 ++++++++++++++++++++++ projects/rccl/test/CMakeLists.txt | 36 ++++++++ 4 files changed, 243 insertions(+), 12 deletions(-) create mode 100644 projects/rccl/test/AmdSmiLifecycleStub.cpp diff --git a/projects/rccl/src/misc/amdsmi_wrap.cc b/projects/rccl/src/misc/amdsmi_wrap.cc index 5e5646810550..4f0f24dff060 100644 --- a/projects/rccl/src/misc/amdsmi_wrap.cc +++ b/projects/rccl/src/misc/amdsmi_wrap.cc @@ -132,8 +132,10 @@ std::mutex fabricLock; // Thread safety for fabric operations bool fabricInitialized = false; thread_local bool threadFabricInitialized = false; ncclResult_t fabricInitResult = ncclSuccess; +std::mutex amdSmiInitLock; ncclResult_t amdSmiInitResult = ncclSuccess; -std::atomic amdSmiInitCalled{false}; // Track if amd_smi_init has been called +bool amdSmiInitCalled = false; +bool amdSmiLibInitialized = false; // Major version of the loaded amd_smi, as it reports itself. Selects the telemetry // name call convention; see amdSmiTelemIdUsesOutParam(). @@ -186,10 +188,7 @@ static bool amd_smi_FabricFunctionsLoaded() { * Existing AMD SMI Wrapper Functions ************************************************************************/ -ncclResult_t amd_smi_init() { - // Ensure we only initialize once - if (amdSmiInitCalled.exchange(true)) return amdSmiInitResult; - +static ncclResult_t amd_smi_init_impl() { if (__atomic_load_n(&is_wsl2, __ATOMIC_ACQUIRE) == -1) __atomic_store_n(&is_wsl2, (access("/dev/dxg", F_OK) == -1) ? 0 : 1, __ATOMIC_RELEASE); if (__atomic_load_n(&is_wsl2, __ATOMIC_ACQUIRE)) { @@ -204,7 +203,6 @@ ncclResult_t amd_smi_init() { static void* libhandle = dlopen(RCCL_AMDSMI_LIBNAME, RTLD_NOW); if (libhandle == nullptr) { WARN("Failed to open %s: %s", RCCL_AMDSMI_LIBNAME, dlerror()); - amdSmiInitResult = ncclInternalError; return ncclInternalError; } @@ -242,6 +240,7 @@ ncclResult_t amd_smi_init() { // initialize amd-smi for AMD GPUs AMDSMITRY(amdsmi_init, AMDSMI_INIT_AMD_GPUS); + amdSmiLibInitialized = true; // get amd-smi version amdsmi_version_t version; @@ -262,15 +261,30 @@ ncclResult_t amd_smi_init() { return ncclSuccess; } +ncclResult_t amd_smi_init() { + std::lock_guard lock(amdSmiInitLock); + if (amdSmiInitCalled) return amdSmiInitResult; + + // Cache failures too; shutdown explicitly resets this state for a retry. + amdSmiInitResult = amd_smi_init_impl(); + amdSmiInitCalled = true; + return amdSmiInitResult; +} + ncclResult_t amd_smi_shutdown() { - // Only shutdown if we actually initialized with amd_smi_lib - if (!rcclParamUseAmdSmiLib() || pfn_amdsmi_shut_down == nullptr) { - return ncclSuccess; + std::lock_guard lock(amdSmiInitLock); + + if (!amdSmiInitCalled) return ncclSuccess; + + // The backend may be initialized even if a later version query failed. + if (amdSmiLibInitialized) { + AMDSMITRY(amdsmi_shut_down); + amdSmiLibInitialized = false; } - amdSmiInitCalled.store(false); - amdSmiInitResult = ncclSuccess; - AMDSMITRY(amdsmi_shut_down); + amdSmiLibMajor.store(kAmdSmiLibVersionUnknown, std::memory_order_release); + amdSmiInitResult = ncclSuccess; + amdSmiInitCalled = false; return ncclSuccess; } diff --git a/projects/rccl/test/AmdSmiLifecycleStub.cpp b/projects/rccl/test/AmdSmiLifecycleStub.cpp new file mode 100644 index 000000000000..f6d50fedf941 --- /dev/null +++ b/projects/rccl/test/AmdSmiLifecycleStub.cpp @@ -0,0 +1,85 @@ +/************************************************************************* + * Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved. + * + * See LICENSE.txt for license information + ************************************************************************/ + +#include +#include +#include +#include +#include + +namespace +{ + +constexpr int kAmdSmiSuccess = 0; +constexpr int kAmdSmiUnknownError = -1; + +struct AmdSmiVersion +{ + uint32_t major; + uint32_t minor; + uint32_t release; + const char* build; +}; + +std::atomic initCount{0}; +std::atomic versionCount{0}; +std::atomic shutdownCount{0}; + +bool envEnabled(const char* name) +{ + const char* value = std::getenv(name); + return value != nullptr && value[0] == '1'; +} + +} // namespace + +extern "C" int amdsmi_init(uint64_t) +{ + ++initCount; + if(envEnabled("RCCL_AMDSMI_TEST_DELAY_INIT")) + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + return envEnabled("RCCL_AMDSMI_TEST_FAIL_INIT") ? kAmdSmiUnknownError : kAmdSmiSuccess; +} + +extern "C" int amdsmi_get_lib_version(AmdSmiVersion* version) +{ + ++versionCount; + if(envEnabled("RCCL_AMDSMI_TEST_FAIL_VERSION")) + return kAmdSmiUnknownError; + + version->major = 0; + version->minor = 0; + version->release = 0; + version->build = "test"; + return kAmdSmiSuccess; +} + +extern "C" int amdsmi_shut_down() +{ + ++shutdownCount; + return kAmdSmiSuccess; +} + +extern "C" int amdsmi_status_code_to_string(int, const char** statusString) +{ + *statusString = "injected amd_smi failure"; + return kAmdSmiSuccess; +} + +extern "C" unsigned amdsmi_test_init_count() +{ + return initCount.load(); +} + +extern "C" unsigned amdsmi_test_version_count() +{ + return versionCount.load(); +} + +extern "C" unsigned amdsmi_test_shutdown_count() +{ + return shutdownCount.load(); +} diff --git a/projects/rccl/test/AmdSmiWrapTests.cpp b/projects/rccl/test/AmdSmiWrapTests.cpp index 23780b30f436..3700d0d186ff 100644 --- a/projects/rccl/test/AmdSmiWrapTests.cpp +++ b/projects/rccl/test/AmdSmiWrapTests.cpp @@ -19,9 +19,14 @@ #include "common/ProcessIsolatedTestRunner.hpp" #include +#include #include #include +#include #include +#include +#include +#include namespace RcclUnitTesting { @@ -29,6 +34,39 @@ namespace RcclUnitTesting namespace { +using StubCounterFn = unsigned (*)(); + +std::string lifecycleStubLibraryPath() +{ + std::string path = AMDSMI_TEST_STUB_DIR; + if(const char* inheritedPath = std::getenv("LD_LIBRARY_PATH")) + path += ":" + std::string(inheritedPath); + return path; +} + +unsigned lifecycleStubCounter(const char* symbol) +{ + void* handle = dlopen(AMDSMI_TEST_STUB_SONAME, RTLD_NOW | RTLD_NOLOAD); + if(handle == nullptr) + { + ADD_FAILURE() << "lifecycle stub is not loaded: " << dlerror(); + return 0; + } + + dlerror(); + auto counter = reinterpret_cast(dlsym(handle, symbol)); + if(const char* error = dlerror()) + { + ADD_FAILURE() << "failed to resolve " << symbol << ": " << error; + dlclose(handle); + return 0; + } + + const unsigned value = counter(); + dlclose(handle); + return value; +} + // amd_smi_init() succeeds on the sysfs fallback path too, so a failure here // means neither backend is usable and there is nothing to assert against. class AmdSmiWrapTest : public ::testing::Test @@ -81,6 +119,64 @@ TEST(AmdSmiWrapLifecycle, InitIsRepeatable) EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); } +TEST(AmdSmiWrapLifecycle, ConcurrentCallersReceiveCachedFailure) +{ + RUN_ISOLATED_TEST_WITH_ENV( + "ConcurrentCallersReceiveCachedFailure", + []() { + constexpr size_t kThreadCount = 16; + std::vector results(kThreadCount, ncclSuccess); + std::vector threads; + threads.reserve(kThreadCount); + + for(size_t i = 0; i < kThreadCount; ++i) + threads.emplace_back([&, i]() { results[i] = amd_smi_init(); }); + for(auto& thread : threads) + thread.join(); + + for(size_t i = 0; i < results.size(); ++i) + EXPECT_EQ(results[i], ncclInternalError) << "caller " << i; + + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_init_count"), 1u); + EXPECT_EQ(amd_smi_init(), ncclInternalError); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_init_count"), 1u); + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_shutdown_count"), 0u); + }, + {{"RCCL_USE_AMD_SMI_LIB", "1"}, + {"RCCL_AMDSMI_TEST_DELAY_INIT", "1"}, + {"RCCL_AMDSMI_TEST_FAIL_INIT", "1"}, + {"LD_LIBRARY_PATH", lifecycleStubLibraryPath()}} + ); +} + +TEST(AmdSmiWrapLifecycle, ShutdownCleansUpAfterVersionFailure) +{ + RUN_ISOLATED_TEST_WITH_ENV( + "ShutdownCleansUpAfterVersionFailure", + []() { + EXPECT_EQ(amd_smi_init(), ncclInternalError); + EXPECT_EQ(amd_smi_init(), ncclInternalError); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_init_count"), 1u); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_version_count"), 1u); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_shutdown_count"), 0u); + + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_shutdown_count"), 1u); + + // Shutdown clears the cached failure and permits a fresh attempt. + EXPECT_EQ(amd_smi_init(), ncclInternalError); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_init_count"), 2u); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_version_count"), 2u); + EXPECT_EQ(amd_smi_shutdown(), ncclSuccess); + EXPECT_EQ(lifecycleStubCounter("amdsmi_test_shutdown_count"), 2u); + }, + {{"RCCL_USE_AMD_SMI_LIB", "1"}, + {"RCCL_AMDSMI_TEST_FAIL_VERSION", "1"}, + {"LD_LIBRARY_PATH", lifecycleStubLibraryPath()}} + ); +} + TEST_F(AmdSmiWrapTest, PciBusIdIsPopulatedForEveryDevice) { requireDevices(1); diff --git a/projects/rccl/test/CMakeLists.txt b/projects/rccl/test/CMakeLists.txt index dcbb83aec4d0..632a5924bcc5 100644 --- a/projects/rccl/test/CMakeLists.txt +++ b/projects/rccl/test/CMakeLists.txt @@ -451,10 +451,46 @@ if(BUILD_TESTS) AmdSmiWrapTests.cpp common/ProcessIsolatedTestRunner.cpp ) + + # Loaded only by isolated lifecycle tests, never by the normal test process. + add_library(rccl-AmdSmiLifecycleStub SHARED AmdSmiLifecycleStub.cpp) + set_target_properties(rccl-AmdSmiLifecycleStub PROPERTIES + OUTPUT_NAME "amd_smi" + LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/amdsmi_lifecycle_stub" + ) + + # Match the versioned name used by amdsmi_wrap when headers are present. + # Older ROCm configurations may not define SMI_INCLUDE_DIR, so also check + # ROCM_PATH; without either header amdsmi_wrap uses the unversioned name. + if(EXISTS "${SMI_INCLUDE_DIR}/amd_smi/amdsmi.h") + set(AMDSMI_TEST_HEADER "${SMI_INCLUDE_DIR}/amd_smi/amdsmi.h") + elseif(EXISTS "${ROCM_PATH}/include/amd_smi/amdsmi.h") + set(AMDSMI_TEST_HEADER "${ROCM_PATH}/include/amd_smi/amdsmi.h") + endif() + if(AMDSMI_TEST_HEADER) + file(STRINGS "${AMDSMI_TEST_HEADER}" AMDSMI_VERSION_MAJOR_LINE + REGEX "^#define AMDSMI_LIB_VERSION_MAJOR [0-9]+" + LIMIT_COUNT 1 + ) + string(REGEX REPLACE ".* ([0-9]+)$" "\\1" + AMDSMI_TEST_SOVERSION "${AMDSMI_VERSION_MAJOR_LINE}" + ) + set_target_properties(rccl-AmdSmiLifecycleStub PROPERTIES + SOVERSION "${AMDSMI_TEST_SOVERSION}" + ) + endif() endif() add_executable(rccl-UnitTestsAmdSmi ${TEST_AMDSMI_SOURCE_FILES}) + if(CMAKE_BUILD_TYPE MATCHES "Debug") + add_dependencies(rccl-UnitTestsAmdSmi rccl-AmdSmiLifecycleStub) + target_compile_definitions(rccl-UnitTestsAmdSmi PRIVATE + AMDSMI_TEST_STUB_DIR="$" + AMDSMI_TEST_STUB_SONAME="$" + ) + endif() + # amdsmi_wrap.h needs both halves of this pairing: the include path so its # __has_include finds amdsmi, and AMDSMI_FABRIC_DIRECT, which is PRIVATE to the # rccl target and so does not reach test TUs. With the headers reachable but the From 9d99d366e4ae2e101ea3cfd0e92d8699fba740a4 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Fri, 31 Jul 2026 16:43:22 +0000 Subject: [PATCH 11/12] fix(rccl): accept both amd_smi fabric payload sizes Some pre-release ROCm 7.14 snapshots used an 8-GPU fabric payload while final ROCm 7.14 and ROCm 7.15 use 16 GPUs under the same amd_smi major. The absolute 16-GPU ABI guard therefore rejected a supported GIN build configuration. Accept only the coherent size and offset tuple for either layout, preserving a compile-time failure for any other ABI drift. Update the layout tests to cover the selected tuple and its trailing field offsets. Co-authored-by: Cursor --- projects/rccl/src/include/amdsmi_wrap.h | 26 ++++++++++++++++-------- projects/rccl/test/AmdSmiFabricTests.cpp | 15 +++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/projects/rccl/src/include/amdsmi_wrap.h b/projects/rccl/src/include/amdsmi_wrap.h index 7e4fb0561eeb..06bc3930f404 100644 --- a/projects/rccl/src/include/amdsmi_wrap.h +++ b/projects/rccl/src/include/amdsmi_wrap.h @@ -472,12 +472,22 @@ amdsmi_status_t amdsmi_fabric_telem_id_to_string(uint64_t telem_id, const char** * struct using the shipped library's layout, not ours. If our declaration is * smaller the library writes past the end of the caller's object, and any * shift moves the fields we read. Neither shows up as a compiler diagnostic, - * so pin the layout here: a mismatch must be a deliberate update, not a - * silent memory bug. Values measured against amdsmi 26.5.0 (ROCm 7.15). + * so recognize the two shipped layouts here: a mismatch must be a deliberate + * update, not a silent memory bug. Some pre-release ROCm 7.14 snapshots used + * AMDSMI_FABRIC_MAX_LOCAL_GPUS=8, while final ROCm 7.14 and ROCm 7.15 use 16 + * without changing the amd_smi major. ************************************************************************/ -static_assert(sizeof(amdsmi_fabric_info_v1_t) == 244, "amdsmi fabric v1 payload layout changed"); -static_assert(sizeof(amdsmi_fabric_info_t) == 320, "amdsmi fabric info struct size changed"); -static_assert(offsetof(amdsmi_fabric_info_t, reserved) == 256, "amdsmi fabric payload region moved"); +constexpr bool amdSmiFabricLayoutIs8Gpu = + sizeof(amdsmi_fabric_info_v1_t) == 212 && sizeof(amdsmi_fabric_info_t) == 288 && + offsetof(amdsmi_fabric_info_v1_t, addr_mode) == 204 && offsetof(amdsmi_fabric_info_v1_t, accel_state) == 208 && + offsetof(amdsmi_fabric_info_t, reserved) == 224; + +constexpr bool amdSmiFabricLayoutIs16Gpu = + sizeof(amdsmi_fabric_info_v1_t) == 244 && sizeof(amdsmi_fabric_info_t) == 320 && + offsetof(amdsmi_fabric_info_v1_t, addr_mode) == 236 && offsetof(amdsmi_fabric_info_v1_t, accel_state) == 240 && + offsetof(amdsmi_fabric_info_t, reserved) == 256; + +static_assert(amdSmiFabricLayoutIs8Gpu || amdSmiFabricLayoutIs16Gpu, "unsupported amdsmi fabric layout"); /************************************************************************* * AMD SMI Fabric Info Cache @@ -517,9 +527,9 @@ inline bool amdSmiFabricStateUsable(amdsmi_fabric_type_t type, amdsmi_fabric_acc * amd_smi 27.0 flattened this struct. The version moved from a nested * fabric_info.version to a top-level fabric_version, and the payload from * fabric_info.fabric_version.v1 to fabric_info.v1 -- note that fabric_version - * names different things either side of the change. Field offsets did not move - * (version at byte 8, v1 at 12), so this is a source-level rename only and the - * ABI guard above holds for both. + * names different things either side of the change. The version and payload + * offsets did not move (bytes 8 and 12), so the shape change itself is only a + * source-level rename; the ABI guard above handles payload capacity separately. * * Detect the shape rather than test a version macro, so a header that carries * the change under any version still resolves correctly. Only the flattened diff --git a/projects/rccl/test/AmdSmiFabricTests.cpp b/projects/rccl/test/AmdSmiFabricTests.cpp index 7c789384ae58..6e504bccde83 100644 --- a/projects/rccl/test/AmdSmiFabricTests.cpp +++ b/projects/rccl/test/AmdSmiFabricTests.cpp @@ -115,21 +115,22 @@ TEST(AmdSmiFabricState, PayloadFromSysfsBackedDeviceIsUsable) // // The library writes these structs through dlopen using its own layout, so a // declaration that disagrees overflows the caller's object and shifts the -// fields we read. amdsmi_wrap.h pins this at compile time; assert the field -// offsets too, since those are what a short local_accelerators[] would move. +// fields we read. amdsmi_wrap.h accepts the coherent 8-GPU layout used by some +// pre-release ROCm 7.14 snapshots and the 16-GPU layout used by final ROCm 7.14 +// and ROCm 7.15, but rejects any other combination. // --------------------------------------------------------------------------- TEST(AmdSmiFabricLayout, MatchesShippedLibraryAbi) { - EXPECT_EQ(sizeof(amdsmi_fabric_info_v1_t), 244u); - EXPECT_EQ(sizeof(amdsmi_fabric_info_t), 320u); - EXPECT_EQ(offsetof(amdsmi_fabric_info_t, reserved), 256u); + EXPECT_TRUE(amdSmiFabricLayoutIs8Gpu || amdSmiFabricLayoutIs16Gpu); + EXPECT_NE(amdSmiFabricLayoutIs8Gpu, amdSmiFabricLayoutIs16Gpu); } TEST(AmdSmiFabricLayout, TrailingFieldsAreNotShifted) { - EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, addr_mode), 236u); - EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, accel_state), 240u); + const size_t expectedAddrMode = amdSmiFabricLayoutIs8Gpu ? 204u : 236u; + EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, addr_mode), expectedAddrMode); + EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, accel_state), expectedAddrMode + sizeof(uint32_t)); } // --------------------------------------------------------------------------- From 4efee06b94b3a85333c0697678e17dfe47f52dd4 Mon Sep 17 00:00:00 2001 From: nileshnegi Date: Fri, 31 Jul 2026 17:19:53 +0000 Subject: [PATCH 12/12] fix(rccl): fall back on mixed amd_smi fabric ABI Probe the loaded fabric payload layout before typed access so mixed header/runtime installations safely use sysfs instead of corrupting memory. Co-authored-by: Cursor --- projects/rccl/src/include/amdsmi_wrap.h | 53 ++++++++++++++++++++++-- projects/rccl/src/misc/amdsmi_wrap.cc | 24 +++++++++++ projects/rccl/test/AmdSmiFabricTests.cpp | 28 +++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/projects/rccl/src/include/amdsmi_wrap.h b/projects/rccl/src/include/amdsmi_wrap.h index 06bc3930f404..c8f749582a9f 100644 --- a/projects/rccl/src/include/amdsmi_wrap.h +++ b/projects/rccl/src/include/amdsmi_wrap.h @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -477,14 +478,21 @@ amdsmi_status_t amdsmi_fabric_telem_id_to_string(uint64_t telem_id, const char** * AMDSMI_FABRIC_MAX_LOCAL_GPUS=8, while final ROCm 7.14 and ROCm 7.15 use 16 * without changing the amd_smi major. ************************************************************************/ +constexpr size_t kAmdSmiFabricInfo8GpuSize = 288; +constexpr size_t kAmdSmiFabricInfo16GpuSize = 320; +constexpr size_t kAmdSmiFabricState8GpuOffset = 208; +constexpr size_t kAmdSmiFabricState16GpuOffset = 240; + constexpr bool amdSmiFabricLayoutIs8Gpu = - sizeof(amdsmi_fabric_info_v1_t) == 212 && sizeof(amdsmi_fabric_info_t) == 288 && - offsetof(amdsmi_fabric_info_v1_t, addr_mode) == 204 && offsetof(amdsmi_fabric_info_v1_t, accel_state) == 208 && + sizeof(amdsmi_fabric_info_v1_t) == 212 && sizeof(amdsmi_fabric_info_t) == kAmdSmiFabricInfo8GpuSize && + offsetof(amdsmi_fabric_info_v1_t, addr_mode) == kAmdSmiFabricState8GpuOffset - sizeof(uint32_t) && + offsetof(amdsmi_fabric_info_v1_t, accel_state) == kAmdSmiFabricState8GpuOffset && offsetof(amdsmi_fabric_info_t, reserved) == 224; constexpr bool amdSmiFabricLayoutIs16Gpu = - sizeof(amdsmi_fabric_info_v1_t) == 244 && sizeof(amdsmi_fabric_info_t) == 320 && - offsetof(amdsmi_fabric_info_v1_t, addr_mode) == 236 && offsetof(amdsmi_fabric_info_v1_t, accel_state) == 240 && + sizeof(amdsmi_fabric_info_v1_t) == 244 && sizeof(amdsmi_fabric_info_t) == kAmdSmiFabricInfo16GpuSize && + offsetof(amdsmi_fabric_info_v1_t, addr_mode) == kAmdSmiFabricState16GpuOffset - sizeof(uint32_t) && + offsetof(amdsmi_fabric_info_v1_t, accel_state) == kAmdSmiFabricState16GpuOffset && offsetof(amdsmi_fabric_info_t, reserved) == 256; static_assert(amdSmiFabricLayoutIs8Gpu || amdSmiFabricLayoutIs16Gpu, "unsupported amdsmi fabric layout"); @@ -563,6 +571,43 @@ inline const amdsmi_fabric_info_v1_t* amdSmiFabricInfoV1(const FabricInfoT& info } } +// amd_smi 26.x used both supported payload sizes under the same SONAME. Use a +// maximum-sized canary buffer so either runtime can write safely, then identify +// how much it wrote. Both implementations value-initialize a local struct and +// assign the complete object, making the final 32 bytes zero only for the +// 16-GPU runtime. +constexpr unsigned char kAmdSmiFabricBufferCanary = 0xA5; + +struct alignas(amdsmi_fabric_info_t) amdSmiFabricInfoBuffer { + unsigned char bytes[kAmdSmiFabricInfo16GpuSize]; +}; + +enum class amdSmiFabricRuntimeLayout { + EightGpu, + SixteenGpu, + Unknown, +}; + +inline void amdSmiPrepareFabricInfoBuffer(amdSmiFabricInfoBuffer& buffer) { + memset(buffer.bytes, kAmdSmiFabricBufferCanary, sizeof(buffer.bytes)); +} + +inline amdsmi_fabric_info_t* amdSmiFabricInfoBufferAsInfo(amdSmiFabricInfoBuffer& buffer) { + return reinterpret_cast(buffer.bytes); +} + +inline amdSmiFabricRuntimeLayout amdSmiDetectFabricRuntimeLayout(const amdSmiFabricInfoBuffer& buffer) { + bool tailIsCanary = true; + bool tailIsZero = true; + for (size_t i = kAmdSmiFabricInfo8GpuSize; i < kAmdSmiFabricInfo16GpuSize; ++i) { + tailIsCanary &= buffer.bytes[i] == kAmdSmiFabricBufferCanary; + tailIsZero &= buffer.bytes[i] == 0; + } + if (tailIsCanary) return amdSmiFabricRuntimeLayout::EightGpu; + if (tailIsZero) return amdSmiFabricRuntimeLayout::SixteenGpu; + return amdSmiFabricRuntimeLayout::Unknown; +} + // amd_smi 27.0 also changed amdsmi_fabric_telem_id_to_string from returning the name // to writing it through an out-parameter. The two are not call-compatible and we reach // it through dlopen, so the choice has to follow the version the loaded runtime diff --git a/projects/rccl/src/misc/amdsmi_wrap.cc b/projects/rccl/src/misc/amdsmi_wrap.cc index 4f0f24dff060..7c2b27533f62 100644 --- a/projects/rccl/src/misc/amdsmi_wrap.cc +++ b/projects/rccl/src/misc/amdsmi_wrap.cc @@ -626,6 +626,30 @@ ncclResult_t amd_smi_ensureFabricInitialized() { fabricInitResult = ncclInternalError; return fabricInitResult; } + + // amd_smi 26.x shipped both fabric payload sizes under the same SONAME, so + // the library version cannot identify a mixed header/runtime installation. + // Probe through an oversized canary buffer before using the typed structure. + // If the layouts differ (or cannot be identified), use the sysfs backend. + if (!useSysfs && amd_smi_FabricFunctionsLoaded()) { + amdsmi_processor_handle probeHandle; + if (getProcessorHandle(0, &probeHandle) == ncclSuccess) { + amdSmiFabricInfoBuffer probeBuffer; + amdSmiPrepareFabricInfoBuffer(probeBuffer); + pfn_amdsmi_get_gpu_fabric_info(probeHandle, amdSmiFabricInfoBufferAsInfo(probeBuffer)); + const amdSmiFabricRuntimeLayout runtimeLayout = amdSmiDetectFabricRuntimeLayout(probeBuffer); + const bool runtimeLayoutIs8Gpu = runtimeLayout == amdSmiFabricRuntimeLayout::EightGpu; + if (runtimeLayout == amdSmiFabricRuntimeLayout::Unknown) { + WARN("AMD SMI fabric: unable to verify the loaded library's fabric ABI; falling back to sysfs"); + useSysfs = true; + } else if (runtimeLayoutIs8Gpu != amdSmiFabricLayoutIs8Gpu) { + WARN("AMD SMI fabric ABI mismatch: RCCL was built for the %u-GPU layout, but the loaded library uses the " + "%u-GPU layout; falling back to sysfs", + amdSmiFabricLayoutIs8Gpu ? 8 : 16, runtimeLayoutIs8Gpu ? 8 : 16); + useSysfs = true; + } + } + } amdsmiFabricDeviceCount = (int)numDevs; for (uint32_t d = 0; d < numDevs; d++) { diff --git a/projects/rccl/test/AmdSmiFabricTests.cpp b/projects/rccl/test/AmdSmiFabricTests.cpp index 6e504bccde83..fc59032188ab 100644 --- a/projects/rccl/test/AmdSmiFabricTests.cpp +++ b/projects/rccl/test/AmdSmiFabricTests.cpp @@ -7,6 +7,7 @@ #include "amdsmi_wrap.h" #include +#include #include namespace RcclUnitTesting @@ -133,6 +134,33 @@ TEST(AmdSmiFabricLayout, TrailingFieldsAreNotShifted) EXPECT_EQ(offsetof(amdsmi_fabric_info_v1_t, accel_state), expectedAddrMode + sizeof(uint32_t)); } +TEST(AmdSmiFabricRuntimeLayout, DetectsEightGpuWriter) +{ + amdSmiFabricInfoBuffer buffer; + amdSmiPrepareFabricInfoBuffer(buffer); + memset(buffer.bytes, 0, kAmdSmiFabricInfo8GpuSize); + + EXPECT_EQ(amdSmiDetectFabricRuntimeLayout(buffer), amdSmiFabricRuntimeLayout::EightGpu); +} + +TEST(AmdSmiFabricRuntimeLayout, DetectsSixteenGpuWriter) +{ + amdSmiFabricInfoBuffer buffer; + amdSmiPrepareFabricInfoBuffer(buffer); + memset(buffer.bytes, 0, kAmdSmiFabricInfo16GpuSize); + + EXPECT_EQ(amdSmiDetectFabricRuntimeLayout(buffer), amdSmiFabricRuntimeLayout::SixteenGpu); +} + +TEST(AmdSmiFabricRuntimeLayout, RejectsUnknownWriter) +{ + amdSmiFabricInfoBuffer buffer; + amdSmiPrepareFabricInfoBuffer(buffer); + buffer.bytes[kAmdSmiFabricInfo8GpuSize] = 0; + + EXPECT_EQ(amdSmiDetectFabricRuntimeLayout(buffer), amdSmiFabricRuntimeLayout::Unknown); +} + // --------------------------------------------------------------------------- // Both amdsmi_fabric_info_t shapes //