Repository navigation
[rocjitsu] Dispatch operand validation by type - #10371
Merged
Merged
Conversation
Generated operand constructors validated each encoding by walking every operand-type predicate. Operand construction is the largest decoder hot-path cost, and common operand types appeared late in that linear chain. Emit a switch over OperandType so construction evaluates only the matching selector ranges while preserving deferred diagnostics. Delegate the literal16 constructor to the primary constructor so each generated translation unit contains one validation switch, regenerate all ten AMDGPU models, and extend the generator profile gates for the shared structure. On the pinned large-object benchmark, median valid-decode throughput improves 25.1%, cycles fall 19.7%, and instructions fall 35.6% across nine alternating CPU-pinned pairs with identical corpus results and checksums.
❌ PR Check — Action Required
📖 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes ROCjitsu’s AMDGPU decoder hot path by changing generated operand construction validation from a linear chain of operand-type predicates into a switch (opr_type) dispatch. This reduces work for common operand types while keeping the existing “deferred diagnostics” behavior, and consolidates literal16 validation by delegating the literal16 constructor to the primary constructor so each generated TU contains a single validation switch.
Changes:
- Update generated
Operandconstructors to validate selector intervals via aswitch (opr_type)instead of repeatedif (opr_type == ...)checks. - Make the literal16-display constructor delegate to the primary constructor and then set
literal16_display_value_/has_literal16_display_, ensuring only one validation switch per file. - Update generator-gating tests and generator output to match the new emitted structure and verify the “single switch” invariant.
Reviewed changes
Copilot reviewed 2 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/operand.cpp | Replace linear validation chain with switch (opr_type); delegate literal16-display ctor to primary ctor. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/operand.cpp | Same generated validation switch + literal16 ctor delegation for RDNA3 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/operand.cpp | Same generated validation switch + literal16 ctor delegation for RDNA3.5 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna2/operand.cpp | Same generated validation switch + literal16 ctor delegation for RDNA2 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna1/operand.cpp | Same generated validation switch + literal16 ctor delegation for RDNA1 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/operand.cpp | Same generated validation switch + literal16 ctor delegation for CDNA5 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna4/operand.cpp | Same generated validation switch + literal16 ctor delegation for CDNA4 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna3/operand.cpp | Same generated validation switch + literal16 ctor delegation for CDNA3 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna2/operand.cpp | Same generated validation switch + literal16 ctor delegation for CDNA2 model. |
| emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna1/operand.cpp | Same generated validation switch + literal16 ctor delegation for CDNA1 model. |
| emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py | Update/extend assertions to match switch-based emission and verify only one validation switch per generated operand TU. |
| emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py | Change generator emission from linear predicate validation to switch-based cases; make literal16 ctor delegate to primary ctor. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
atgutier
approved these changes
Aug 19, 2026
atgutier
pushed a commit
that referenced
this pull request
Aug 19, 2026
Generated operand constructors validated each encoding by walking every operand-type predicate. Operand construction is the largest decoder hot-path cost, and common operand types appeared late in that linear chain. Emit a switch over OperandType so construction evaluates only the matching selector ranges while preserving deferred diagnostics. Delegate the literal16 constructor to the primary constructor so each generated translation unit contains one validation switch, regenerate all ten AMDGPU models, and extend the generator profile gates for the shared structure. On the pinned large-object benchmark, median valid-decode throughput improves 25.1%, cycles fall 19.7%, and instructions fall 35.6% across nine alternating CPU-pinned pairs with identical corpus results and checksums. (cherry picked from commit f2e8786)
1 task done
atgutier
added a commit
that referenced
this pull request
Aug 20, 2026
…t calls (#10444) ## Motivation hipTensor and rocThrust gfx1250 objects are refused at code-object load time with "indirect branch or call target recovery is not implemented for relocated kernel text" on `s_swap_pc_i64`. A refused object leaves HIP holding an invalid kernel handle, so the failure surfaces as a segfault rather than a diagnostic. The fix is upstream as #10376; this backports it to `release/bkc/therock-10.1-20260811`. ## Commits Cherry-Picked | # | Upstream | PR | Backport | Subject | |---|----------|-----|----------|---------| | 1 | `b6d838028e` | #10197 | `846fb16d12` | [rocjitsu][docs] Refresh architecture support documentation | | 2 | `5c8e33e391` | #10200 | `db5c58b1be` | [rocjitsu] Define standalone decoder ownership | | 3 | `be1f91998f` | #10199 | `ed102baf1c` | [rocjitsu] Apply async IOFFSET to LDS addresses | | 4 | `fda635e5b4` | — | `ad32ff7c18` | [rocjitsu] Extract per-scope rollback and object-level checks from translate() | | 5 | `8f75eb781e` | — | `4c0cb05207` | [rocjitsu] Emit leading V_NOPs ahead of a gfx1250 MODE write | | 6 | `00aa1e260f` | #9941 | `ded43b1787` | fix(rocjitsu): disable zero-count TDM descriptors | | 7 | `c810647609` | #10192 | `50ca7f1732` | [rocjitsu] Apply MUBUF LDS-load instruction offset to the LDS address | | 8 | `1d7866269d` | #9560 | `2e6bf26943` | [rocjitsu] code rewrite condition validator | | 9 | `95b6c410bb` | #10175 | `8418901076` | [rocjitsu] Fix ds_read_b64_tr_b16 lane layout in gfx950 simulator | | 10 | `bfaebcfb5c` | #10240 | `c4eac1e1f7` | [rocjitsu][util] Add result and diagnostic utilities | | 11 | `dbb7221fa7` | #9775 | `cdd0993a5f` | fix(rocjitsu): route tensor DMA through wave VMID | | 12 | `2779247222` | #9779 | `7bfeba6f5d` | feat(rocjitsu): Lazy vgpr allocation system | | 13 | `a659172a31` | #9936 | `180cf00325` | fix(rocjitsu): honor TDM tensor bounds across iterations | | 14 | `60c29efb39` | #10204 | `370bfb9796` | [rocjitsu] Fix gfx950 decoder extension handling | | 15 | `c1dba564e7` | #10345 | `536110a083` | [rocjitsu] Model gfx1250 wave ABI state | | 16 | `7f71be9f32` | #10346 | `3f922ad5f2` | [rocjitsu] Model gfx1250 barrier synchronization | | 17 | `bea7ac8937` | #10368 | `7b5ecf6c7b` | [rocjitsu] Replace decoder exceptions with results | | 18 | `f2e878694e` | #10371 | `8cf750d23f` | [rocjitsu] Dispatch operand validation by type | | 19 | `dd619bed77` | #9735 | `1a612c47c0` | fix(rocjitsu): respect same-wave LDS ordering | | 20 | `c5fae8d2b2` | #10137 | `4360c4b978` | [rocjitsu] Rename gfx1250 to cdna5 for enums, configs, amdisa codegen helpers and generated symbols. (NFC) | | 21 | `b05a587f89` | #10274 | `bfb5f9fab0` | [rocjitsu] Align CDNA4 MI350X configs with captured topology | | 22 | `f81f9d40db` | #10362 | `8177b5725c` | feat(rocjitsu): cdna5 OOB behavior implementation | | 23 | `53a939bd00` | #10283 | `141b109432` | [rocjitsu] add per-XCD workgroup dispatch observability | | 24 | `a0fb11bc17` | #10414 | `6bb4c5a959` | [rocjitsu] Refactor test encoding handling in execution tests (NFC) | | 25 | `e5109e315e` | #9844 | `322ac26b44` | feat(emulation): debug emulated GPU kernels with real ROCgdb | | 26 | `efa24f23c9` | — | `c3c9641073` | rocjitsu: Point the XCD distribution test at the config that exists | | 27 | `8b7763ef5e` | #10376 | `c3f419555a` | [rocjitsu] Translate gfx1250 lane-banked and kernarg-supplied indirect calls | ## Issue Tracking [ROCM-29436] ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. [ROCM-29346]: https://amd-hub.atlassian.net/browse/ROCM-29346?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [ROCM-29436]: https://amd-hub.atlassian.net/browse/ROCM-29436?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com> Signed-off-by: hanhanW <hanhan0912@gmail.com> Co-authored-by: Jakub Kuderski <jakub@nod-labs.com> Co-authored-by: James Newling <james.newling@gmail.com> Co-authored-by: Fei Kou <feikou@meta.com> Co-authored-by: Austin Lu <austin.lu@amd.com> Co-authored-by: Fei Kou <feikou@anthropic.com> Co-authored-by: Erick Ochoa Lopez <eochoalo@amd.com> Co-authored-by: Han-Chung Wang <hanhan0912@gmail.com> Co-authored-by: Rosa, Ada <arosa@amd.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
cmcknigh
pushed a commit
that referenced
this pull request
Aug 25, 2026
Generated operand constructors validated each encoding by walking every operand-type predicate. Operand construction is the largest decoder hot-path cost, and common operand types appeared late in that linear chain. Emit a switch over OperandType so construction evaluates only the matching selector ranges while preserving deferred diagnostics. Delegate the literal16 constructor to the primary constructor so each generated translation unit contains one validation switch, regenerate all ten AMDGPU models, and extend the generator profile gates for the shared structure. On the pinned large-object benchmark, median valid-decode throughput improves 25.1%, cycles fall 19.7%, and instructions fall 35.6% across nine alternating CPU-pinned pairs with identical corpus results and checksums.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generated operand constructors validated each encoding by walking every operand-type predicate. Operand construction is the largest decoder hot-path cost, and common operand types appeared late in that linear chain.
Emit a switch over OperandType so construction evaluates only the matching selector ranges while preserving deferred diagnostics. Delegate the literal16 constructor to the primary constructor so each generated translation unit contains one validation switch, regenerate all ten AMDGPU models, and extend the generator profile gates for the shared structure.
On the pinned large-object benchmark, median valid-decode throughput improves 25.1%, cycles fall 19.7%, and instructions fall 35.6% across nine alternating CPU-pinned pairs with identical corpus results and checksums.