[rocjitsu] DBT changes to make Qwen bf16 models work by Groverkss · Pull Request #8406 · ROCm/rocm-systems

Groverkss · 2026-07-10T15:22:52Z

Motivation

rocjitsu emulates AMD GPUs by intercepting the KMD/driver and syscall/ioctl layer. To run a CDNA4 (gfx950) workload on a CDNA3 (gfx942) or RDNA host, the guest kernels' machine code must be translated at load time — a dynamic binary translation (DBT) problem. The concrete driver for this work is running Qwen bf16 models end-to-end, which exercises three things the translator did not previously handle correctly:

Instruction-level ISA gaps. Real transformer kernels (Triton/HIP GEMM, flash-attention, HipKittens matmul) emit CDNA4 opcodes with no 1:1 CDNA3/RDNA equivalent — 64-bit address arithmetic (v_lshl_add_u64), bf16 conversions, v_dot2, and permlane/swap forms — which must be lowered to multi-instruction target sequences that preserve exact semantics,
including carry, liveness, and lane layout.
LDS capacity mismatch. A CDNA4 kernel can request more LDS (static or dynamic) than a CDNA3/RDNA host physically provides. Rather than reject those kernels, we virtualize the overflow into a global-memory backing buffer addressed through a per-dispatch sidecar descriptor, so LDS-heavy kernels still run.
Correctness under the emulator's real threading and ELF-relocation model. The translator rewrites .text (compaction, expansion, per-kernel placement) and appends descriptors, so any relocation or symbol whose value it does not remap must be detected and rejected rather than silently miscompiled. The dispatch path runs across the doorbell/scanner thread,
ROCr intercept callbacks, and HSA client threads, so the hook state must be race- and lifetime-correct.

This PR makes those three areas work and hardens every failure path to fail closed (reject the load or abort with a diagnostic) instead of fail open (miscompile, or submit a host-faulting launch).

What's in the change

124 files, of which 35 are auto-generated ISA/DBT tables regenerated from the amdisa Python code-generation pipeline (no hand edits to generated code). The remaining changes are the DBT engine, the virtual-LDS mechanism, the HSA hook layer, the code-object patcher, and tests. New end-to-end kernel assets (virtual_lds_smoke.hip, HipKittens bf16 matmul.s, cvt_pk_bf16_f32.hip) exercise the real workloads.

Technical details

Semantic instruction lowering (code/dbt/semantic/)

Each unsupported CDNA4 instruction is lowered to a target sequence via per-ISA rule tables shared through cdna4_to_rdna_common. Notable lowerings:

v_lshl_add_u64 → shift + 64-bit carry-add. Computes D = (S0 << S1[5:0]) + S2 as v_lshlrev_b64 followed by v_add_co_u32/v_add_co_ci_u32. The carry destination is a dead SGPR pair obtained from the liveness analysis (find_free_sgpr_pair + monotonic require_sgprs), never VCC — VCC is not liveness-tracked and the source instruction defines no carry, so clobbering it would be a silent correctness bug. When the destination aliases the VGPR addend the shift lands in a scratch VGPR pair so the addend survives to the add. Non-register addends (inline constants, literals, and v255 — whose high half selector 512 cannot encode a VGPR) are rejected rather than miscompiled.
bf16 / v_dot2 / cvt paths and DPP/DPP8 on read/write-output swaps. The generator's DPP preamble assumed src0 at operand index 0; for the swap classes (vector_swap, vector_permlane16/32_swap) index 0 is the read/write output, so applying DPP there would shuffle vdst. The generator now emits throw UnimplementedInst for a DPP/DPP8 source on those classes across all affected ISAs (regenerated), rather than producing wrong code.

Virtual LDS via sidecar descriptors (code/dbt/virtual_lds*, code/patch/sidecar_metadata*, hooks/virtual_lds*)

When a kernel's requested LDS exceeds the host limit, the translator emits a sidecar descriptor variant that redirects LDS traffic to a dense global backing buffer, plus a kernarg wrapper carrying a runtime-state block (backing pointer + per-dimension strides). At dispatch, the hook layer plans the rewrite (plan_virtual_lds_dispatch: KeepNormal / UseSidecar /
Reject), allocates the backing/kernarg buffers, writes the wrapper, and republishes the AQL packet on the sidecar descriptor under a transient INVALID header. Metadata is a versioned wire format (RJVLDS1/RJSIDE1) with bounds-checked parsing.

Fail-closed dispatch state machine. A nullopt from prepare_virtual_lds_dispatch now means strictly "the normal descriptor is safe" (no plan and packet fits host LDS, or below threshold). Any case where the normal descriptor would be oversized — an unregistered kernel whose group segment exceeds the host limit, or any preparation failure after the planner selects a sidecar (geometry reject, missing runtime-state ABI, null kernarg, wrapper-layout mismatch, backing/kernarg
OOM, wrapper-write failure) — calls a [[noreturn]] abort_virtual_lds_dispatch with a diagnostic. Falling back to the
oversized normal descriptor would fault the GPU, and a persistent INVALID header would stall the queue forever; abort is strictly ordered before any INVALID publish so no slot is left half-written.

ELF / code-object patching safety (code/patch/code_object_patcher.*, code/amdgpu_elf.h)

Because DBT moves .text without remapping symbol st_value or relocation addends, the patcher fails the translation closed when it sees anything it cannot relocate:

In-.text relocation places (has_relocations_within_text) — handles both ET_DYN (r_offset = vaddr) and ET_REL (r_offset section-relative via sh_info).
Relocations resolving into .text (has_relocation_to_text_symbol) — rejects any symbol defined in .text regardless of type (STT_FUNC, STT_NOTYPE labels, STT_SECTION+addend), keyed on st_shndx; and rejects a symbol-less R_AMDGPU_RELATIVE64 whose r_addend lands in the source .text interval (the loader forms its value from load bias + addend, which DBT does not remap). The reloc-type constant is pinned to the AMDGPU ELF ABI value (13) with a static_assert in the tests so it cannot drift.

HSA hook thread-safety (hooks/rj_hsa_dbt_hooks.cpp,hooks/sidecar_registry.*)

Scanner jthread lifecycle: on teardown the thread is moved to a local under the lifecycle mutex and joined with no lock held, avoiding the self-deadlock (the scanner body takes the queue mutex).
HSA runtime calls made under the dispatch-queue mutex are the non-re-entrant saved originals (captured at OnLoad), documented as a load-bearing invariant so a future change cannot route them through the wrappers and deadlock.
Consistent cross-registry lock ordering (DispatchQueue → Runtime → Sidecar), copy-on-return lookups, and correct release/acquire ordering on the 64-bit AQL header via atomic_ref.

Supporting infrastructure

Liveness analysis extensions (restrict_live_before_to_instructions, VGPR/SGPR free-run search), kernarg extension layout, kernel text layout, spill manager, and the amdisa generator changes that produce the regenerated ISA tree.

Issue Tracking

#8406

Test Plan

Focused unit tests for every lowering and guard: v_lshl_add_u64 (carry-uses-SGPR-not-VCC, v255 reject, inline-constant reject, dest-aliasing scratch), text-symbol and RELATIVE64 relocation rejection (patcher + E2E, in-text vs out-of-text addend), sidecar metadata parse guards (version, truncation, oversized string/record-count, non-zero reserved).
E2E DBT translation of real Triton/HipKittens/flash-attention kernels.
Run CI tests

Test Result

All CI tests: PASSED

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

therock-pr-bot · 2026-07-10T15:25:27Z

✅ 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 · 2026-07-10T15:25:29Z

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

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

Groverkss · 2026-07-15T15:21:59Z

I need to regenerate gfx1250 files btw, but will do that once reviewed.

kuhar

A few comments

newling · 2026-07-15T21:34:37Z

It is easier to review PRs that have 1 fix/feature, rather than a single PR that gets an integration working. Can you at least in the PR description, include a summary of what fixes/features are added to get the integration test working?

Groverkss · 2026-07-16T15:04:06Z

It is easier to review PRs that have 1 fix/feature, rather than a single PR that gets an integration working. Can you at least in the PR description, include a summary of what fixes/features are added to get the integration test working?

I mentioned this offline, but it's really hard to split up this PR. This PR lets us run some pytorch models which is the minimum validation we need. Changes after this will be smaller, but it's almost impossible for me to split this up and catch bugs without having some real validation through a model. This sets the design for DBT and further changes will be modifications to the design, not a redesign.

I can include a summary of features in the summary.

amd-arosa

Overall looks pretty good to me.
But it breaks the build for multiple reasons, and that needs to be resolved.
Also Have a couple comments that I would like you to resolve.
In future this should have been like 3-6 prs to make reviewing easier.

atgutier

From the agent:

MAJOR — must fix (domain correctness / silent corruption)

BF16 narrowing turns NaN into Inf — cdna4_to_cdna3.cpp:157 (emit_cdna3_f32_to_bf16_rne, used by
v_cvt_pk_bf16_f32). The emitted sequence unconditionally applies the RNE rounding bias (bits +
0x7fff + ((bits>>16)&1)) >> 16 to every input including exp==0xFF. The authoritative guest reference
util::f32_to_bf16_rne (data_types.h:278) special-cases NaN/Inf (no bias-add; sets bit 16 to keep
NaN a NaN). So a NaN activation like 0x7F800001 becomes +Inf — a real value change, not the "~1ulp
drift" the PR accepts. Concrete, in-scope, fixable: add the exp==0xFF guard before the bias add. The
existing test can't catch it — its oracle copies the emulation's own truncated (NaN-incorrect)
formula and restricts to finite inputs.
v_permlane16_swap_b32: no correct oracle — the translation (cdna4_to_cdna3.cpp:625) swaps all
four 16-lane groups (0-15↔16-31 and 32-47↔48-63, per its own doc comment and public ISA knowledge),
but the guest simulator (cdna4/vop1.cpp:10956, vop3.cpp:2102) only swaps 0-15↔16-31, leaving lanes
32-63 untouched. The translation is very likely the hardware-faithful side (simulator is buggy), but
the disagreement means there's no correct in-tree oracle and any execute test would validate
against the wrong behavior. Reconcile — fix the simulator loop to cover all groups, then add a
wave64 execute test covering lanes 32-63. (Reviewer flagged it couldn't confirm 4-vs-2-group ground
truth without quoting the confidential ISA spec — please confirm before relying on either side.)
v_dot2_f32_bf16: simulator widens as f16, not bf16 — the guest execute_v_dot2_f32_bf16_vop3p
(execute_shared.h:10033) uses util::f16_to_f32 (5-bit exponent) but the instruction operates on bf16
(8-bit exponent); the translation correctly widens as bf16. Again the translation is right and the
simulator is the buggy oracle, so the dot2 translate-and-execute path is unvalidated for numerics
(the translate_test checks are structural-only). Fix the guest to use util::bf16_to_f32 and add an
execute-vs-guest test.
Non-atomic set-once g_runtime_api global — hooks/virtual_lds.cpp:23. A struct of 9 pointers
written by set_virtual_lds_runtime_api and read on every dispatch/allocate path across app threads +
the scanner jthread. Race-free only because it's set once in OnLoad before threads exist — an
undocumented, fragile invariant (any reload or moved call is an immediate data race). Fix: document

assert set-once, publish behind std::atomic<const T*> with release/acquire, and clear it in
uninstall().

Unbounded retired_buffers growth — rj_hsa_dbt_hooks.cpp:2318. Virtual-LDS dispatches whose
completion signal is never pending (borrowed handle 0 with no signal_create, or a signal already ≤0
at enqueue) move their backing+kernarg buffers to retired_buffers, never reclaimed until queue
destruction. A long-lived queue repeatedly dispatching such kernels leaks for the queue's life. Fix:
bound the list (flush by polling) or force an internal completion signal for every rewritten
dispatch.

The systemic finding (ties the translation MAJORs together)

The correctness tests for the new bf16/gfx950-only sequences are structural (opcode-sequence)
checks, and the one execute test deliberately uses finite inputs and validates against a helper that
copies the emulation's own NaN-incorrect formula. No new sequence has a translate-and-execute test
comparing against the true guest semantics on edge inputs (NaN/Inf/denormal/±0, full-wave lane
coverage for cross-lane ops), and two of the guest simulators that should be the oracle are
themselves buggy (permlane16, dot2). For a PR whose entire justification is numeric fidelity for
bf16 inference, this is the gap that matters most: the layer-by-layer "~1ulp drift" validation was
done externally, but the in-tree test suite can't catch a regression in any of these sequences.

MINOR (across subsystems)

kernel_text_layout.cpp (+799) and code_object_patcher.cpp have no dedicated unit tests — the
branch-relocation/long-branch/island-chain code most able to produce a wild jump is exercised only
indirectly, while the easier serialization helpers all got tests. Add a kernel_text_layout_test
(in-range/out-of-range branch resolution decoded and followed to target; kernarg-preload dual-stub
256-byte spacing; body-entry residue preservation).
Silent verbatim copy of untranslated encodings — binary_translator.cpp:1815: a gfx950-only opcode
with no rule and no legalization entry is copied through as-is (may be a valid-but-different CDNA3
instruction). The Expand-required and matched-but-failed paths fail loudly, but the "no entry at
all" case doesn't — and the VOP3 forms of the permlane swaps exist in the decoder but aren't in the
rule table, so they'd hit this path. Add the VOP3 rule entries + a gfx950-only deny-list forcing a
loud diagnostic.
Unguarded kernarg memcpy bound (kernarg_extension.cpp:141 — original_kernarg_size not checked
against wrapper_size before the copy; safe for in-tree callers, latent for a hand-built layout);
polling-scanner races the AQL producer on non-intercept queues (inherent to the fallback design,
documented, intercept path is primary); find_free_run doesn't assert power-of-two alignment
(liveness.cpp:283); no concurrency test for the (correctly-locked) registries under TSan; duplicated
scalar-add opcode constants (instruction_builder); g_runtime_api not cleared on unload; dot2 clamp
v_med3_f32 vs guest std::clamp differ on NaN.

NIT

VgprForbiddenRange hand-unrolled padding idiom (use std::span); manual kSrcLiteral64=254 magic
constant that should be generated; LivenessAnalysisOptions tri-state bool+span; SGPR-write-mask
silent clamp on wide refs; the launch-stub residue invariant documented only in code comments.

…2/permlane co13:44:46 [22/1985] Motivation ---------- Expert review and the PR 8406 comments surfaced correctness and safety gaps in the CDNA4->CDNA3 DBT path used for bf16 inference, plus a set of dispatch-time lifetime/race issues in the virtual-LDS HSA interposition layer. Several are silent-corruption bugs (a NaN turning into +Inf, a bf16 dot widened as f16, a cross-lane swap dropping half the wave) that the existing structural tests could not catch. This change fixes the root causes and adds execute-vs-guest coverage. Technical details ----------------- Virtual-LDS dispatch layer (hooks/rj_hsa_dbt_hooks.cpp, hooks/virtual_lds.*): - Clamp the ring-slot scanner's rescan window so it never mutates a slot the doorbell hook already published to the CP; document the residual-safety invariant on the no-intercept fallback path. - Sweep completed retired_buffers at intercept-batch entry so a queue that issues one virtual-LDS dispatch then only normal dispatches no longer leaks backing storage until queue destruction. - Propagate hsa_amd_queue_intercept_register status: on failure the queue is recorded as non-intercept so doorbell rewriting / polling still cover it. - Turn prepare_virtual_lds_dispatch into a tri-state: a hard-rejected plan now neutralizes the packet (header type -> INVALID via make_invalid_packet_header) on both the ring and intercept paths instead of dispatching an unusable packet. - Publish g_runtime_api behind std::atomic<const T*> with release/acquire. Required-sidecar descriptor stubbing (code/dbt/binary_translator.cpp, kernel_descriptor_translator.*): add static_lds_requires_sidecar so a normal descriptor whose static LDS exceeds the host limit (only launchable via its virtual sidecar) is redirected to the skipped-kernel trap when that sidecar fails, instead of remaining a dispatchable-but-faulting launch target. Numeric correctness: - BF16 narrowing (code/dbt/semantic/cdna4_to_cdna3.cpp): rewrite the hand-lowered f32->bf16 RNE sequence as a branchless, VCC-clobber-free path matching util::f32_to_bf16_rne — exp==0xFF inputs skip the rounding bias so a NaN stays a NaN rather than rounding up into Inf. Widen the scratch window to 5 VGPRs. - v_dot2_f32_bf16 (amdisa codegen: semantics.py, sema_derive.py, packed.py, simd_codegen.py; util/simd.h): give bf16 its own dot2 class so it widens via bf16_to_f32 (8-bit exponent) instead of f16_to_f32; add bf16_to_f32_simd and parameterize the SIMD dot helper by format. Regenerated across rdna4/gfx1250/cdna4. - v_permlane16/32_swap_b32 (amdisa codegen: vector_special.py): swap every 2*stride-lane block, not just the first, so a wave64 v_permlane16_swap exchanges all four 16-lane groups. Regenerated for cdna4/gfx1250. Metadata parser hardening (code/dbt/virtual_lds.*, code/patch/kernarg_extension.cpp): reject flag bits outside the known mask and non-power-of-two payload alignments. Codegen regeneration also reconciled the gfx1250 sopk/vop3 files that no longer compiled against the current generator. Tests: split the sidecar keep/stub cases; exact-word waitcnt assertions; negative virtual-LDS and sidecar metadata parse cases; a TSan-clean sidecar-registry concurrency stress test; NaN/Inf inputs for the cvt_pk_bf16 dispatch test; new execute-vs-guest tests for v_dot2_f32_bf16 (SIMD + scalar) and the wave64 4-group permlane16 swap; installed the four missing kernel fixtures and pointed ROCJITSU_HIPKITTENS_KERNEL_DIR at the install-tree kernel directory. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

… safety Motivation ---------- A re-review of the DBT branch plus an independent external review surfaced correctness and safety defects, several of which are silent-miscompile or queue-hang bugs the existing structural tests could not catch. Two external findings also invalidated an earlier fix on this branch (publishing INVALID to reject a dispatch actually stalls the command processor forever). This change fixes every finding at the root cause, adds regression coverage, and keeps the operand-model/codegen changes reproducible through the amdisa generator. Technical details ----------------- Semantic lowering (code/dbt/semantic): - cdna4_to_rdna_common.cpp: v_lshl_add_u64 -> RDNA no longer drops the shift. It now emits v_lshlrev_b64 followed by the 64-bit carry-add of S2. When the destination aliases the VGPR addend (the common (idx<<n)+base pattern, e.g. vector_add) it shifts into a liveness-allocated dead scratch pair so the addend survives; only a literal shift count or no-free-scratch case fails closed. - cdna4_to_rdna4.cpp: remove the dead second EXEC restore in the MFMA->WMMA lowering; document that the ds_bpermute tail intentionally runs under full EXEC. Binary translator / patch (code/dbt, code/patch): - binary_translator.cpp: fail loudly when a cross-arch opcode has no legalization entry or is marked Action::Illegal, instead of silently re-encoding it verbatim with the guest opcode. Replace the fragile positional cursor into live_before_instructions with a direct has_expand_rule() query. Fail closed when a code object has relocations whose place lies inside .text (they cannot be remapped after instruction relocation). - kernel_descriptor_translator.{h,cpp}: snapshot the 64 source descriptor bytes at translation time so sidecar descriptors are materialized from the snapshot, not from a descriptor_file_offset that .text growth has invalidated. - code_object_patcher.{h,cpp}: copy the sidecar template from the source-descriptor snapshot; add has_relocations_within_text() for the fail-closed guard above. - kernel_text_layout.cpp: document the near-conditional-branch long-branch window limitation (fail-closed, kernel skipped, not a miscompile). HSA hooks (hooks): - rj_hsa_dbt_hooks.cpp / virtual_lds.{h,cpp}: stop publishing HSA_PACKET_TYPE_INVALID to reject a virtual-LDS dispatch — the CP treats INVALID as "producer not done" and stalls the queue permanently. Instead validate load-time-determinable virtual-LDS invariants (runtime-state ABI, kernarg wrapper layout) at code-object load and fail the load; dispatch-time nullopt falls back to the normal descriptor (a required-oversized normal descriptor is already trap-stubbed at translation). Serialize the scanner jthread lifecycle with a dedicated mutex + stopping_ flag so clear() never joins under the data mutex or races record_queue(). Reset the virtual-LDS runtime-API table on unload. Fix the make_invalid_packet_header doc (INVALID is 1, not 0) and note the O(queues x slots) signal-destroy sweep. Codegen (lib/python/amdisa, generated ISA files): - _generator.py: s_trap is no longer marked PROGRAM_TERMINATOR (it returns to the following instruction / is a NOP with no handler configured), so the CFG keeps its reachable fallthrough; skipped-kernel stubs still halt via their trailing s_endpgm. Sub-dword read-write outputs are surfaced only through implicit_uses(), not appended to src_operands_ (which double-printed the destination in disassembly); read-write outputs are deferred to the end of src_operands_ only on DPP/SDWA-capable encodings, preserving architectural source order elsewhere (e.g. s_addk_i32/s_mulk_i32 keep sdst as src0). All ISAs regenerated. - isa_traits.h: name the descriptor SGPR-allocation limits. Tests: v_lshl_add_u64 shift + destination-aliasing coverage; relocation-targeting- .text rejection and has_relocations_within_text detection; sidecar reserved-byte preservation after .text growth; s_trap fallthrough + endpgm-terminates-CFG cases. Verification: full Release suite 1752/1752 passing. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

…guards Motivation ---------- A full re-review of the rebased branch (three domain reviewers plus an outside expert) found a shutdown deadlock, a silent-miscompile in the RDNA v_lshl_add_u64 lowering, and several places where unsupported or malformed inputs could be loaded/dispatched instead of rejected. This change fixes each at the root cause and adds fail-closed guards so the translator never resolves to a wrong address or lets a broken dispatch appear to succeed. Technical details ----------------- Hooks / concurrency (hooks/rj_hsa_dbt_hooks.cpp): - Scanner shutdown deadlock: clear() held scanner_lifecycle_mutex_ across scanner_.join(), but the scanner takes mutex_ and a concurrent record_queue() holds mutex_ while waiting on the lifecycle mutex — a three-way cycle. Move the jthread into a local under the lifecycle lock, then join with no lock held. - ensure_queue_packet_private_size() published a transient header of 0 (VENDOR_SPECIFIC, a processable type) while editing the packet body; use make_invalid_packet_header() so an in-flight slot reads as not-ready, matching the ring rewrite path. - Symbol iteration could bypass sidecar registration: rj_executable_iterate_agent_symbols now always routes through the name-recording callback (even when the agent mapping is identity), and the deprecated agentless hsa_executable_iterate_symbols is now wrapped and recorded too. Without this a kernel object obtained by iteration had no name association and its virtual-LDS sidecar could not be found, leaving an oversized dispatch on its normal descriptor. - Pass-through ("already target") load now parses and validates virtual-LDS metadata BEFORE calling the real loader, and rejects malformed metadata like the translated path (previously it validated after mutating executable state and silently ignored malformed metadata). - Translated load now fails when any kernel is skipped: a skipped kernel's s_trap; s_endpgm stub would fall through the (no-handler) s_trap and complete normally, so dispatching it would silently produce stale output. Refuse the code object rather than hand back an executable that can appear to succeed. DBT semantic lowering (code/dbt/semantic/cdna4_to_rdna_common.cpp): - v_lshl_add_u64 -> RDNA: the 64-bit addend S2 was split into halves via src2 and src2 + 1, which is only valid for register pairs. An inline-constant or literal addend (consecutive single encodings) silently corrupted the high word (e.g. addend 0 gained 1<<32). Reject non-register 64-bit addends (VGPR pair / even SGPR pair / VCC only). - The alias path (vdst overlapping the VGPR addend) allocates a dead scratch pair via liveness but never reported it, so the target descriptor could under- allocate VGPRs; call TranslationContext::require_vgprs(shift_dst + 2). Code-object patching (code/patch/code_object_patcher.{cpp,h}, code/dbt/ binary_translator.cpp): - has_relocations_within_text(): the ET_REL path compared a section-relative r_offset against a file offset, missing an .rela.text record with r_offset 0. For ET_REL, select relocation sections by sh_info == text and compare against [0, text size); keep the virtual-address check for ET_DYN. - Add has_relocation_to_text_function_symbol() and fail translation closed when a relocation references a function symbol defined in .text: DBT moves/duplicates .text blocks but does not remap text-defined STT_FUNC st_value, so such a relocation (e.g. a function-pointer table in .data) would resolve to the stale pre-move PC. Kernel entries are descriptor-dispatched and unaffected. Tests: reject-inline-constant-addend case for v_lshl_add_u64. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

…P-on-swap Address three external-review findings on the text-relocation branch. v_lshl_add_u64 lowering (cdna4_to_rdna_common.cpp): - Stop hardcoding VCC as the carry destination of the emitted v_add_co_u32 / v_add_co_ci_u32 pair. v_lshl_add_u64 defines no carry output and VCC is not liveness-tracked, so writing VCC silently clobbers a value the surrounding code may still depend on. Allocate a dead ordinary SGPR pair via find_free_sgpr_pair() and report it through require_sgprs(), matching the EXEC-save precedent in the MFMA lowering. - Cap the VGPR-pair addend selector at 510: v255 (511) has no valid high half (the derived src2+1 selector 512 cannot encode a VGPR), so reject it alongside the other non-register addend forms. Text-symbol relocation guard (code_object_patcher, binary_translator): - Broaden has_relocation_to_text_function_symbol() to reject a relocation resolving against ANY symbol defined in .text, not just STT_FUNC. DBT moves .text without remapping st_value, so STT_NOTYPE labels and an STT_SECTION(.text)+addend alias stale post-move offsets just as a function symbol does. Key the check on st_shndx and rename it to has_relocation_to_text_symbol(). DPP/DPP8 on read/write-output swaps (amdisa generator + regenerated ISAs): - The DPP preamble assumed src0 lives at src_operands_[0], but for the swap classes (vector_swap, vector_permlane16/32_swap) index 0 is the read/write output (vdst). apply_dpp() there would shuffle vdst instead of a source. Emit a throw UnimplementedInst for a DPP/DPP8 source on those classes rather than miscompiling. Regenerated all affected ISAs. Fix the stale test_generator_profile_gates assertion that expected a true16 destination to be reported as a source; sub-dword partial defs surface via implicit_uses(), not src_operands_. Add DBT translate tests: v255-addend rejection, carry-uses-scalar-SGPR (not VCC), and text-symbol relocation rejection across STT_FUNC / STT_NOTYPE / STT_OBJECT / STT_SECTION (text rejected, data accepted). Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

Address the outside-expert and internal re-review findings on the text-relocation branch. All changes are dispatch/patch fail-closed hardening plus test coverage. Virtual-LDS dispatch (rj_hsa_dbt_hooks.cpp): - prepare_virtual_lds_dispatch used one nullopt to mean both "keep the normal packet" and "hard failure after a sidecar was required". Both delivery paths (intercept batch and ring scan) treat nullopt as "submit the normal packet", so a kernel whose static LDS fits but whose dynamic (packet) group segment exceeds the host limit would submit its oversized normal packet -- a GPU fault -- when runtime sidecar allocation failed. Once the plan selects UseSidecar/Reject the request exceeds the host LDS limit, so the normal descriptor is unsafe for that dispatch regardless of translation-time stubbing. Every post-UseSidecar failure (geometry reject, missing runtime-state-block, null kernarg, wrapper-layout mismatch, backing/kernarg allocation, wrapper write) now calls a new [[noreturn]] abort_virtual_lds_dispatch with a diagnostic instead of falling back. nullopt now strictly means KeepNormal. Falling back to the normal descriptor or a persistent INVALID header (which stalls the queue) are both unacceptable, and there is no per-dispatch recovery. Text-symbol relocation guard (code_object_patcher.cpp, amdgpu_elf.h, binary_translator.cpp): - has_relocation_to_text_symbol() skipped symbol index 0, so a symbol-less R_AMDGPU_RELATIVE64 whose r_addend lands in source .text passed both guards (its place is in .data; its symbol is zero). DBT moves .text without remapping the addend, so the loader would publish a pointer to the stale source PC. Now decode the relocation type and reject a RELATIVE64 record whose addend falls in the source .text vaddr interval. Added elf_reloc_sym/elf_reloc_type helpers and R_AMDGPU_RELATIVE64. - Added an image_.size() >= sizeof(Elf64_Ehdr) guard before the Ehdr cast in both has_relocations_within_text() and has_relocation_to_text_symbol(). Hooks documentation (rj_hsa_dbt_hooks.cpp): - Documented that HSA runtime calls made under the dispatch-queue mutex_ are the non-re-entrant saved originals (routing them through the wrappers would deadlock), and that the intercept private-size path needs no INVALID header fence because it edits a private packet copy. Tests: - translate_test.cpp: RELATIVE64 addend into/out of .text (patcher + E2E). - sidecar_metadata_test.cpp: reserved!=0 and oversized record_count parse guards. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

… fix sign-compare Address re-review findings on the text-relocation branch. Relocation guard (amdgpu_elf.h, tests/dbt/translate_test.cpp): - R_AMDGPU_RELATIVE64 was defined as 10, but the AMDGPU ELF ABI value is 13 (10 is R_AMDGPU_REL32_LO). The symbol-zero RELATIVE64 text-addend guard in has_relocation_to_text_symbol() therefore never matched a real record, so the fail-open it was meant to close stayed open. The tests masked it by hardcoding the same wrong 10. Correct the constant to 13, have both tests consume the production constant, and add a static_assert pinning it to the ABI value so a future drift cannot silently re-mask the bug. Virtual-LDS dispatch (rj_hsa_dbt_hooks.cpp): - Close the remaining fail-open: a kernel with no virtual-LDS variant whose packet group segment exceeds the host LDS limit returned KeepNormal and submitted an oversized (host-faulting) launch. There is no sidecar to fall back to, so this now calls abort_virtual_lds_dispatch (kernel "<unregistered>") like the post-plan paths. Updated the prepare_ doc so nullopt strictly means "normal descriptor is safe". Build (cdna4_to_cdna3_virtual_lds.cpp): - Fix -Werror=sign-compare in append_cdna3_virtual_lds_entry_prologue: the uint16_t base_sgpr/prologue_temp_sgpr + 1 promotes to int and was compared against the uint32_t kCdnaOrdinarySgprLimit. Cast both to uint32_t. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

kuhar

A few comments

kuhar

Approving but it would be nice to address at least #8406 (comment) before landing

artifacts, gate abort to guest queue Address the CI asan-ubsan failure and the latest PR 8406 review comments. Test ELF descriptor alignment (tests/dbt/translate_test.cpp): - The test ELF builders placed .rodata (the kernel descriptor) at text_offset + text_size; an odd .text word count left that offset only 4-byte aligned, so reinterpret_cast<TestKernelDescriptor*> formed a pointer violating the type's 8-byte alignment. This tripped ~24 UBSan misaligned-access errors in the asan-ubsan CI jobs (Release was unaffected). 8-align .rodata (offset and vaddr together, preserving the PT_LOAD p_offset == p_vaddr (mod p_align) congruence) in the two variable-text descriptor builders, matching real AMDHSA code objects. Reject non-dispatchable skipped-kernel artifacts (translation_diagnostic.h, binary_translator.h, tools/dbt_translate_main.cpp, rj_hsa_dbt_hooks.cpp): - skip_failed_kernels reports a KernelSkipped *warning*, so ok() stayed true and rj_dbt_translate --output-mode code-object wrote the ELF and exited 0 even though the skipped kernel's s_trap; s_endpgm stub completes normally without a trap handler and would silently produce wrong results. Add a shared has_skipped_kernel() helper and a TranslatedCodeObject::dispatchable() accessor, gate the CLI code-object emit path on it, and refactor the HSA hook to use the shared helper. Gate the no-metadata oversized-LDS abort to the guest queue (rj_hsa_dbt_hooks.cpp): - rj_queue_create tracks every queue with host_lds_bytes derived from the guest target arch, and map() passes an unrelated agent through unchanged, so the unregistered-oversized fail-close could wrongly abort a legitimate launch on an unrelated agent with a larger real LDS capacity. Add QueueState.is_guest_queue (host_agent == the guest's execution host) and only abort on the guest queue; other agents' oversized packets are forwarded unchanged. Tests: - hsa_hooks_unit_test.cpp: unrelated-agent oversized packet forwarded unchanged on both the doorbell and intercept paths, plus a death test asserting the guest intercept queue aborts on an unregistered oversized dispatch. - translate_test.cpp: assert dispatchable() is false on a genuine skipped-kernel artifact. - kernarg_extension_test.cpp: negative-path coverage for the layout overflow guards, wrapper-write rejections, and parser bounds/version/reserved checks. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

…chable(), validate wrapper layout Fix the asan-ubsan-gcc CI build error and address re-review findings. GCC build error (tests/dbt/hsa_hooks_unit_test.cpp): - A braceless `if (layout)` followed by an EXPECT_EQ (which expands to an if/else) tripped -Werror=dangling-else under g++, failing the asan-ubsan-gcc job while asan-ubsan (clang) passed. Add braces. Forward non-guest memory-backed loads unchanged (rj_hsa_dbt_hooks.cpp): - rj_load_agent_code_object computed guest_load but did not gate on it, so a registered non-guest reader's load still went through source-ISA override, translation to the configured host, rocjitsu metadata validation, and registry mutation -- which could misdecode an unrelated GPU's native code object as the guest ISA and load a wrong artifact on that agent. Return original_load(...) immediately for !guest_load, before any reader lookup, target detection, translation, or registry change. Simplify the now always-guest downstream branches. Expose non-dispatchability through the tool API (tools/dbt_translate.h, tools/dbt_translate_main.cpp): - Add TranslateOutput::ok()/dispatchable() so a programmatic caller of translate_code_object() can gate on dispatchability instead of re-deriving has_skipped_kernel(); the CLI code-object emit guard now uses it. Close the rj_code_translate public-API fail-open (rj_code_translate.cpp): - The C API returned success on a non-empty ELF without checking ok() or dispatchable(), so a skipped-kernel (or error-diagnostic) artifact was handed back as success. Gate on dispatchable() (which implies ok()). Validate wrapper layout before copy (kernarg_extension.cpp): - write_kernarg_extension_wrapper never checked layout.original_kernarg_size against the wrapper size or saved-pointer offset. Since KernargExtensionLayout is public plain data, a forged layout (original_kernarg_size = UINT32_MAX) passed the guards and would memcpy ~4 GiB into a tiny wrapper. Require original_kernarg_size <= wrapper_size and <= original_kernarg_pointer_offset. Fix inverted architecture comments (rj_hsa_dbt_hooks.cpp, hsa_hooks_unit_test.cpp): - host_lds_bytes is derived from config->target.arch, which is the configured HOST target (gfx1201 -> 64 KiB), not the guest. Correct the comments that attributed it to the guest arch. Tests: - hsa_hooks_unit_test.cpp: non-guest loads (different-target image and malformed-metadata bytes) on an unrelated agent reach the original loader verbatim; the fake loader now records the forwarded agent/reader. - kernarg_extension_test.cpp: forged oversized-original-kernarg layout is rejected, plus a positive control asserting the baseline metadata parses. Signed-off-by: Tony Gutierrez <anthony.gutierrez@amd.com>

DBI for arbitrary probes requires being able to spill registers that are live at the anchor if the probe would clobber them. This PR enables spilling for VGPRs, SGPRs, EXEC, VCC, and M0. VGPRs are spilled directly into scratch. SGPRs are spilled into scratch using v_writelane/v_readlane into a bridge VGPR. EXEC, VCC, and M0 are stored into a dead SGPR. Relies on SpillManager changes made in #8406 and moves DBT's visit_kernel_descriptors functions to a shared location for kernel discovery. Moves instruction_builders into a new builders folder. * Multi-kernel and multi-.text code objects * RDNA2-3.5 (AmdGpuCodeObject doesn't support, trivial to add register spilling once support is added there) * CDNA1-2 (ABI convention is to use MUBUF instructions instead of FLAT to store, requires a lot of additional overhead) * AccVGPR * FLAT_SCRATCH-clobbering probes * EXEC/VCC spilling only when clobbered (relies on 100% accurate def/use tracking, can't guarantee now) * Unit tests for spill plans * Static tests that patch and decode the ELF file (similar to existing nop probe tests) * End-to-end simulator/behavior tests that execute a patched kernel All tests pass

## Motivation DBI for arbitrary probes requires being able to spill registers that are live at the anchor if the probe would clobber them. This PR enables spilling for VGPRs, SGPRs, EXEC, VCC, and M0. VGPRs are spilled directly into scratch. SGPRs are spilled into scratch using v_writelane/v_readlane into a bridge VGPR. EXEC, VCC, and M0 are stored into a dead SGPR. Relies on SpillManager changes made in #8406 and moves DBT's visit_kernel_descriptors functions to a shared location for kernel discovery. Moves instruction_builders into a new builders folder. Defers: - Multi-kernel and multi-.text code objects - RDNA2-3.5 (AmdGpuCodeObject doesn't support, trivial to add register spilling once support is added there) - CDNA1-2 (ABI convention is to use MUBUF instructions instead of FLAT to store, requires a lot of additional overhead) - AccVGPR - FLAT_SCRATCH-clobbering probes - EXEC/VCC spilling only when clobbered (relies on 100% accurate def/use tracking, can't guarantee now) ## Test Plan - Unit tests for spill plans - Static tests that patch and decode the ELF file (similar to existing nop probe tests) - End-to-end simulator/behavior tests that execute a patched kernel ## Test Result All tests pass ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

github-actions Bot added documentation Improvements or additions to documentation project: rocjitsu labels Jul 10, 2026

systems-assistant Bot added the organization: ROCm label Jul 10, 2026

therock-pr-bot Bot added the Not ready to Review label Jul 10, 2026

Groverkss force-pushed the users/Groverkss/text-relocation-land branch 2 times, most recently from 1714286 to 6768521 Compare July 15, 2026 14:03

Groverkss marked this pull request as ready for review July 15, 2026 14:07

Groverkss requested review from a team and atgutier as code owners July 15, 2026 14:07

Copilot AI review requested due to automatic review settings July 15, 2026 14:07

Copilot AI reviewed Jul 15, 2026

Groverkss requested a review from Copilot July 15, 2026 14:09

Copilot AI reviewed Jul 15, 2026

Groverkss force-pushed the users/Groverkss/text-relocation-land branch from 6768521 to 4e16603 Compare July 15, 2026 15:12

kuhar reviewed Jul 15, 2026

View reviewed changes