Skip to content

[rocSHMEM] Reduce IPC RMA latency in symmetric address translation - #9872

Open
avinashkethineedi wants to merge 4 commits into
developfrom
users/akethine/ipc-constmem-latency-fix
Open

[rocSHMEM] Reduce IPC RMA latency in symmetric address translation#9872
avinashkethineedi wants to merge 4 commits into
developfrom
users/akethine/ipc-constmem-latency-fix

Conversation

@avinashkethineedi

@avinashkethineedi avinashkethineedi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

Symmetric user-buffer registration routed IPC get/put operations through ipcPeerPtr() to distinguish symmetric-heap addresses from registered buffers. This added heap metadata loads and a range check to the common RMA hot path, increasing short-message latency.

This PR removes the avoidable memory indirection while preserving registered-buffer support and the existing address-translation behavior.

Technical Details

  • Cache the local symmetric-heap base and size in device constant memory.
  • Use the cached values in the ipcPeerPtr() heap fast path.
  • Preserve registered symmetric-buffer lookup for addresses outside the heap.
  • Remove the unused local PE argument and update IPC/GDA call sites.

Issue Tracking

  • JIRA ID : AIROCSHMEM-487

Test Plan

  • Build IPC and GDA configurations.
  • Run the IPC heatmap against pre-feature, develop, and fix revisions.

Test Result

  • IPC builds and heatmap tests passed.
  • Median RMA latency improved by 10.3% versus develop.
  • Wave RMA latency improved by 12.1%.

Submission Checklist

@avinashkethineedi
avinashkethineedi requested a review from a team as a code owner August 7, 2026 18:50
Copilot AI lite review requested due to automatic review settings August 7, 2026 18:50
@therock-pr-bot

therock-pr-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
JIRA ID : TESTAUTO-6039
JIRA ID - #330
JIRA ID #330
JIRA ID (on separate line)
ROCM-25757
ISSUE ID : TESTUTO-3334
ISSUE ID #3334
ISSUE ID - TESTAUTO-3433
ISSUE ID (on separate line)
AIRUNTIME-2352
ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Closes #10
Fixes octo-org/octo-repo#100
Resolves: #123
#123
https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
Forbidden Files ✅ Pass
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocshmem/src/constmem.cpp, projects/rocshmem/src/constmem.hpp, projects/rocshmem/src/gda/context_gda_device.cpp, projects/rocshmem/src/gda/context_gda_device.hpp, projects/rocshmem/src/gda/context_gda_tmpl_device.hpp (+3 more); no test file found
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the rocSHMEM IPC RMA hot path by avoiding per-operation heap metadata loads/range checks during symmetric address translation, while preserving the fallback path for symmetrically-registered user buffers and updating affected IPC/GDA call sites.

Changes:

  • Cache the local symmetric heap base/size in device constant memory and use them in ipcPeerPtr()’s heap fast path.
  • Remove the unused local-PE argument from ipcPeerPtr() and update IPC + GDA device call sites accordingly.
  • Remove the GDA-side ipc_peer_ptr() wrapper and call ipcPeerPtr() directly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
projects/rocshmem/src/ipc/context_ipc_tmpl_device.hpp Updates IPC AMO paths to use the new ipcPeerPtr(sym_addr, pe) signature.
projects/rocshmem/src/ipc/context_ipc_device.cpp Updates IPC put/get and shmem_ptr paths to use the new ipcPeerPtr() signature.
projects/rocshmem/src/ipc_policy.hpp Updates ipcPeerPtr() implementation to use constmem.heap_base/heap_size and removes the my_pe parameter.
projects/rocshmem/src/gda/context_gda_tmpl_device.hpp Switches GDA templated ops to call ipcImpl_.ipcPeerPtr() directly.
projects/rocshmem/src/gda/context_gda_device.hpp Removes the now-unneeded ipc_peer_ptr() wrapper helper.
projects/rocshmem/src/gda/context_gda_device.cpp Updates GDA device operations to call ipcImpl_.ipcPeerPtr() directly.
projects/rocshmem/src/constmem.hpp Adds heap_base and heap_size fields to device constant memory.
projects/rocshmem/src/constmem.cpp Initializes the new constant-memory heap base/size values from the backend heap.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread projects/rocshmem/src/constmem.hpp
Comment thread projects/rocshmem/src/ipc_policy.hpp
Comment thread projects/rocshmem/src/ipc/context_ipc_device.cpp
Avoid per-operation local heap metadata indirection on IPC RMA paths, reducing fixed get/put latency without changing registered-buffer translation.
- Remove the unused PE parameter from ipcPeerPtr
- Update IPC and GDA call sites
- Call ipcPeerPtr directly from GDA paths
- Remove the forwarding helper
@avinashkethineedi
avinashkethineedi force-pushed the users/akethine/ipc-constmem-latency-fix branch from 188c5c8 to dff8fa9 Compare August 10, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants