Skip to content

[rocjitsu] Return DBI test fixture encoding failures - #14869

Merged
IanWood1 merged 18 commits into
developfrom
users/IanWood1/rocjitsu-dbi-fixture-errors
Nov 5, 2026
Merged

IanWood1 merged 18 commits into
developfrom
users/IanWood1/rocjitsu-dbi-fixture-errors

Conversation

@IanWood1

@IanWood1 IanWood1 commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Motivation

Make DBI test fixtures report encoding failures explicitly so rejected inputs cannot be consumed as successful instruction encodings.

Technical Details

Return Result or FailureOr from global-memory builders and log-record probe helpers, preserve their rejection diagnostics, and check results in simulator fixtures. Invalid offsets, register tuples, architectures, slot counts, and oversized probe sites have regression coverage. The RelWithDebInfo wrapper build, changed-file hooks, and 295 focused builder, probe, spill, and DBI tests passed. HSA simulation tests required running outside the sandbox after topology and memory-keeper permission failures. Full-suite, corpus, sanitizer, and hardware validation were not run.

Issue Tracking

Part of #10239. Builds on the merged scalar-builder changes in #14536.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Preserve builder reasons and probe-call stage context at remaining callers. Simplify migrated test sequences while retaining checked setup.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Keep the builder success checks while using the encoded word directly in repeated setup and expectations.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Reject opcode values that change instruction families and preserve architecture
validation diagnostics. Forward builder reasons with stage context and cover
the failure paths with focused regressions.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Retain checked scalar encodings while adopting fallible topology setup.
Adapt new upstream VM test kernels to check builder results before use.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Return checked failures for invalid DBI fixture encodings and log probe inputs so failed setup stops before consumers use the output. Preserve malformed operand cases and cover probe bounds and unsupported targets.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Forward global-encoder reasons through probe construction to the test
assertion, and check the existing rejection messages. Clarify the RDNA4
signed offset boundary and the unwrapped words type.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Preserve unsupported-instruction prefixes and route converted probe argument,
slot-count, and scalar builder failures into fixture assertions. Forward the
existing emitter to shared builders and check reasons in rejection tests.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Pass the existing emitter to the shared shift builder so its failure reason follows the same path as the other probe instructions.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
Base automatically changed from users/IanWood1/rocjitsu-scalar-builder-errors to develop November 3, 2026 23:05
@therock-pr-bot

therock-pr-bot Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

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

🙋 Wish to Override Policy?

Resolve integration conflicts with the merged scalar-builder changes.
Retain current production builders and diagnostics while preserving checked
results and failure propagation in the DBI test fixtures.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@therock-pr-bot

therock-pr-bot Bot commented Nov 3, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@IanWood1
IanWood1 marked this pull request as ready for review November 5, 2026 07:29
@IanWood1
IanWood1 requested review from a team and atgutier as code owners November 5, 2026 07:29
Copilot AI balanced review requested due to automatic review settings November 5, 2026 07:29

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

There are no active review issues requiring resolution.

No open issues 🎉

What changed in this PR

This PR replaces exception-based encoding failures in rocjitsu’s DBI test helpers with explicit results, continuing the exception-reduction work in #10239 and the scalar-builder changes in #14536.

Changes:

  • Return Result or FailureOr from encoding and probe helpers, with failure diagnostics.
  • Update simulator fixtures to check results before consuming encoded instructions or probe arguments.
  • Add regression coverage for unsupported architectures, invalid operands and slot counts, and oversized probe sites.
File Description
emulation/​rocjitsu/​tests/​dbi/​scalar_builders.h Return explicit failures from the read-first-lane builder.
emulation/​rocjitsu/​tests/​dbi/​scalar_builders_test.cpp Check builder results and unsupported-architecture diagnostics.
emulation/​rocjitsu/​tests/​dbi/​log_record_probe.h Propagate probe argument and body encoding failures.
emulation/​rocjitsu/​tests/​dbi/​log_record_probe_test.cpp Cover invalid sites, slot counts, and architectures.
emulation/​rocjitsu/​tests/​dbi/​global_memory_builders.h Replace load/store encoding exceptions with diagnostic results.
emulation/​rocjitsu/​tests/​dbi/​dbi_spill_sim_test.cpp Return and check vector-add encoding results.
emulation/​rocjitsu/​tests/​dbi/​dbi_sim_memory_test.cpp Update result checks and expand rejection coverage.
emulation/​rocjitsu/​tests/​dbi/​dbi_log_record_sim_test.cpp Validate probe bodies and arguments before instrumentation.

Review effort: Balanced

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@IanWood1
IanWood1 merged commit bc0f3ea into develop Nov 5, 2026
79 of 81 checks passed
@IanWood1
IanWood1 deleted the users/IanWood1/rocjitsu-dbi-fixture-errors branch November 5, 2026 20:17
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

rocjitsu post-commit fuzz passed

The 32-worker multiplexer campaigns for commit bc0f3eaf1617 completed without recording a crash or hang.

  • ASan+UBSan (6-minute): 587,858 input executions (1,668.09 inputs/s)
  • TSan (6-minute): 488,219 input executions (1,396.12 inputs/s)

View the workflow run.

@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