[rocshmem] QP refactor to support GIN#7304
Merged
Merged
Conversation
Refactor the GDA QueuePair interface to support explicit rkey/lkey RDMA operations alongside the existing symmetric-heap path: - Drop unused `pe` parameter from put_nbi/get_nbi (already in ActiveWFInfo) - Add put_nbi/get_nbi overloads taking explicit raddr+rkey / laddr+lkey - Add atomic_add overload with explicit raddr+rkey - Add fenced put with doorbell coalescing (ring_db=false + fence flag) - Add quiet overload with ActiveWFInfo (matching new call sites) - Add dereg_mr_erase helper to ibv_wrapper for MR cleanup - Add LOG_TRACE diagnostics to buffer_register - Update all three providers (bnxt/mlx5/ionic) and context_gda_device callers for the new signatures
Thread explicit rkey/lkey through the entire _single chain so it can be used with externally registered memory regions: - post_wqe_rma_single: add lkey, rkey parameters - post_wqe_amo_single: add rkey, fence parameters - All three providers (mlx5/bnxt/ionic) _rma_single and _amo_single updated to use passed keys instead of QP member fields - ionic _rma_single: add ring_db support (matching non-single path) - All three providers _amo_single: add fence support Add new public overloads with explicit keys: - put_nbi_single(raddr, rkey, laddr, lkey, length, ring_db) - atomic_add_single(raddr, rkey, value, fence) Existing callers (put_nbi_single, get_nbi_single, atomic_nofetch_single) resolve keys from QP defaults at the call site, preserving behavior.
abouteiller
commented
Jun 16, 2026
- Remove forward declaration of rocshmem_gin_qp_set and friend access (gin_qp_factory moved to RCCL) - Make provider-specific QP fields public until gin_qp_factory uses a proper init API
1 task
edgargabriel
approved these changes
Jun 16, 2026
Previously mlx5 stored rkey/lkey/atomic_lkey in big-endian (htobe32 at init) and the WQE builders passed them directly. When the _with_keys API was merged, byteswap was added in the WQE builder, causing a double-swap on the rocshmem path (keys were already BE from init). Standardize on native little-endian for all key storage: - Remove htobe32 from QueuePair constructor (atomic lkeys) - Remove htobe32 from backend_gda_mlx5 (heap rkey/lkey) - Remove conditional htobe32 from buffer_register (user lkeys) - Add byteswap<uint32_t>(atomic_lkey) at mlx5 WQE construction This makes the key convention consistent across all providers and compatible with the GIN API which passes keys in native LE from verbs.
1 task
yalmusaf
pushed a commit
that referenced
this pull request
Jun 18, 2026
## Motivation Rework the QP class to be a generic GDA provider Despecialize the code so that it is not tied to the shmem API. * Explicit lkey/rkey * optional ring_db * optional fence-flag on signals * No context/backend dependency ## Technical Details The computation of lkey/rkey and identification of heap/loffset computation is up-leveled to the context class. ## JIRA ID https://amd-hub.atlassian.net/browse/AICOMRCCL-1040 ## Test Plan * [x] functional tests on bnxt/ionic/mlx5 * [ ] CI ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
abouteiller
added a commit
that referenced
this pull request
Jul 13, 2026
## Motivation Introduce an implementation for GIN KI using GDA QueuePairs imported rocSHMEM. Does not require rocshmem_init or other parts of rocshmem to be active at runtime. ## Technical Details * The integration does not require rocshmem_init or other internals of rocshmem to be active. * The GIN plugin calls directly into the QueuePair class (bypassing the shmem_api and other muxers) * The QueuePair.[ch]pp remains in rocSHMEM for the time being (to avoid code duplication and ongoing maintenance burden, this is temporary) * The initialization of the QueuePairs is done standalone from the Gin plugin. Most patches to rocshmem have already been upstreamed in PR #7304 and this PR will be resynched on it when its merged. ## JIRA ID AICOMRCCL-1040 ## Test Plan Run rccl-tests with alltoall -R 2 -D 3, env NCCL_GIN_TYPE=4 ## Test Result Runs on all 3 GDA providers, see Jira for details. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Kapil-Shyam-Pawar <KapilShyam.Pawar@amd.com> Co-authored-by: Sriraj <sriraj.paul@amd.com> Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Rework the QP class to be a generic GDA provider
Despecialize the code so that it is not tied to the shmem API.
Technical Details
The computation of lkey/rkey and identification of heap/loffset computation is up-leveled to the context class.
JIRA ID
https://amd-hub.atlassian.net/browse/AICOMRCCL-1040
Test Plan
Submission Checklist