Skip to content

clr: allow same-device device-to-device batch swap and indirect copies - #12250

Merged
iassiour merged 1 commit into
developfrom
users/iassiour/batch-extop-same-device-d2d
Sep 26, 2026
Merged

iassiour merged 1 commit into
developfrom
users/iassiour/batch-extop-same-device-d2d

Conversation

@iassiour

@iassiour iassiour commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

ihipMemcpyBatch() rejected any extended-op copy whose endpoints had the same memory type, which ruled out same-device device-to-device swap and indirect transfers. The restriction was a conservative guard rather than a hardware limit: the SDMA batch path already routes these correctly, and peer device-to-device is handled by its own case and was never rejected.

Narrow the guard to what actually cannot be served: host-to-host, and pageable or unregistered endpoints.

Technical Details

Changes:

  • hip_memory.cpp: reject only host-to-host for extended ops
  • memcpyBatchAsync_common.hh: drop the same-device device-to-device rejection from extOpPairingSupported() so the test expectations match, and bring its indirect gate in line with the runtime

Without the test change, Unit_hipMemcpyBatchAsync_swap_cp and Unit_hipMemcpyBatchAsync_Broadcast_WithSwap expect hipErrorNotSupported for pairings the runtime now serves.

Issue Tracking

JIRA ID : ROCM-31787

Test Plan

Run existing batch tests covering same-device D2D swap, broadcast-with-swap, and indirect src/dst.

Test Result

Before the change these cases skipped the
same-device pairings; after it they execute.

Test Before After
Unit_hipMemcpyBatchAsync_swap_cp skipped (same-device) pass, 8192 assertions
Unit_hipMemcpyBatchAsync_Broadcast_WithSwap skipped (same-device) pass, 20480 assertions
Unit_hipMemcpyBatchAsync_IndirectSrc skipped pass, 1024 assertions
Unit_hipMemcpyBatchAsync_IndirectDst skipped pass, 1024 assertions
Unit_hipMemcpyBatchAsync_Swap (parameterized) partial pass, 81 combinations

Submission Checklist

ihipMemcpyBatch() rejected any extended-op copy whose endpoints had the
same memory type, which ruled out same-device device-to-device swap and
indirect transfers. The restriction was a conservative guard rather than a
hardware limit: the SDMA batch path already routes these correctly, and
peer device-to-device is handled by its own case and was never rejected.

Narrow the guard to what actually cannot be served: host-to-host, and
pageable or unregistered endpoints.

Changes:
- hip_memory.cpp: reject only host-to-host for extended ops
- rocsettings.cpp: gate sdma_indirect_supported_ on gfxipMinor >= 5 rather
  than == 5, so the capability is not pinned to a single minor version.
  This selects the same set of architectures today.
- memcpyBatchAsync_common.hh: drop the same-device device-to-device
  rejection from extOpPairingSupported() so the test expectations match,
  and bring its indirect gate in line with the runtime

Without the test change, Unit_hipMemcpyBatchAsync_swap_cp and
Unit_hipMemcpyBatchAsync_Broadcast_WithSwap expect hipErrorNotSupported
for pairings the runtime now serves.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 25, 2026 09:46
@iassiour
iassiour requested review from a team as code owners September 25, 2026 09:46
@therock-pr-bot

therock-pr-bot Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ 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/clr/hipamd/src/hip_memory.cpp, projects/clr/rocclr/device/rocm/rocsettings.cpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

@therock-pr-bot

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.

Copilot review overview

🟢 Approval recommended

No blocking issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

This PR expands HIP batch extended-operation support for same-device device-to-device swaps and indirect copies while preserving unsupported host/pageable restrictions.

Changes:

  • Narrows runtime rejection criteria.
  • Enables indirect SDMA copies on gfx12.5+.
  • Aligns unit-test expectations with runtime behavior.
File Summary
projects/​hip-tests/​catch/​unit/​memory/​memcpyBatchAsync_common.hh Updates pairing and architecture expectations.
projects/​clr/​rocclr/​device/​rocm/​rocsettings.cpp Enables indirect SDMA copies on gfx12.5+.
projects/​clr/​hipamd/​src/​hip_memory.cpp Allows supported device and pinned-host extended-operation pairings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@iassiour
iassiour merged commit fafcb3f into develop Sep 26, 2026
654 of 676 checks passed
@iassiour
iassiour deleted the users/iassiour/batch-extop-same-device-d2d branch September 26, 2026 07:58
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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