Repository navigation
test(hip-tests): add a dual-stream graph capture test - #14166
Conversation
A graph captured from two streams with an event fork and join per layer can run a node before its producer on another hardware queue finishes when the runtime drops a cross-queue wait. Small graph tests don't catch this: their kernels finish before the other queue starts, and their graphs are too short to spread over several queues. - Capture 32 layers shaped like a dual-stream MoE layer: a three-kernel branch and a one-kernel branch run in parallel on the two streams and an add joins them, feeding the next layer. - Kernels spin so the branches overlap; every intermediate buffer is poisoned before each of 20 replays and the result is compared exactly with a host reference. - Two cases: the one-kernel branch on the side stream, and the three-kernel branch on the side stream.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
🟡 Changes recommended
The layer arithmetic can erase early corruption before final validation, allowing ordering failures to escape detection.
1 open issue
What changed in this PR
Adds regression coverage for cross-queue ordering in deep, dual-stream HIP graph captures.
Changes:
- Adds two 32-layer graph tests with reversed branch placement and event-based fork/join dependencies.
- Replays each graph 20 times with poisoned buffers and host-reference validation.
- Registers both tests in the AMD build and level-2 graph test configuration.
| File | Description |
|---|---|
projects/hip-tests/catch/unit/graph/hipGraphDualStreamCapture.cc |
Implements dual-stream capture and replay tests. |
projects/hip-tests/catch/unit/graph/CMakeLists.txt |
Adds the test source to the AMD build. |
projects/hip-tests/catch/config/configs/unit/graph.yaml |
Registers both cases as level-2 execution tests. |
Review effort: Balanced
Each layer computed 146 * x + c. With an even multiplier the product over 32 layers is 0 mod 2^32, so every element ended at the same value and an even error in an early layer was erased by the later ones. - Use 10 instead of 11 for the shared branch: the per-layer multiplier becomes 145, which is odd, so a wrong value in any layer still changes the final result.
The graph executor assigns queues per dependency level, not per capture stream, so either branch can share a queue with the add. With kernels that spun before reading their input, a consumer that started early waited out its own spin and usually read finished data, so a dropped wait was only caught for some placements. - Every kernel reads its inputs first, then spins, then writes; the add spins too. A consumer that starts before its producer has finished reads poison wherever the branches run. - Spin lengths are in microseconds, scaled by the device's wall clock rate. - Check that the host reference doesn't collapse all elements to one value. - Describe the two cases by capture order instead of by stream.
mangupta
left a comment
mangupta
left a comment
There was a problem hiding this comment.
🤖 AI-assisted pre-review (for @mangupta)
[SUGGESTION] Graph, events, streams and about 40 MB of device buffers leak when a REQUIRE fails
In RunDualStreamGraph, cleanup only happens at the end of the function. REQUIRE(expected[0] != expected[1]) and the per-replay REQUIRE(mismatches == 0) both run before that cleanup, as does every HIP_CHECK in CaptureLayers and the setup code. If any of them fails, Catch2 throws and nothing is released: graph_exec, graph, 64 events, 2 non-blocking streams, and 161 device buffers of 256 KiB each (about 40 MB). If a HIP_CHECK fails inside CaptureLayers between hipStreamBeginCapture and hipStreamEndCapture, the main stream also stays in global capture mode.
Suggested fix: Hold the resources in RAII wrappers or a scope guard so they are released when an exception unwinds: streams, events, buffers, graph and graph exec. A simpler option is to record the mismatch count, run cleanup, and only then REQUIRE. In CaptureLayers, end or abort the capture before propagating a failure, for example with a guard that calls hipStreamEndCapture and destroys any partial graph.
## Motivation New untested graph path ## Technical Details A graph captured from two streams with an event fork and join per layer can run a node before its producer on another hardware queue finishes when the runtime drops a cross-queue wait. Small graph tests don't catch this: their kernels finish before the other queue starts, and their graphs are too short to spread over several queues. - Capture 32 layers shaped like a dual-stream MoE layer: a three-kernel branch and a one-kernel branch run in parallel on the two streams and an add joins them, feeding the next layer. - Kernels spin so the branches overlap; every intermediate buffer is poisoned before each of 20 replays and the result is compared exactly with a host reference. - Two cases: the one-kernel branch on the side stream, and the three-kernel branch on the side stream ## Issue Tracking JIRA ID : ROCM-1667 ## Test Plan It's the test ## Test Result Pass ## Submission Checklist N/A
TheRock Submodule Bump ActivityNewest first
|
Motivation
New untested graph path
Technical Details
A graph captured from two streams with an event fork and join per layer can run a node before its producer on another hardware queue finishes when the runtime drops a cross-queue wait. Small graph tests don't catch this: their kernels finish before the other queue starts, and their graphs are too short to spread over several queues.
Issue Tracking
JIRA ID : ROCM-1667
Test Plan
It's the test
Test Result
Pass
Submission Checklist
N/A