Repository navigation
feat(rccl/gin): enable rocSHMEM GDA alongside other GIN backends #14219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
c6dbbfe
79a372a
46c7fda
d8ef52f
85a839c
137f0eb
d9798cc
ba55da2
8f4bf8b
054b3cd
f7ab571
22bb53b
77782ff
9d23431
3b39e40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| /************************************************************************* | ||
| * Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved. | ||
| * | ||
| * See LICENSE.txt for license information | ||
| ************************************************************************/ | ||
|
|
||
| #ifndef RCCL_GIN_TYPE_ENV_H_ | ||
| #define RCCL_GIN_TYPE_ENV_H_ | ||
|
|
||
| #include <cerrno> | ||
| #include <cstdint> | ||
| #include <cstdlib> | ||
|
|
||
| // NCCL_GIN_TYPE forces one backend; if unset, RCCL_GIN_TYPE_LIST lists the types allowed together. | ||
| // Uncached getenv() so tests can setenv() between cases. | ||
|
|
||
| // Parses list into a bitmask of GIN types (bit t set for type t). Returns false if | ||
| // any entry is not an integer in [1, 63]; valid entries are still recorded in *mask. | ||
| static inline bool ncclGinParseTypeList(const char* list, uint64_t* mask) { | ||
| bool valid = true; | ||
| *mask = 0; | ||
| const char* p = list; | ||
| while (*p) { | ||
| while (*p == ' ' || *p == '\t') p++; | ||
| char* end = nullptr; | ||
| errno = 0; | ||
| long type = strtol(p, &end, 10); | ||
| while (*end == ' ' || *end == '\t') end++; | ||
| if (end == p || errno != 0 || type < 1 || type > 63 || (*end != ',' && *end != '\0')) { | ||
| valid = false; | ||
| while (*end != ',' && *end != '\0') end++; | ||
| } else { | ||
| *mask |= (uint64_t)1 << type; | ||
| } | ||
| p = (*end == ',') ? end + 1 : end; | ||
| } | ||
| return valid; | ||
| } | ||
|
|
||
| // Returns true and fills *mask when NCCL_GIN_TYPE is unset and RCCL_GIN_TYPE_LIST names | ||
| // at least one valid type. Otherwise the list does not apply. | ||
| static inline bool ncclGinTypeListMask(uint64_t* mask) { | ||
| if (getenv("NCCL_GIN_TYPE")) return false; | ||
| const char* list = getenv("RCCL_GIN_TYPE_LIST"); | ||
| if (!list) return false; | ||
| ncclGinParseTypeList(list, mask); | ||
| return *mask != 0; | ||
| } | ||
|
|
||
| // Whether the backend of the given type may initialize. enabledByDefault is its | ||
| // behavior when neither NCCL_GIN_TYPE nor a usable RCCL_GIN_TYPE_LIST is set. | ||
| static inline bool ncclGinTypeEnabled(int type, bool enabledByDefault) { | ||
| const char* single = getenv("NCCL_GIN_TYPE"); | ||
| if (single) return atoi(single) == type; | ||
| uint64_t mask; | ||
| if (ncclGinTypeListMask(&mask)) return (mask >> type) & 1; | ||
| return enabledByDefault; | ||
| } | ||
|
|
||
| #endif // RCCL_GIN_TYPE_ENV_H_ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,13 +14,15 @@ | |
| #include "nccl_gin.h" | ||
| #include "graph/topo.h" | ||
| #include "gin/gin_host_proxy.h" | ||
| #include "gin/gin_type_env.h" | ||
| #ifdef ENABLE_ROCSHMEM_GIN | ||
| #include "gin/gin_host_rocshmem_gda.h" | ||
| #include "gin/gin_host_anvil_sdma.h" | ||
| #endif | ||
|
|
||
| #include <string.h> | ||
| #include <errno.h> | ||
| #include <stdlib.h> | ||
| #include <mutex> | ||
|
|
||
| typedef ncclGin_t* getNcclGin_t(void* ginPluginLib); | ||
|
|
@@ -175,6 +177,13 @@ static ncclResult_t ncclGinPluginAssignToComm(struct ncclComm* comm, int pluginI | |
| return ncclSuccess; | ||
| } | ||
|
|
||
| uint64_t ginTypeMask; | ||
| if (ginType == -1 && ncclGinTypeListMask(&ginTypeMask) && !(ginTypeMask & BIT(props.netDeviceType))) { | ||
| INFO(NCCL_INIT | NCCL_NET, "GIN/Plugin: Skipping plugin %s index %d type %d: not in RCCL_GIN_TYPE_LIST=%s", | ||
| gin->name, pluginIndex, props.netDeviceType, getenv("RCCL_GIN_TYPE_LIST")); | ||
| return ncclSuccess; | ||
| } | ||
|
|
||
| for (int idx = 0; idx < pluginIndex; idx++) { | ||
| if (pluginLibs[idx].state >= ncclGinPluginStateInitReady) { | ||
| ncclNetProperties_t prev_props; | ||
|
|
@@ -240,6 +249,24 @@ static void initPluginLibsOnceFunc() { | |
| int pluginCounter = 0; | ||
|
|
||
| memset(pluginLibs, 0, NCCL_GIN_MAX_PLUGINS * sizeof(ginPluginLib_t)); | ||
|
|
||
| // Same getenv() source as ncclGinTypeListMask / ncclGinTypeEnabled, so an | ||
| // NCCL_ENV_PLUGIN value cannot be logged here and then ignored at init. | ||
| const char* envGinTypes = getenv("RCCL_GIN_TYPE_LIST"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The production path should use ncclGetEnv, ideally this probe should be cached in production path. That may entail stubbing more code for the test fixture (because the test fixture cannot cache the values). out-of-scope for this PR, a followup. |
||
| if (envGinTypes) { | ||
| uint64_t ginTypeMask; | ||
| bool valid = ncclGinParseTypeList(envGinTypes, &ginTypeMask); | ||
| if (getenv("NCCL_GIN_TYPE")) { | ||
| INFO(NCCL_ENV | NCCL_NET, "RCCL_GIN_TYPE_LIST=%s ignored: NCCL_GIN_TYPE is set", envGinTypes); | ||
| } else if (ginTypeMask == 0) { | ||
| WARN("RCCL_GIN_TYPE_LIST=%s names no valid GIN type, ignoring it", envGinTypes); | ||
| } else if (!valid) { | ||
| WARN("RCCL_GIN_TYPE_LIST=%s contains an invalid entry, ignoring that entry", envGinTypes); | ||
| } else { | ||
| INFO(NCCL_ENV | NCCL_NET, "RCCL_GIN_TYPE_LIST set by environment to %s", envGinTypes); | ||
| } | ||
| } | ||
|
|
||
| envGinPlugin = ncclGetEnv("NCCL_GIN_PLUGIN"); | ||
| if (envGinPlugin) { | ||
| INFO(NCCL_ENV | NCCL_NET, "NCCL_GIN_PLUGIN set by environment to %s", envGinPlugin); | ||
|
|
@@ -313,7 +340,7 @@ static void initPluginLibsOnceFunc() { | |
| pluginCounter++; | ||
|
|
||
| #ifdef ENABLE_ROCSHMEM_GIN | ||
| // Add internal rocshmem GDA plugin (device-initiated, NCCL_GIN_TYPE=6) | ||
| // Add internal rocshmem GDA plugin (device-initiated, NCCL_GIN_TYPE=6 or listed in RCCL_GIN_TYPE_LIST) | ||
| pluginLibs[pluginCounter].ncclGin = &ncclGinRocshmemGdaPlugin; | ||
| pluginLibs[pluginCounter].state = ncclGinPluginStateInitReady; | ||
| pluginLibs[pluginCounter].version = ncclGinVersion[0]; | ||
|
|
@@ -407,16 +434,17 @@ static int ncclGetBackendPriority(struct ncclGinBackendState* backend) { | |
| case NCCL_GIN_TYPE_GDAKI: | ||
| return 0; | ||
| #ifdef ENABLE_ROCSHMEM_GIN | ||
| // [RCCL] Device-initiated AMD backends rank with GDAKI, ahead of the host proxy. | ||
| case NCCL_GIN_TYPE_ROCSHMEM_GDA: | ||
| // [RCCL] Distinct priorities: qsort is unstable, so ties would make the default backend arbitrary. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that being said the high priority conflicts here are mutually exclusive at selection time. Is it a real issue that the ordering between GDA/SDMA is unstable? |
||
| case NCCL_GIN_TYPE_ANVIL_SDMA: | ||
| return 0; | ||
| case NCCL_GIN_TYPE_ROCSHMEM_GDA: | ||
| return 1; | ||
| #endif | ||
| case NCCL_GIN_TYPE_PROXY: | ||
| return 1; | ||
| return 2; | ||
| case NCCL_GIN_TYPE_EFA_GDA: | ||
| case NCCL_GIN_TYPE_GPI: | ||
| return 2; | ||
| return 3; | ||
| default: | ||
| return INT_MAX; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new file would be a target for elimination if we can reuse the existing infra for parsing string-based lists (e.g. from GIN_PLUGINS). Defer to string-based list.