Skip to content

[rocjitsu] Return SOPP encoding errors as results - #14403

Merged
IanWood1 merged 2 commits into
developfrom
users/IanWood1/rocjitsu-sopp-builder-errors
Oct 28, 2026
Merged

IanWood1 merged 2 commits into
developfrom
users/IanWood1/rocjitsu-sopp-builder-errors

Conversation

@IanWood1

Copy link
Copy Markdown
Member

Motivation

Remove the explicit unsupported-architecture throw from build_sopp_encoding.

Technical Details

Return FailureOr with optional diagnostics and update callers to propagate failure or unwrap after architecture validation. Other SOPP opcode helpers and frontends retain their existing exceptions for a later change.

Cover unsupported-architecture results and diagnostics, and update existing encoding and DBI tests for the result contract.

Issue Tracking

Part of #10239.

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

therock-pr-bot Bot commented Oct 27, 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?

@therock-pr-bot

Copy link
Copy Markdown

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

@IanWood1
IanWood1 marked this pull request as ready for review October 27, 2026 20:36
@IanWood1
IanWood1 requested review from a team and atgutier as code owners October 27, 2026 20:36
Copilot AI balanced review requested due to automatic review settings October 27, 2026 20:36

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 was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@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.

Should we also forward error through the build_sopp_encoding calls in build_wait_scalar_loads_complete (emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/builders/smem_builders.h:228, with the same pattern through line 246)? With the new emitter parameter in emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/builders/instruction_builder.h:124, this caller already forwards the result but leaves its diagnostic context behind. Passing the same emitter would keep the two parts of the composed API together. This is optional plumbing cleanup: every current architecture case succeeds, so there is no missing diagnostic on a reachable failure today.

Signed-off-by: Ian Wood <ianwood@u.northwestern.edu>
@IanWood1
IanWood1 requested a review from kuhar October 28, 2026 18:57

@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 bab0ba7 into develop Oct 28, 2026
67 checks passed
@IanWood1
IanWood1 deleted the users/IanWood1/rocjitsu-sopp-builder-errors branch October 28, 2026 20:12
@github-actions

Copy link
Copy Markdown
Contributor

rocjitsu post-commit fuzz passed

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

  • ASan+UBSan (6-minute): 591,534 input executions (1,675.60 inputs/s)
  • TSan (6-minute): 581,554 input executions (1,657.77 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