Skip to content

[rocjitsu] gfx1250 B0 -> A0 translation - #8947

Merged
atgutier merged 6 commits into
developfrom
users/Groverkss/gfx1250-b0-to-a0
Jul 22, 2026
Merged

atgutier merged 6 commits into
developfrom
users/Groverkss/gfx1250-b0-to-a0

Conversation

@Groverkss

@Groverkss Groverkss commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Because A0 and B0 share one ELF machine ID and the gfx1250 target triple, a compiler-produced code object is byte-identical for both steppings — the binary carries no signal that distinguishes them. To run B0-targeted code on A0 silicon, rocjitsu needs a same-ISA DBT path that rewrites the affected instructions in place while leaving everything else untouched.

This PR adds that path: a gfx1250 B0 → A0 legalization/expansion stage in the DBT pipeline, the supporting analyses it needs (VGPR-MSB bank tracking, indirect-branch/lane-stash recovery, relocation function tables), and the plumbing to select the silicon revision out of band (since it can't be inferred from the code object). Every workaround whose hardware behavior is not yet proven fails closed rather than silently emitting a form that could miscompile.

Technical Details

Instruction legalization + semantic expansion (code/dbt/{legalization,semantic}/gfx1250_b0_to_a0.*)

A two-stage design: a classifier (requires_errata_expansion) decides which mnemonics need attention, and a semantic rule table performs the rewrite. Handled families:

  • Cluster loads — kept as cluster loads but wrapped to execute with M0 = 0 (save M0 → set M0=0 → load → restore M0). gfx1250 encodes M0 = 125, NULL = 124 (inverse of
  • Fail-closed (no proven lowering yet): bare low-precision K=128 FP8/BF8 WMMA, standalone 32x16 FP4 WMMA, and the deferred barrier/sleep family. These reject the whole kernel rather than emit a form the legalizer itself would refuse.

gfx1250 VGPR-MSB analysis (analysis/gfx1250_vgpr_msb.*)

gfx1250 vector operands encode only the low 8 bits of a VGPR index; s_set_vgpr_msb supplies the high 2 bits per role. A dedicated analysis proves the per-role bank at each program point so expansions that must name a physical VGPR (or reject when the bank is unknown) are correct across control flow.

Indirect-branch / lane-stash recovery (analysis/indirect_branch_discovery.*)

Recovers statically-built setpc/swappc targets (including the RCCL idiom that stashes a getpc-built PC into VGPR lanes via writelane/readlane) so relocation can rewrite the builder in place. Recovery is block-local and fails closed: state resets at every basic-block leader (branch targets included), so a stash reached on only one path — or jumped over — cannot seed a consumer it never dynamically reaches. Wide (≥64-bit) writes and MSB-bank changes invalidate stashed lanes; incomplete facts propagate monotonically and disqualify a consumer from a direct transfer window.

Relocation function tables (code/relocation_function_table.*)

Support for rewriting getpc-relative address-builder ranges once final target offsets are known, enabling correct relocation of recovered indirect edges.

Revision plumbing (config-driven, not ABI-driven)

  • simulation_config.fbs: new DbtSiliconRevision enum + guest_revision/host_revision on DbtGuestConfig (appended, default unspecified, backward-compatible).
  • DbtGuestConfig + HSA hook map the config revision into BinaryTranslatorOptions; a same-target gfx1250 load now translates when both revisions are set and fails closed otherwise.
  • rj_code.h keeps rj_code_dbt_options_t at its original two fields; the C entry point rejects same-arch gfx1250.

amdisa codegen

Parser/generator support for implied-literal encodings (extension DWORDs appended to a parent form), regenerated into the gfx1250 encoding tables.

Issue Tracking

#8939

Test Plan

Run CI tests.

  • Unit: translate_test.cpp (per-family expansion + fail-closed coverage, operand-pinned assertions for M0/byte-select lane-mask/ADDTID formula), liveness_test.cpp (VGPR-MSB banks, CFG-local lane recovery incl. a branch-into-consumer diamond regression), relocation_function_table_test.cpp, config_test.cpp (revision round-trip), rj_dbt_translate_smoke_test.cpp (CLI flags).
  • HIP dispatch: relocation_function_table_hip_test.cpp + device kernel.
  • Full Release serial suite: 1902/1902 pass (1 intentionally skipped).
  • ASan/UBSan clean — this PR also fixes the enum-load UB that failed the asan-ubsan CI job (removed with the C-API enum).

Test Result

All passed

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
🌿 Branch Name ✅ Pass —
📝 PR Title/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.

@therock-pr-bot

therock-pr-bot Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

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

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

A few comments

@atgutier
atgutier force-pushed the users/Groverkss/gfx1250-b0-to-a0 branch from fd167e4 to dc765f3 Compare July 22, 2026 01:25
@atgutier
atgutier force-pushed the users/Groverkss/gfx1250-b0-to-a0 branch from 242f25a to 09e509a Compare July 22, 2026 14:51
Groverkss and others added 5 commits July 22, 2026 08:19
Semantic rules (code/dbt/semantic/gfx1250_b0_to_a0.cpp):
- gfx1250 encodes M0 as 125 and NULL as 124 (the inverse of CDNA). Two
  hand-written rules used 124 where they meant M0, so the DS ADDTID address
  read its base from NULL (0) and the cluster-load M0 sequence read/wrote a
  discarded operand. Add named kGfx1250M0 / kGfx1250Null constants and route
  every M0 reference through them.
- Cluster loads (all SADDR and off-form widths) now keep their opcode and run
  with M0 = 0 (save to a dead SGPR, set 0, run, restore); the previous
  off-form -> global_load rewrite is removed. Replace
  gfx1250_cluster_load_replacement() with the is_gfx1250_cluster_load()
  predicate.

Analysis (analysis/):
- indirect_branch_discovery.cpp: the signed-delta prefetch-padding skip now
  rejects an s_mov that writes the temporary it depends on. Lane-stash PC
  recovery is made block-local: the lane-slot map is cleared at every block
  terminator and on any S_SET_VGPR_MSB / MODE write.
- gfx1250_vgpr_msb.cpp: read the S_SETREG_IMM32_B32 immediate from the .text
  image at src_loc()+4 instead of raw_encoding()[1] (which indexed past the
  4-byte SOPK encoding). Thread a text span through LivenessAnalysisOptions.
  Extract kModeHwreg + decode_hwreg() and reuse it across the HWREG-decode
  sites.
- def_use_chain.cpp: expand an unknown-bank VGPR operand to all four banks for
  uses only; a def records nothing (with an assert documenting the invariant)
  so it cannot falsely kill tuples it does not write.

Translation robustness (code/):
- code_object_patcher.cpp: a RELATIVE64 function-pointer whose source block was
  placed differently across scopes now fails the patch instead of collapsing to
  one placement.
- binary_translator.cpp: on kernel skip, the text and PC-relative-data
  relocation vectors are truncated back to their pre-scope sizes along with the
  translated text.
- relocation_function_table.cpp: the dispatch dataflow worklist also seeds
  blocks[0] so a kernel whose entry has an in-edge is still analyzed.

Cleanup:
- Consolidate duplicate ELF reloc-info accessors onto elf_reloc_type/sym;
  remove the unused REGISTER_SET_ALLOCATABLE_VGPRS; document the
  classifier-only mnemonics that fail closed and fix the exact-mnemonic array
  size.

Tests:
- Operand-encoding assertions for the DS ADDTID and cluster-load rules;
  signed-delta temporary-clobber rejection; block-local lane-stash;
  VGPR_MSB unreachable-predecessor exclusion; reloc-dispatch misaligned-slot
  and out-of-.text rejection; and a fail-closed check for
  s_barrier_signal_isfirst. Full Release suite passes (1891/1891).

Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>
…-closed coverage

Follow-up to the gfx1250 B0->A0 change addressing a second review pass. Fixes
several analysis/relocation correctness bugs (each with a regression test proven
to catch it), tightens a number of paths to fail closed, and closes API/test
gaps. Where a workaround's emitted form is not yet confirmed, the affected
instruction is classified to fail closed (or, for very common ops, passed
through with a warning) rather than emitting an unverified sequence.

Analysis correctness (analysis/):
- indirect_branch_discovery.cpp: append_unique() deduplicates on the full fixup
  identity (source_getpc_offset and recovery range included), so two distinct PC
  builders reaching one consumer are both retained instead of one being dropped
  with a stale pre-relocation address.
- indirect_branch_discovery.cpp: lane-stashed PC recovery keys slots by the
  physical VGPR (low selector plus the role's VGPR-MSB bank: destination bank for
  writelane, SRC0 bank for readlane) and fails closed when the bank is unknown, so
  constant differing banks no longer alias two physical registers.
- indirect_branch_discovery.cpp / binary_translator.cpp: a recovered consumer is
  replaced with a direct transfer window only when its fact is COMPLETE. An
  incomplete fact (one unconstrained predecessor) still records concrete targets
  for relocation/liveness but keeps the dynamic consumer, so an unconstrained path
  is not redirected. IndirectCallFixup carries a source_incomplete flag.
- def_use_chain.cpp: replace a tautological assert with a documented invariant for
  the unknown-bank VGPR def case (control only reaches it for a VGPR ref).

Relocation tables (relocation_function_table.cpp):
- Track only a single address add per builder; a second literal add fails closed
  rather than resolving to a value whose earlier addend still executes.
- Use modulo-2^64 addition for the table-base add so a table or GOT below .text
  (addressed with a two's-complement negative literal) resolves instead of being
  rejected as overflow.
- Seed the dispatch dataflow worklist with blocks[0] as well as zero-predecessor
  blocks, so a kernel whose entry has an in-edge is still analyzed.

gfx1250 B0->A0 legalization/semantics:
- semantic/gfx1250_b0_to_a0.cpp: emit s_wait_xcnt 0 before every generated
  s_set_vgpr_msb so a bank transition drains outstanding cross-lane/memory work.
- semantic/gfx1250_b0_to_a0.cpp: name the WMMA-scale prefix opcodes instead of
  hardcoding 0x35/0x3a.
- legalization/gfx1250_b0_to_a0.cpp: classify the previously unhandled bare
  low-precision WMMA/SWMMAC forms (v_wmma_f32_16x16x128_f8f6f4, the K=64 FP8/BF8
  WMMA family, and the FP8/BF8 SWMMAC family) so they fail closed instead of being
  copied through unchanged.
- legalization/gfx1250_b0_to_a0.cpp: inspect 64-bit source operands and fail
  closed when FLAT_SCRATCH_BASE_HI is used in a 64-bit source position (a 32-bit
  use is unaffected).
- legalization/gfx1250_b0_to_a0.cpp: pass the barrier-state and sleep/monitor
  families through unchanged with a warning (deferred), since fail-closing common
  ops such as s_sleep would over-reject; documented for follow-up.

Revision selection (rj_code_translate.cpp, rj_code.h, hooks):
- Add a revision-bearing rj_code_dbt_options_t (ABI-safe: revisions default to
  UNSPECIFIED) and fail closed when a gfx1250 side has no revision, or a
  non-gfx1250 side has one.
- HSA load hook fails closed for a same-target gfx1250 load when it has no silicon
  revision information, rather than silently passing the object through.

Tests:
- New/updated regression tests for each fix above, several verified to fail
  without the corresponding change: distinct-builder retention, chained-add and
  backward-address relocation, differing-bank lane stash, incomplete-fact
  flagging, VGPR-MSB truncated-literal and RELATIVE64 conflicting-placement
  fail-closed cases, the s_wait_xcnt drain, the bare-WMMA/SWMMAC and
  FLAT_SCRATCH_BASE_HI fail-closed cases, and the C-API revision handling. The
  e5m3 and Scale16 tests now assert value-defining constants/operands rather than
  only mnemonic counts.

Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>
…FG-safe recovery

Address expert-review BLOCKERs and NITs on the gfx1250 B0->A0 same-ISA
translation path. Every deferred/unproven lowering now fails closed, and the
indirect-branch and liveness analyses are corrected so unverified state can
never be silently trusted.

Semantic rules (semantic/gfx1250_b0_to_a0.cpp):
- expand_gfx1250_wmma_32x16_f4 and expand_gfx1250_k128_wmma now fail closed
  instead of splitting. Both split paths emitted bare low-precision WMMA halves
  that are exactly the standalone forms the legalizer rejects on input; the safe
  regular-Scale-prefixed lowering is not yet implemented, so the whole
  instruction is rejected rather than emitting a form needing re-legalization.

C API (rj_code.h, rj_code_translate.cpp):
- to_processor_revision returns std::optional and maps only the known closed set;
  an unknown/garbage/future revision value is now rejected up front instead of
  being silently coerced to Unspecified (which would identity-copy and skip every
  gfx1250 workaround). rj_code_translate validates both mapped revisions before
  any policy decision.
- rj_code_dbt_options_t documented as NOT ABI-stable while DBT is in development
  (callers must recompile); removed the incorrect zero-init/older-two-field claim.

Binary translator (binary_translator.cpp):
- Same-arch gfx1250 translation now also requires both silicon revisions and a
  supported direction inside translate(), so a direct/internal caller cannot
  bypass the C-API check and get a silent identity copy.
- An indirect-branch consumer with any incomplete recovered fact now fails closed
  for the whole consumer: an unconstrained predecessor path may carry an original
  .text address that DBT relocation would leave dangling, and cannot be proven
  safe, so it is no longer eligible for a direct transfer window.

Indirect-branch discovery (indirect_branch_discovery.cpp):
- append_unique OR-merges source_incomplete on duplicate fixups so a later
  fixed-point pass rediscovering incompleteness is not dropped.
- VGPR-MSB bank is marked UNKNOWN at every block boundary: the scan is lexical,
  not CFG-ordered, so a later block must re-establish its bank locally rather than
  inherit a lexical predecessor's; lane transport otherwise fails closed.
- Non-writelane defs now invalidate stashed lane slots via the full RegisterSet of
  destinations (any width) plus implicit defs, so a 64-bit-or-wider write
  overlapping a stashed lane clears it.

Liveness (liveness.cpp):
- Document that the VGPR/ACC_VGPR kill suppression in kill_defs is load-bearing
  for the gfx1250 unknown-bank VGPR-def handling in def_use_chain.cpp.

Tests (tests/dbt/translate_test.cpp, tests/analysis/liveness_test.cpp):
- New fail-closed coverage for standalone 32x16 FP4, every K=128 fp8/bf8 WMMA,
  unknown C-API revision values, same-arch unspecified revision, and the
  incomplete indirect consumer; removed the six obsolete K128-split detail tests.
- New CFG-analysis tests: bank is not inherited from a lexical predecessor, and a
  wide VGPR write invalidates a stashed lane slot.
- Pin value-defining constants so operand miscompiles cannot pass: the E5M3
  byte-select (opsel=2 -> byte 1) and the Scale16 low/high lane-mask literals
  (0x0000ffff/0xffff0000).

Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>
@atgutier
atgutier force-pushed the users/Groverkss/gfx1250-b0-to-a0 branch from 09e509a to 39d94e2 Compare July 22, 2026 15:24
@Groverkss
Groverkss marked this pull request as ready for review July 22, 2026 15:25
Copilot AI review requested due to automatic review settings July 22, 2026 15:25
@Groverkss
Groverkss requested review from a team and atgutier as code owners July 22, 2026 15:25

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.

Pull request overview

This PR extends rocjitsu’s DBT pipeline to support gfx1250 B0 → A0 translation policy (where A0/B0 share the same ELF machine ID) and adds infrastructure to safely translate/relocate code objects that contain relocation-backed device-function dispatch tables and gfx1250-specific register semantics.

Changes:

  • Add explicit input/output silicon revision plumbing (CLI + C API + translator) to select gfx1250 B0→A0 errata behavior and fail closed for ambiguous cases.
  • Add discovery/modeling of relocation-backed device-function tables, including patcher support to retarget RELATIVE64 addends through exact .text offset maps.
  • Improve gfx1250 correctness in CFG/def-use/liveness via VGPR_MSB tracking, additional indirect-branch recovery patterns, and implied-literal sizing fixes.

Reviewed changes

Copilot reviewed 55 out of 56 changed files in this pull request and generated no comments.

Show a summary per file
File Description
emulation/rocjitsu/tools/dbt_translate.h Add revision fields to tool API and reporting.
emulation/rocjitsu/tools/dbt_translate.cpp Validate gfx1250 revision requirements; pass revisions into translator.
emulation/rocjitsu/tools/dbt_translate_main.cpp Add gfx1250 target + CLI flags for input/output revisions.
emulation/rocjitsu/tests/tools/rj_dbt_translate_smoke_test.cpp Add CLI validation tests for revision flag behavior.
emulation/rocjitsu/tests/patch/instruction_builder_test.cpp Add gfx1250 opcode expectation coverage for s_call.
emulation/rocjitsu/tests/kernels/relocation_function_table_dispatch.hip New HIP fixture emitting a relocation-backed function table.
emulation/rocjitsu/tests/kernels/CMakeLists.txt Probe toolchain for gfx1250 support; conditionally build/install fixture.
emulation/rocjitsu/tests/gfx1250_sim_test.cpp Add decode coverage for implied-literal sizing case.
emulation/rocjitsu/tests/dbt/relocation_function_table_hip_test.cpp New end-to-end DBT test over HIP-linked relocation table dispatch.
emulation/rocjitsu/tests/code/relocation_function_table_test.cpp New unit tests for table discovery/dispatch identification/patching.
emulation/rocjitsu/tests/CMakeLists.txt Wire new relocation-function-table tests and conditional HIP test.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/isa_traits.h Treat gfx1250 descriptor/limits distinctly from generic RDNA.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/gfx1250/encodings.h Declare variable implied-literal sizing helper.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/gfx1250/encodings.cpp Fix VOP2 size accounting for variable implied literals.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/hooks/rj_hsa_dbt_hooks.cpp Fail closed on same-target gfx1250 loads without revision info.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/relocation_function_table.h New API for discovering relocation-backed call tables/dispatches.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/relocation_function_table.cpp Implement table discovery and dispatch identification analysis.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/patch/kernel_text_layout.h Rename/clarify kernarg-preload compatibility + skipped-kernel stub semantics.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/patch/kernel_text_layout.cpp Adjust skipped-kernel stub policy; refine branch window heuristics; fix conditional inversion logic.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/patch/instruction_builder.h Add gfx1250 s_call opcode mapping; clarify trap doc.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/patch/instruction_builder.cpp Add gfx1250 s_add_nc_u64 opcode support and builder emission logic.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/patch/code_object_patcher.h Add .text offset map + PC-relative data relocation support; rename relocation predicate.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/patch/code_object_patcher.cpp Implement symbol/addend relocation via offset maps; support RELATIVE64 addend retargeting.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/translation_diagnostic.h Update “dispatchable” docs for new skipped-kernel stub behavior.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/semantic/rules.h Declare gfx1250 B0→A0 semantic expansion rule table.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/semantic_translator.h Thread revisions into semantic translator selection.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/semantic_translator.cpp Select gfx1250 errata rules only for explicit B0→A0 direction.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/rj_code_translate.cpp Add C API revision validation and gfx1250 fail-closed policy.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/processor_revision.h New internal revision enum for DBT policy selection.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/legalization/gfx1250_b0_to_a0.h New legalization classifier API for gfx1250 errata candidates.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/legalization/gfx1250_b0_to_a0.cpp Implement B0→A0 errata classification and deferred-family warnings.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/kernel_descriptor_translator.h Clarify kernarg-preload compatibility semantics and skipped-kernel wording.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/kernel_descriptor_translator.cpp Add gfx1250 ISA traits; tighten wave-size/preload handling for gfx1250.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/binary_translator.h Add revision options + legalization lookup hook; update docs.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/CMakeLists.txt Build new relocation table + gfx1250 legalization/semantic sources.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/basic_block.h Add undecodable-fallthrough tracking and public call-edge injection.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/basic_block.cpp Handle gfx1250 padding holes; ensure decode gaps become CFG boundaries.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/amdgpu_elf.h Add gfx1250 mach mapping + relocation type constants.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/liveness.h Add gfx1250-aware analysis options and VGPR_MSB query plumbing.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/liveness.cpp Integrate gfx1250 VGPR_MSB analysis into def/use-based liveness.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/indirect_branch_discovery.h Add “incomplete” recovery flag to prevent unsafe direct-window rewriting.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/indirect_branch_discovery.cpp Add gfx1250 patterns (add_nc_u64, lane-stashed PCs) and perf improvements.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/gfx1250_vgpr_msb.h New CFG analysis interface for VGPR_MSB bank state.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/gfx1250_vgpr_msb.cpp Implement MODE/s_set_vgpr_msb propagation and joins.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/def_use_chain.h Allow def/use expansion using VGPR_MSB analysis results.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/def_use_chain.cpp Expand gfx1250 VGPR refs by bank for uses; avoid unsound “must-def” expansion.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/analysis/CMakeLists.txt Build new gfx1250 VGPR_MSB analysis unit.
emulation/rocjitsu/lib/rocjitsu/include/rocjitsu/code/rj_code.h Add public C revision enum and DBT option fields (non-ABI-stable note).
emulation/rocjitsu/lib/python/amdisa/tests/test_literal_codegen.py Add tests for variable implied-literal sizing metadata.
emulation/rocjitsu/lib/python/amdisa/parser.py Track implied-literal extension size per opcode.
emulation/rocjitsu/lib/python/amdisa/gpuisa.py Change implied-literal metadata to opcode→wordcount mapping.
emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py Emit impliedLiteralWordCount() and handle variable implied-literal sizes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

A few comments and one reply here: #8947 (comment)

@Groverkss

Copy link
Copy Markdown
Contributor Author

I know it's my PR but Tony's changes LGTM, I left some comments earlier.

…-local recovery, UBSan fix

Address the latest PR review round: move the silicon revision off the C ABI and
into the DBT guest configuration, make lane-stash recovery truly block-local, fix
an UndefinedBehaviorSanitizer finding, and refresh stale comments.

Silicon revision moves from the C ABI to the guest config:
- rj_code.h: delete rj_code_revision_e and restore rj_code_dbt_options_t to its
  original two fields (guest_arch, host_arch). gfx1250 A0 and B0 share an ELF
  machine ID, so the revision cannot be inferred from the code object; carrying it
  through this C ABI added an out-of-bounds-read hazard for callers built against
  the old layout. rj_code_translate reverts to the two-arg BinaryTranslator; a
  same-architecture gfx1250 translation is direction-ambiguous through the C entry
  point and fails closed via BinaryTranslator's internal guard.
- simulation_config.fbs: add a DbtSiliconRevision enum and guest_revision/
  host_revision fields to DbtGuestConfig (appended, default unspecified, so
  existing configs stay valid).
- DbtGuestConfig (dbt_guest_config.h/.cpp): carry the two revisions and convert
  them from the generated FlatBuffers enum.
- HSA hook (rj_hsa_dbt_hooks.cpp): map the config revisions into
  BinaryTranslatorOptions. A same-target gfx1250 load now proceeds through
  translation when both revisions are configured (selecting the B0->A0
  workarounds) and still fails closed when they are not, instead of always failing
  closed. The rj_dbt_translate CLI keeps its own --input/output-revision flags;
  both paths drive the internal ProcessorRevision, which is unchanged.

Lane-stash recovery is made block-local (indirect_branch_discovery.cpp):
- Extract the block-leader computation into compute_block_leaders() shared by the
  CFG skeleton and the lane-stash scan, then reset all recovery state (builders,
  lane slots, reconstructed halves, VGPR-MSB bank) at every block LEADER rather
  than only after a terminator. A conditional branch can jump directly into a
  readlane/swappc block whose lexical predecessor falls through with the stash, so
  a leader that is a branch target must start from a clean state or it would
  recover a target for a value that never reaches the call on that path.

UBSan fix (this also fixed the failing asan-ubsan CI job):
- The removal of rj_code_revision_e eliminates the enum-load UB that
  -fsanitize=enum flagged when an out-of-range revision value was read.

Comment refresh:
- legalization/gfx1250_b0_to_a0.cpp: cluster loads are kept and run with M0=0 (not
  demoted); K=128 FP8/BF8 and standalone 32x16 FP4 now fail closed (not split).
- semantic/gfx1250_b0_to_a0.cpp: the expand_gfx1250_k128_wmma doxygen describes the
  fail-closed behavior instead of the removed two-instruction split.
- docs/rocjitsu_dbt_guest.md: document the guest_revision/host_revision fields.

Tests:
- translate_test.cpp: the C-API tests become RejectsSameArchGfx1250 (no revision
  field) and the B0->A0 success case moves to a BinaryTranslator-level test.
- liveness_test.cpp: add Gfx1250DoesNotReuseStashFromSkippedFallthroughPredecessor,
  a branch-into-consumer/diamond regression asserting discover_indirect_branch_edges
  emits no fixup; verified to fail before the leader-reset fix.
- config_test.cpp: assert revisions default to unspecified and round-trip
  gfx1250_b0/gfx1250_a0 through the config loader.

Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jul 22, 2026
@atgutier
atgutier merged commit 5b4ba01 into develop Jul 22, 2026
26 of 28 checks passed
@atgutier
atgutier deleted the users/Groverkss/gfx1250-b0-to-a0 branch July 22, 2026 18:04
kuhar added a commit that referenced this pull request Jul 23, 2026
## Motivation

The gfx1250 B0-to-A0 translation added in #8947 is consumed at load time,
but the existing `librocjitsu_hooks.so` image also carries every AMDGPU ISA
model, simulator execution semantics, the VM, and the HSA tools integration.
That makes the stripped hooks library 50,447,272 bytes and gives a fixed
gfx1250 translator a much broader dependency surface than it needs.

This PR adds a dedicated Linux shared library for that fixed translation
profile. It uses the model-only ISA layering from #8933, links only the
generic DBT base and the gfx1250 model, and produces a stripped image below
5 MB.

## Technical Details

- Add Linux-only `librocjitsu_gfx1250_b0_to_a0.so`, composed from
  `rocjitsu_dbt_internal`, `rocjitsu_isa_gfx1250_model`, and the header-only
  SimDojo interface. It does not link the simulator implementation, VM, ISA
  registry, or non-gfx1250 decoders.
- Add a revision-fixed C API that accepts a standalone AMDGPU ELF, always
  translates gfx1250 B0 to gfx1250 A0, and returns caller-owned output storage.
- Export only `rj_gfx1250_b0_to_a0_translate` and
  `rj_gfx1250_b0_to_a0_free`; hide archive internals with the existing hidden
  visibility policy plus a Linux version script.
- Install the library and public header with the normal Linux RocJITsu
  artifacts.
- Add a real raw gfx1250 code-object fixture and end-to-end translation test.

This PR provides the narrow library and loader-facing ABI. It does not change
the separate COMGR HotSwap shim or selector; that integration must load this
SONAME and resolve the two exported RocJITsu entry points.

## Issue Tracking

Related: #8939

## Test Plan

- Build `rocjitsu_gfx1250_b0_to_a0` and its raw gfx1250 fixture.
- Run the model-only and fixed-profile library tests.
- Run the focused gfx1250 B0-to-A0 DBT regression suite.
- Compile the public header from a strict C11 client.
- Inspect the library's symbols and dynamic dependencies, then measure its
  stripped size.
- Run the RocJITsu pre-commit checks.

## Test Result

- Model-only and fixed-profile library tests: 4/4 passed.
- Focused gfx1250 B0-to-A0 DBT tests: 42/42 passed.
- Real raw gfx1250 code-object translation: passed.
- Strict C11 public-header client: passed.
- Manual binary audit: exactly two public exports; only standard C/C++ runtime
  shared-library dependencies; no simulator, VM, execution, registry, COMGR,
  HSA runtime, or non-gfx1250 ISA implementation symbols.
- Stripped library size: 4,522,664 bytes, below the 5,000,000-byte limit and
  11.15x smaller than the stripped 50,447,272-byte hooks library.
- Pre-commit checks: passed.

## Submission Checklist

- [x] Look over the contributing guidelines at
  https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation organization: ROCm project: rocjitsu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants