Repository navigation
[rocjitsu] Remove exceptions from runtime test fixtures - #14957
Conversation
Move fallible queue and synthetic-instruction setup out of constructors, check wave and memory access helpers, and stop dependent test work on failure. Preserve partial-init cleanup and intentional exception injection. Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Keep fatal-failure guards at consuming test calls. Tail calls to queue registration need no additional wrapper because no dependent work follows. Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the changes replacing setup exceptions with explicit failure checks.
No open issues 🎉
What changed in this PR
This PR reduces exception use in rocjitsu’s runtime and SDMA test fixtures, supporting #10239 by replacing setup exceptions with explicit failure checks.
Changes:
- Move fallible queue and synthetic-instruction setup out of constructors into checked initialization methods.
- Check wave allocation and memory-access results before use, stopping dependent test work on failure.
- Preserve queue and address-space cleanup, and add a regression test for partial-workgroup wave cleanup.
| File | Description |
|---|---|
emulation/rocjitsu/tests/sdma_queue_scheduler_test.cpp |
Move address-space registration into checked SetUp(). |
emulation/rocjitsu/tests/perfsim_plugin_test.cpp |
Check synthetic-instruction initialization and wave allocation. |
emulation/rocjitsu/tests/memory_wait_scoreboard_test.cpp |
Replace instruction setup exceptions with checked initialization. |
emulation/rocjitsu/tests/ds_gws_test.cpp |
Return workgroup setup failures and test partial-wave cleanup. |
emulation/rocjitsu/tests/cpu_dispatch_pool_test.cpp |
Return and check plugin installation failure without throwing. |
emulation/rocjitsu/tests/cdna5_sdma_test.cpp |
Add checked queue initialization and memory access while preserving cleanup. |
Review effort: Balanced
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
We should stop the test when wave allocation fails in DispatchPoolFixture (emulation/rocjitsu/tests/cpu_dispatch_pool_test.cpp:85). The constructor's EXPECT_NE records the failure and still appends the null pointer. Forcing that allocation to fail in ZeroThreadsFallsBackToCallingThread produced the assertion and then a segfault at line 404. Moving allocation into a checked initializer would let each caller stop before it uses the fixture, alongside the CU check from #14611.
Nit: Could we group "util/result.h" with the other quoted project headers in emulation/rocjitsu/tests/ds_gws_test.cpp:40 and emulation/rocjitsu/tests/cdna5_sdma_test.cpp:11 wherever it is still needed? Both files already have a project include block, and this would keep the GWS test's three util dependencies together.
Stop fixture consumers after failed CU or wave setup. Preserve reserved wave rollback, scope stderr capture, and check SDMA snapshot and access preconditions without duplicating synthetic-instruction initialization. Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
rocjitsu post-commit fuzz passedThe 32-worker multiplexer campaigns for commit
|
TheRock Submodule Bump ActivityNewest first
|
Motivation
Remove setup exceptions from SDMA and runtime test fixtures.
Technical Details
Move fallible queue and synthetic-instruction setup out of constructors and check wave and memory-access results before use. Stop dependent test work after setup failure, preserve queue/address-space cleanup, and release earlier waves when workgroup setup fails. A focused regression covers partial-wave cleanup; intentional callback exception injection remains.
Fixture construction overlaps #14611; preserve its checked CU initialization when integrating.
Validated with the RelWithDebInfo wrapper build, 611 passing focused runtime/SDMA and AQL/SoC tests (22 architecture-gated skips), and changed-file pre-commit hooks. Full-suite, corpus, and sanitizer validation were not run.
Issue Tracking
Part of #10239.