From a5abd685f501ecc54e4e4962738caacce6ab3cb7 Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Tue, 15 Sep 2026 18:23:37 +0000 Subject: [PATCH 1/5] [AICOMRCCL-2359] [rccl] Probe fabric APIs with plain compiler in topo_expl The topo_expl tool bypasses RCCL's CMake configuration and reproduces the HIP/amdsmi fabric feature probes in its own Makefile. Those probes piped a snippet through hipcc, i.e. hipcc's HIP driver mode. On recent ROCm toolchains hipcc's HIP driver pulls in cuda_wrappers/bits/c++config.h, which clashes with a recent system libstdc++ (redeclaration of __glibcxx_assert_fail). That makes the otherwise-valid probe fail to compile; the '&& echo yes || echo no' guard swallows the error and returns 'no', so AMDSMI_FABRIC_DIRECT is left undefined. With AMDSMI_FABRIC_DIRECT unset, amdsmi_wrap.h compiles its compat copies of the UALoE fabric types. On base images whose amd_smi/amdsmi.h now defines those types natively (ROCm 10.2.0 / TheRock main), the compat definitions collide with the real header and the topo_expl target fails to build, breaking the whole extra build. The main librccl build was unaffected because CMake probes with the plain compiler (check_cxx_source_compiles), which does not go through the HIP driver and correctly detected the API. Probe the fabric APIs the same way CMake does: compile a real temp file with the plain compiler (amdclang++, overridable via PROBE_CXX) instead of piping through hipcc's HIP driver. Both probes now agree with the CMake build, so AMDSMI_FABRIC_DIRECT is set correctly and the compat block is skipped when the real header provides the fabric API. Co-authored-by: Pi Agent --- projects/rccl/tools/topo_expl/Makefile | 32 ++++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/projects/rccl/tools/topo_expl/Makefile b/projects/rccl/tools/topo_expl/Makefile index e48f7f7e4171..6cf9da18a49a 100644 --- a/projects/rccl/tools/topo_expl/Makefile +++ b/projects/rccl/tools/topo_expl/Makefile @@ -1,6 +1,15 @@ # Copyright (c) 2019-2026 Advanced Micro Devices, Inc. All rights reserved. ROCM_PATH ?= /opt/rocm HIPCC = $(ROCM_PATH)/bin/hipcc + +# Plain (non-HIP-driver) compiler used only for compile-time feature probes. +# The probes below check host headers (amd_smi/amdsmi.h) and API surface, not +# device code, so they must NOT go through hipcc's HIP driver mode: that pulls +# in the cuda_wrappers/bits/c++config.h shim, which clashes with recent system +# libstdc++ headers and makes an otherwise-valid probe fail. RCCL's CMake build +# probes with the plain compiler (check_cxx_source_compiles) and gets the right +# answer; use the same mechanism here so the two build systems agree. +PROBE_CXX ?= $(ROCM_PATH)/bin/amdclang++ HIPIFY_PL_EXE ?= $(ROCM_PATH)/bin/hipify-perl HIPIFY_PL_FLAGS = -inplace -quiet-warnings @@ -29,11 +38,16 @@ 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. +# compile-time feature probes needed by headers it imports. Probes compile a +# real temp file (like CMake's check_cxx_source_compiles) with the plain +# compiler PROBE_CXX rather than piping through hipcc's HIP driver. +PROBE_TMP := $(shell mktemp --suffix=.cpp) + 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 - \ + 'int main() { hipMemFabricHandle_t h{}; (void)h; auto fn = &hipMemImportFromShareableHandle; (void)fn; }' \ + > $(PROBE_TMP); \ + $(PROBE_CXX) -x c++ -std=c++17 -fsyntax-only -D__HIP_PLATFORM_AMD__ -I$(ROCM_PATH)/include $(PROBE_TMP) \ >/dev/null 2>&1 && echo yes || echo no) ifeq ($(HAVE_HIP_FABRIC_API),yes) FEATURE_DEFINES += -DHIP_FABRIC_API @@ -41,8 +55,9 @@ 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 - \ + 'int main() { amdsmi_fabric_info_t info{}; (void)info; }' \ + > $(PROBE_TMP); \ + $(PROBE_CXX) -x c++ -std=c++17 -fsyntax-only -I$(ROCM_PATH)/include $(PROBE_TMP) \ >/dev/null 2>&1 && echo yes || echo no) ifeq ($(HAVE_AMDSMI_FABRIC_API),yes) FEATURE_DEFINES += -DAMDSMI_FABRIC_DIRECT @@ -53,13 +68,16 @@ endif # compat typedefs that collide with the system header (extra topo_expl compile). HAVE_AMDSMI_FABRIC_TELEMETRY := $(shell printf '%s\n' \ '#include ' \ - 'int main() { amdsmi_fabric_telemetry_category_t cat = AMDSMI_FABRIC_TELEMETRY_CATEGORY_UNKNOWN; (void)cat; }' | \ - $(HIPCC) -x c++ -std=c++17 -fsyntax-only -I$(ROCM_PATH)/include - \ + 'int main() { amdsmi_fabric_telemetry_category_t cat = AMDSMI_FABRIC_TELEMETRY_CATEGORY_UNKNOWN; (void)cat; }' \ + > $(PROBE_TMP); \ + $(PROBE_CXX) -x c++ -std=c++17 -fsyntax-only -I$(ROCM_PATH)/include $(PROBE_TMP) \ >/dev/null 2>&1 && echo yes || echo no) ifeq ($(HAVE_AMDSMI_FABRIC_TELEMETRY),yes) FEATURE_DEFINES += -DAMDSMI_HEADER_HAS_FABRIC_TELEMETRY -DAMDSMI_FABRIC_DIRECT endif +$(shell rm -f $(PROBE_TMP)) + CXXFLAGS = -g -ffunction-sections -fdata-sections -DROCM_VERSION=$(ROCM_VERSION) \ -std=c++17 \ -Wl,--gc-sections -fgpu-rdc \ From 127a426574b38ce1e4ab7d2a4cab14f994b0ade5 Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Tue, 15 Sep 2026 19:22:04 +0000 Subject: [PATCH 2/5] [AICOMRCCL-2359] [rccl] Harden topo_expl feature-probe compiler detection Locate the probe compiler across bin/ and llvm/bin/ like toolchain-linux.cmake and fail loudly when none is executable, so a missing compiler no longer degrades a probe to "no". Gate each probe compile on a successful temp-file write and assign the cleanup shell call to a variable. Co-authored-by: Pi Agent --- projects/rccl/tools/topo_expl/Makefile | 33 ++++++++++++++++---------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/projects/rccl/tools/topo_expl/Makefile b/projects/rccl/tools/topo_expl/Makefile index 6cf9da18a49a..7a16ea5ea46a 100644 --- a/projects/rccl/tools/topo_expl/Makefile +++ b/projects/rccl/tools/topo_expl/Makefile @@ -2,14 +2,23 @@ ROCM_PATH ?= /opt/rocm HIPCC = $(ROCM_PATH)/bin/hipcc -# Plain (non-HIP-driver) compiler used only for compile-time feature probes. -# The probes below check host headers (amd_smi/amdsmi.h) and API surface, not -# device code, so they must NOT go through hipcc's HIP driver mode: that pulls -# in the cuda_wrappers/bits/c++config.h shim, which clashes with recent system -# libstdc++ headers and makes an otherwise-valid probe fail. RCCL's CMake build -# probes with the plain compiler (check_cxx_source_compiles) and gets the right -# answer; use the same mechanism here so the two build systems agree. -PROBE_CXX ?= $(ROCM_PATH)/bin/amdclang++ +# Probe compiler. Not hipcc: its HIP driver mode pulls cuda_wrappers/bits/c++config.h, +# which clashes with recent libstdc++ and makes a valid probe fail. +# Locate it the same way toolchain-linux.cmake does (bin/ then llvm/bin/); a +# missing probe compiler must fail loudly, not silently degrade a probe to "no" +# and recompile the colliding UALoE compat types this build set out to avoid. +PROBE_CXX ?= $(shell \ + for c in $(ROCM_PATH)/bin/amdclang++ $(ROCM_PATH)/llvm/bin/amdclang++ $(ROCM_PATH)/llvm/bin/clang++; do \ + if [ -x "$$c" ]; then echo "$$c"; break; fi; \ + done) +ifeq ($(strip $(PROBE_CXX)),) +$(error Cannot find amdclang++/clang++ under $(ROCM_PATH)/bin or $(ROCM_PATH)/llvm/bin. Set PROBE_CXX explicitly.) +endif +ifeq ($(wildcard $(PROBE_CXX)),) +ifeq ($(shell command -v $(PROBE_CXX) >/dev/null 2>&1 && echo ok),) +$(error PROBE_CXX ($(PROBE_CXX)) is not executable. Set PROBE_CXX to a valid C++ compiler.) +endif +endif HIPIFY_PL_EXE ?= $(ROCM_PATH)/bin/hipify-perl HIPIFY_PL_FLAGS = -inplace -quiet-warnings @@ -46,7 +55,7 @@ PROBE_TMP := $(shell mktemp --suffix=.cpp) HAVE_HIP_FABRIC_API := $(shell printf '%s\n' \ '#include ' \ 'int main() { hipMemFabricHandle_t h{}; (void)h; auto fn = &hipMemImportFromShareableHandle; (void)fn; }' \ - > $(PROBE_TMP); \ + > $(PROBE_TMP) && \ $(PROBE_CXX) -x c++ -std=c++17 -fsyntax-only -D__HIP_PLATFORM_AMD__ -I$(ROCM_PATH)/include $(PROBE_TMP) \ >/dev/null 2>&1 && echo yes || echo no) ifeq ($(HAVE_HIP_FABRIC_API),yes) @@ -56,7 +65,7 @@ endif HAVE_AMDSMI_FABRIC_API := $(shell printf '%s\n' \ '#include ' \ 'int main() { amdsmi_fabric_info_t info{}; (void)info; }' \ - > $(PROBE_TMP); \ + > $(PROBE_TMP) && \ $(PROBE_CXX) -x c++ -std=c++17 -fsyntax-only -I$(ROCM_PATH)/include $(PROBE_TMP) \ >/dev/null 2>&1 && echo yes || echo no) ifeq ($(HAVE_AMDSMI_FABRIC_API),yes) @@ -69,14 +78,14 @@ endif HAVE_AMDSMI_FABRIC_TELEMETRY := $(shell printf '%s\n' \ '#include ' \ 'int main() { amdsmi_fabric_telemetry_category_t cat = AMDSMI_FABRIC_TELEMETRY_CATEGORY_UNKNOWN; (void)cat; }' \ - > $(PROBE_TMP); \ + > $(PROBE_TMP) && \ $(PROBE_CXX) -x c++ -std=c++17 -fsyntax-only -I$(ROCM_PATH)/include $(PROBE_TMP) \ >/dev/null 2>&1 && echo yes || echo no) ifeq ($(HAVE_AMDSMI_FABRIC_TELEMETRY),yes) FEATURE_DEFINES += -DAMDSMI_HEADER_HAS_FABRIC_TELEMETRY -DAMDSMI_FABRIC_DIRECT endif -$(shell rm -f $(PROBE_TMP)) +PROBE_CLEANUP := $(shell rm -f $(PROBE_TMP)) CXXFLAGS = -g -ffunction-sections -fdata-sections -DROCM_VERSION=$(ROCM_VERSION) \ -std=c++17 \ From 3d9d61dce20028a2251dea52d0077b3fb9e5b5d3 Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Tue, 15 Sep 2026 19:55:56 +0000 Subject: [PATCH 3/5] [AICOMRCCL-2359] [rccl] Fix topo_expl PROBE_CXX executability guard The previous guard tested existence via $(wildcard) and nested the command -v check, so a PROBE_CXX that exists but is not runnable skipped the check and the not-executable error never fired. Resolve PROBE_CXX and test -x on the resolved path so the exists-but-not-runnable case is caught. Co-authored-by: Pi Agent --- projects/rccl/tools/topo_expl/Makefile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/projects/rccl/tools/topo_expl/Makefile b/projects/rccl/tools/topo_expl/Makefile index 7a16ea5ea46a..a6d0bc270fea 100644 --- a/projects/rccl/tools/topo_expl/Makefile +++ b/projects/rccl/tools/topo_expl/Makefile @@ -14,11 +14,12 @@ PROBE_CXX ?= $(shell \ ifeq ($(strip $(PROBE_CXX)),) $(error Cannot find amdclang++/clang++ under $(ROCM_PATH)/bin or $(ROCM_PATH)/llvm/bin. Set PROBE_CXX explicitly.) endif -ifeq ($(wildcard $(PROBE_CXX)),) -ifeq ($(shell command -v $(PROBE_CXX) >/dev/null 2>&1 && echo ok),) +# Resolve PROBE_CXX (absolute path or bare name on PATH) and confirm the target +# is executable. Testing -x on the resolved path catches the exists-but-not- +# runnable case that a bare command -v would still report as found. +ifeq ($(shell p=$$(command -v $(PROBE_CXX) 2>/dev/null); [ -n "$$p" ] && [ -x "$$p" ] && echo ok),) $(error PROBE_CXX ($(PROBE_CXX)) is not executable. Set PROBE_CXX to a valid C++ compiler.) endif -endif HIPIFY_PL_EXE ?= $(ROCM_PATH)/bin/hipify-perl HIPIFY_PL_FLAGS = -inplace -quiet-warnings From eccab55af803477d9d9fd85f36d2083bf7658e46 Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Wed, 16 Sep 2026 15:45:16 +0000 Subject: [PATCH 4/5] [AICOMRCCL-2359] [rccl] Build topo_expl against amd_smi instead of rocm_smi topo_expl still included rocm_smi/rocm_smi.h and defined legacy rocm_smi_* discovery stubs, while the main RCCL build long ago migrated to amd_smi and forces USE_AMDSMI ON. On newer base images (ROCm >= 10.2 / TheRock main) the rocm_smi header no longer exists, so once the fabric-probe fix let the compile advance past the UALoE compat-type collision it failed with 'rocm_smi/rocm_smi.h file not found'. Align topo_expl with the library build: define USE_AMDSMI in the Makefile so the staged xml.cc/rccl_wrap.cc take the amd_smi discovery path, replace the rocm_smi include with amdsmi_wrap.h, and swap the rocm_smi_* stubs for the amd_smi_* entry points those sources call (init, getNumDevice, getDevicePciBusIdString, getDeviceIndexByPciBusId, getLinkInfo, getFabricDeviceInfo, getFirmwareVersion). The stubs report "no devices" so model-driven discovery falls through exactly as before. Testing: with rocm_smi/rocm_smi.h hidden to mimic the CI base image, all translation units now compile (-fsyntax-only exit 0) whereas pristine develop reproduces the original failure. Co-authored-by: Pi Agent --- projects/rccl/tools/topo_expl/Makefile | 1 + .../tools/topo_expl/src/topo_expl_impl.cpp | 23 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/projects/rccl/tools/topo_expl/Makefile b/projects/rccl/tools/topo_expl/Makefile index a6d0bc270fea..30db310ce743 100644 --- a/projects/rccl/tools/topo_expl/Makefile +++ b/projects/rccl/tools/topo_expl/Makefile @@ -95,6 +95,7 @@ CXXFLAGS = -g -ffunction-sections -fdata-sections -DROCM_VERSION=$(ROCM_VERSION) -Ihipify_rccl/include/nccl_device \ -Ihipify_rccl/src/device/include -Ihipify_rccl/graph -I$(ROCM_PATH)/include/ \ -DTOPO_EXPL -DENABLE_TRACE -DENABLE_LL128 -DNVTX_NO_IMPL -DRCCL_EXPOSE_STATIC \ + -DUSE_AMDSMI \ $(NCCL_OS_DEFINE) $(FEATURE_DEFINES) \ -lpthread diff --git a/projects/rccl/tools/topo_expl/src/topo_expl_impl.cpp b/projects/rccl/tools/topo_expl/src/topo_expl_impl.cpp index d823ae088025..6cbbaf162130 100644 --- a/projects/rccl/tools/topo_expl/src/topo_expl_impl.cpp +++ b/projects/rccl/tools/topo_expl/src/topo_expl_impl.cpp @@ -30,7 +30,7 @@ #include "coll_net.h" #include "model.h" #include "topo_expl_impl.h" -#include "rocm_smi/rocm_smi.h" +#include "amdsmi_wrap.h" const char* ncclFuncStr[NCCL_NUM_FUNCTIONS+4] = { "AllGather", "AllReduce", "AlltoAllPivot", "AlltoAllGda", "AlltoAllvGda", "Broadcast", "Reduce", "ReduceScatter", "SendRecv" }; const char* ncclAlgoStr[NCCL_NUM_ALGORITHMS] = { "Tree", "Ring", "CollNetDirect", "CollNetChain", "NVLS", "NVLSTree", "PAT" }; @@ -1327,23 +1327,34 @@ ncclResult_t initTransportsRank_3(struct ncclComm* comm, struct allGatherInfo *a goto exit; } -ncclResult_t rocm_smi_init() { +ncclResult_t amd_smi_init() { return ncclSuccess; } -ncclResult_t rocm_smi_getNumDevice(uint32_t* num_devs) { +ncclResult_t amd_smi_getNumDevice(uint32_t* num_devs) { + if (num_devs) *num_devs = 0; return ncclSuccess; } -ncclResult_t rocm_smi_getDevicePciBusIdString(uint32_t deviceIndex, char* busId, size_t len) { +ncclResult_t amd_smi_getDevicePciBusIdString(uint32_t deviceIndex, char* busId, size_t len) { return ncclSuccess; } -ncclResult_t rocm_smi_getDeviceIndexByPciBusId(const char* pciBusId, uint32_t* deviceIndex) { +ncclResult_t amd_smi_getDeviceIndexByPciBusId(const char* pciBusId, uint32_t* deviceIndex) { return ncclSuccess; } -ncclResult_t rocm_smi_getLinkInfo(int srcIndex, int dstIndex, RSMI_IO_LINK_TYPE* rsmi_type, int *hops, int *count) { +ncclResult_t amd_smi_getLinkInfo(int srcDev, int dstDev, amdsmi_link_type_t* type, int* hops, int* count) { + return ncclSuccess; +} + +ncclResult_t amd_smi_getFabricDeviceInfo(uint32_t deviceIndex, struct amdsmiFabricDeviceInfo* info) { + if (info) *info = {}; + return ncclSuccess; +} + +ncclResult_t amd_smi_getFirmwareVersion(uint32_t deviceIndex, uint64_t* fwVersion) { + if (fwVersion) *fwVersion = 0; return ncclSuccess; } From cc701d6717243dd3018bf01609199eab2c71ea0c Mon Sep 17 00:00:00 2001 From: Matt Hughes Date: Thu, 17 Sep 2026 14:17:22 +0000 Subject: [PATCH 5/5] [AICOMRCCL-2359] [rccl] Harden topo_expl probe compiler and temp-file guards Treat an explicitly empty PROBE_CXX override as unset so detection still runs, and fail loudly if mktemp cannot create the feature-probe temp file instead of silently degrading both probes. Co-authored-by: Pi Agent --- projects/rccl/tools/topo_expl/.gitignore | 1 + projects/rccl/tools/topo_expl/Makefile | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/projects/rccl/tools/topo_expl/.gitignore b/projects/rccl/tools/topo_expl/.gitignore index af79f3e4472a..3d5e122cd3d2 100644 --- a/projects/rccl/tools/topo_expl/.gitignore +++ b/projects/rccl/tools/topo_expl/.gitignore @@ -1,4 +1,5 @@ hipify_rccl topo_expl include/nccl.h +include/rccl/rccl.h third_party/ diff --git a/projects/rccl/tools/topo_expl/Makefile b/projects/rccl/tools/topo_expl/Makefile index 30db310ce743..43b3bea2a582 100644 --- a/projects/rccl/tools/topo_expl/Makefile +++ b/projects/rccl/tools/topo_expl/Makefile @@ -7,10 +7,14 @@ HIPCC = $(ROCM_PATH)/bin/hipcc # Locate it the same way toolchain-linux.cmake does (bin/ then llvm/bin/); a # missing probe compiler must fail loudly, not silently degrade a probe to "no" # and recompile the colliding UALoE compat types this build set out to avoid. -PROBE_CXX ?= $(shell \ +# Use ifeq rather than ?= so an explicitly empty override (PROBE_CXX= make) +# still triggers detection instead of being treated as already set. +ifeq ($(strip $(PROBE_CXX)),) +PROBE_CXX := $(shell \ for c in $(ROCM_PATH)/bin/amdclang++ $(ROCM_PATH)/llvm/bin/amdclang++ $(ROCM_PATH)/llvm/bin/clang++; do \ if [ -x "$$c" ]; then echo "$$c"; break; fi; \ done) +endif ifeq ($(strip $(PROBE_CXX)),) $(error Cannot find amdclang++/clang++ under $(ROCM_PATH)/bin or $(ROCM_PATH)/llvm/bin. Set PROBE_CXX explicitly.) endif @@ -52,6 +56,9 @@ endif # real temp file (like CMake's check_cxx_source_compiles) with the plain # compiler PROBE_CXX rather than piping through hipcc's HIP driver. PROBE_TMP := $(shell mktemp --suffix=.cpp) +ifeq ($(strip $(PROBE_TMP)),) +$(error Failed to create a temp file for the feature probes. Check that TMPDIR is writable.) +endif HAVE_HIP_FABRIC_API := $(shell printf '%s\n' \ '#include ' \