Skip to content

fix(hip-tests): limit peer-import VMM copy overlap test to 2 streams - #14829

Merged
px1624 merged 1 commit into
developfrom
amd/dev/px1624/fix-vmm-imported-cpy-overlap-in-cpx
Nov 3, 2026
Merged

px1624 merged 1 commit into
developfrom
amd/dev/px1624/fix-vmm-imported-cpy-overlap-in-cpx

Conversation

@px1624

@px1624 px1624 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Motivation

Unit_hipMemImportFromShareableHandle_ImportFromPeersCopiesOverlapAcrossStreams failed on MI350X in CPX + NPS2

Technical Details

True P2P copies should run on SDMA, so their parallelism is bounded by the copy device's SDMA engines. A CPX partition has only 2, with no preferred engine to the other partitions of the same GPU. The test used up to 4 streams: one blit copy (device 0 -> device 0) and 3 SDMA copies (devices 1-3 -> device 0), so two peer copies shared an engine and the one held by hipStreamWaitValue32 blocked the other. This is expected behavior, not a runtime bug.

Use 2 streams, the minimum to show overlap: the device 0 <-> device 0 blit copy and the device 1 -> device 0 SDMA copy should overlap. Document that the SelfImport and ImportOnPeers cases allocate physical memory on device 0, so all of their copies are blit copies.

Issue Tracking

JIRA ID: ROCM-33753

Test Plan

Validate failed tests is passing with this PR

Test Result

The 5 repeat runs of the fixed ImportFromPeersCopiesOverlapAcrossStreams test:
All tests passed (11 assertions in 1 test case)
All tests passed (11 assertions in 1 test case)
All tests passed (11 assertions in 1 test case)
All tests passed (11 assertions in 1 test case)
All tests passed (11 assertions in 1 test case)

Submission Checklist

Unit_hipMemImportFromShareableHandle_ImportFromPeersCopiesOverlapAcrossStreams
failed on MI350X in CPX + NPS2 ("held back copy 1", other_copies_done == false).

True P2P copies should run on SDMA, so their parallelism is bounded by the copy
device's SDMA engines. A CPX partition has only 2, with no preferred engine to
the other partitions of the same GPU. The test used up to 4 streams: one blit
copy (device 0 -> device 0) and 3 SDMA copies (devices 1-3 -> device 0), so two
peer copies shared an engine and the one held by hipStreamWaitValue32 blocked
the other. This is expected behavior, not a runtime bug.

Use 2 streams, the minimum to show overlap: the device 0 <-> device 0 blit copy
and the device 1 -> device 0 SDMA copy should overlap. Document that the
SelfImport and ImportOnPeers cases allocate physical memory on device 0, so all
of their copies are blit copies.

JIRA ID: ROCM-33753
Copilot AI balanced review requested due to automatic review settings November 2, 2026 18:10
@px1624
px1624 requested a review from a team as a code owner November 2, 2026 18:10
@therock-pr-bot

therock-pr-bot Bot commented Nov 2, 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/hip-tests/catch/unit/virtualMemoryManagement/hipMemImportFromShareableHandle.cc; 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?

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.

🟢 Approval recommended

No active review issues were identified for the peer-import overlap test adjustment.

No open issues 🎉

What changed in this PR

Adjusts the HIP virtual-memory peer-import overlap test to avoid expected SDMA engine contention on MI350X CPX partitions while preserving overlap coverage.

Changes:

  • Limits the peer-import test to two streams: one local blit copy and one peer SDMA copy.
  • Updates comments to explain the stream limit and clarify that SelfImport and ImportOnPeers use only blit copies.
File Description
projects/​hip-tests/​catch/​unit/​virtualMemoryManagement/​hipMemImportFromShareableHandle.cc Limits peer-import overlap testing to two streams and documents copy-engine behavior.

Review effort: Balanced

@px1624
px1624 merged commit f4479fb into develop Nov 3, 2026
281 of 292 checks passed
@px1624
px1624 deleted the amd/dev/px1624/fix-vmm-imported-cpy-overlap-in-cpx branch November 3, 2026 23:38
@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.

4 participants