[kuhar]

Motivation

Allow lightweight fixed-profile DBT/DBI frontends to reuse the RocJITsu decode
and ISA model layers without pulling in simdojo, the VM, or every ISA target.
This also creates a cleaner boundary for future out-of-tree ISA targets and is
a prerequisite for the production integration tracked in #8939.

Technical Details

• Teach the ISA generator to produce explicit model and execution outputs for
  profiles that request the split.
• Split generated gfx1250 implementation files into model sources and
  *_exec*.cpp execution sources. Model objects are compiled once and shared;
  no source is compiled twice under different preprocessor definitions.
• Define rocjitsu_isa_gfx1250_model and rocjitsu_isa_gfx1250_exec object
  targets. Preserve rocjitsu_isa_gfx1250 as their full-simulator aggregate.
• Move the all-architecture Decoder::create() registry out of the generic
  decoder library so fixed-profile frontends can provide a narrow factory.
• Add rocjitsu_dbt_internal, containing shared analysis, code-object, and ISA
  infrastructure without the simulator, VM, or decoder registry.
• Split simdojo_headers from the simdojo implementation so header
  dependencies do not pull simulator objects into decoder-only frontends.
• Add a standalone gfx1250 model-only link/decode test and an ELF
  symbol-boundary test.

The model boundary is structural: execution implementations live in separate
translation units and are omitted from model-only links. It does not rely on
preprocessor variants, sanitizer exceptions, or linker garbage collection.

Scope and Follow-up

This PR establishes and tests the composable library boundary. It does not
migrate a production tool yet; the Hotswap/DBT consumer integration is tracked
in #8939.

This PR also does not introduce separate public shared-library export surfaces.
RocJITsu retains its existing default-hidden, explicit-export visibility
policy. A per-library export map or version script can be added with a future
shared-library consumer if needed.

Issue Tracking

Related: #8939

Test Plan

• Build and run the model-only decode/link test in normal, UBSan, and TSan
  configurations.
• Inspect the linked model-only ELF and reject execution, VM, and
  address-calculation symbols while requiring the gfx1250 decoder symbol.

Test Result

The two focused tests passed in all three local build configurations:

• Normal: 2/2 passed.
• UBSan: 2/2 passed with the full configured sanitizer set, including vptr.
• TSan: 2/2 passed.

The symbol-boundary test verifies that the model-only binary contains the
gfx1250 decoder while excluding:

• instruction execute_impl definitions and generated _exec functions;
• simulator address-calculation helpers;
• VM types such as ComputeUnitCore.

Submission Checklist

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

[therock-pr-bot]

✅ 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]

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

[Copilot]

Pull request overview

This PR refactors RocJITsu to split the ISA model/decode layer from simulator implementation details, enabling “model-only” DBT/DBI frontends to link only the architecture model they need (reducing binary size and avoiding simulator/VM symbol pull-in).

Changes:

• Moved the full all-architecture Decoder::create() registry into a dedicated rocjitsu_isa_registry object library and updated existing tool/test link targets accordingly.
• Introduced rocjitsu_dbt_internal (a narrow static archive) plus a Linux-only gfx1250 model-only ISA variant that returns null execution callbacks for instruction instances.
• Split simdojo_headers (header/interface deps) from simdojo implementation objects, and added a standalone Linux model-only link+decode smoke test.

Reviewed changes

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

Show a summary per file

File	Description
emulation/rocjitsu/tools/rocjitsu/CMakeLists.txt	Links the new rocjitsu_isa_registry into the rocjitsu tool to preserve Decoder::create() availability.
emulation/rocjitsu/tools/CMakeLists.txt	Adds rocjitsu_isa_registry to the tool object library set for consistent decoder factory linkage.
emulation/rocjitsu/tests/model_only_isa_test.cpp	New smoke test that defines a narrow Decoder::create() and validates decode works with execute == nullptr.
emulation/rocjitsu/tests/CMakeLists.txt	Adds a separate Linux-only rocjitsu_model_only_test binary to validate the model-only boundary in isolation.
emulation/rocjitsu/lib/simdojo/CMakeLists.txt	Introduces simdojo_headers as an INTERFACE target and makes simdojo depend on it, decoupling headers from implementation objects.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/instruction.h	Allows model-only builds to produce instructions with null execute callbacks via ROCJITSU_ISA_MODEL_ONLY.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/decoder.cpp	Removes the full decoder registry implementation from the generic decoder translation unit.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/decoder_factory.cpp	New full decoder registry implementation for simulator/public entry points.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/CMakeLists.txt	Adds the rocjitsu_isa_registry object library target containing the full registry.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/gfx1250/CMakeLists.txt	Enables a Linux-only model-only build variant for gfx1250 via MODEL_ONLY_VARIANT.
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/hooks/CMakeLists.txt	Links rocjitsu_isa_registry so hooks continue to resolve Decoder::create().
emulation/rocjitsu/lib/rocjitsu/CMakeLists.txt	Adds rocjitsu_dbt_internal static archive and propagates --gc-sections to consumers on Linux.
emulation/rocjitsu/CMakeLists.txt	Updates top-level linking to include rocjitsu_isa_registry alongside existing ISA components.
emulation/rocjitsu/cmake/rj_add_object_library.cmake	Extends rj_add_object_library to support MODEL_ONLY_VARIANT and builds a _model object lib with section-splitting + ROCJITSU_ISA_MODEL_ONLY.

---

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

[atgutier]

From the agent:

Two distinct concerns are worth separating in your goal:

1. "Separable modules that can be intermixed" — this PR genuinely helps. It carves the decode/model layer away from the simulator/VM so a DBT frontend can link rocjitsu_dbt_internal + one _model object set without dragging in simdojo, the VM, or the all-ISA
   registry. That's real progress toward independently composable units.
2. "Only public ISA symbols exposed" — this PR does not move that needle, and doesn't need to. Symbol export is already handled globally, independent of this change:
   - CMAKE_C/CXX_VISIBILITY_PRESET hidden + VISIBILITY_INLINES_HIDDEN (CMakeLists.txt:44-46)
   - -fvisibility=hidden per object lib (rj_add_object_library.cmake:22)
   - --exclude-libs,ALL on rocjitsu_shared (CMakeLists.txt:316)
   - Explicit RJ_API_EXPORT/attribute((visibility("default"))) opt-in for public symbols (rj_compiler.h:14).

So export control is default-hidden + opt-in already. This PR reorganizes link units but doesn't add/regress visibility control. If your end goal is separate .sos each exporting only their public ISA symbols, that still needs a follow-up (per-.so version scripts or
continued RJ_API_EXPORT discipline) — it's not in this PR, but this PR doesn't block it either.

Caveat that directly bears on your "intermix without symbol-resolution issues" goal: the design's key safety property is currently enforced by a comment, not the build system. See the first MAJOR below — two variants of the same ISA carry the same strong out-of-line
symbols, so intermixing the wrong pair is a silent ODR violation, exactly the class of symbol-resolution bug you want to eliminate.

Findings

MAJOR

1. The "never co-link normal + model" invariant is documentation-only — a latent ODR/symbol-resolution hazard (cmake/rj_add_object_library.cmake:47-52)
   The same generated sources are compiled twice (rocjitsu_isa_gfx1250 and rocjitsu_isa_gfx1250_model) with different ROCJITSU_ISA_MODEL_ONLY values. The generated instruction constructors (e.g. SNopSopp::SNopSopp in gfx1250/sopp.cpp) are out-of-line strong symbols, and
   their bodies differ (real ExecuteFn vs nullptr). If any final target ever links both object sets, the linker silently picks one definition — no diagnostic. EXCLUDE_FROM_ALL keeps model out of the default build but does not prevent an explicit target_link_libraries
   from pulling in both.
   Fix: enforce it. Either a CMake assertion that walks the link graph and FATAL_ERRORs if both rocjitsu_isa and __model are reachable, or — more robust — put the model variant in an inline namespace selected by the macro so co-linking becomes an ordinary
   duplicate-symbol error instead of silent UB. This directly serves your "intermix without symbol issues" goal.

2. Model-only link relies on --gc-sections for the link to resolve, not just to shrink (cmake/rj_add_object_library.cmake:64-71, lib/rocjitsu/CMakeLists.txt:27-28)
   In model-only mode the execute_impl bodies are still compiled (only make_exec_fn's returned pointer changes to null). Those bodies reference out-of-line VM symbols (Wavefront, etc.) that the model frontend does not link. The link only succeeds because --gc-sections
   strips every now-unreachable execute_impl before symbol resolution. So GC is load-bearing for correctness, not size — fragile, and the reason for the Linux-only FATAL_ERROR.
   Fix: compile-guard the execute_impl bodies (or their VM includes) under ROCJITSU_ISA_MODEL_ONLY so the model objects are genuinely self-contained. This also removes the Linux-only restriction and makes the modules truly independent rather than linker-trick-dependent.

MINOR

• Uneven section-GC (lib/rocjitsu/CMakeLists.txt:27-28): --gc-sections is propagated to consumers, but the base archive objects (analysis/code/isa) aren't compiled with -ffunction-sections/-fdata-sections, so GC only works at whole-object granularity for them. The
  size-trimming story reads as more complete than it is. Either add section flags to the base libs or add a clarifying comment.
• No production consumer (tools/CMakeLists.txt): only the new test uses rocjitsu_dbt_internal/_model; rj_dbt_translate still links the full VM + all ISAs. Fine as scaffolding — just state in the PR body that the tool migration is a follow-up.
• Duplicated Linux gating (gfx1250/CMakeLists.txt:22-27): the per-ISA if(Linux) boilerplate re-encodes a rule the function already enforces via FATAL_ERROR. Will get copy-pasted per ISA. Let callers pass MODEL_ONLY_VARIANT unconditionally and have the function
  silently skip on non-Linux.

NIT

• tests/model_only_isa_test.cpp:19-24: the test-local Decoder::create is the crux of the design (frontend supplies its own narrow factory, resolving the rj_decode.cpp reference instead of linking rocjitsu_isa_registry) — worth one more sentence of comment so it's not
  mistaken for scaffolding.

[atgutier]

Approved but consider resolving the below issues.

GPT 5.6 SOL expert review:

Does it help the modular .so goal?

Yes—substantially. This establishes an important link-time composition boundary, but it does not yet
establish a shared-library ABI boundary.

The strongest improvements are:

• gfx1250 model and execution code are now physically different translation units and object targets
  in lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/gfx1250/CMakeLists.txt:24-73.

• The model is compiled once. A decoder/DBT consumer can link rocjitsu_isa_gfx1250_model, while the
  simulator composes model plus execution objects through rocjitsu_isa_gfx1250.

• Generated constructors obtain execution callbacks through a registration slot in
  instruction.h:323-338; omitting execution objects cleanly leaves callbacks null.

• The all-architecture Decoder::create() registry is separated from generic decoder infrastructure
  in isa/CMakeLists.txt:11-16.

• simdojo_headers separates header requirements from the simulator implementation in lib/simdojo/
  CMakeLists.txt:4-19.

• rocjitsu_dbt_internal provides a simulator-free static composition base in lib/rocjitsu/
  CMakeLists.txt:13-28.

• The model-only test decodes both scalar and vector instructions and verifies null execution
  callbacks.

• The symbol test rejects execution, VM, and address-calculation symbols from the model-only image.

This is much stronger than relying on dead-section elimination: the boundary is structural and
generator-owned.

What remains before the final goal is achieved

1. There are still no new component DSOs

The new artifacts are OBJECT, STATIC, and INTERFACE targets. rocjitsu_dbt_internal is explicitly
STATIC, and the only installed shared targets remain rocjitsu_shared and rocjitsu_hooks at root
CMakeLists.txt:345-349.

Therefore, this PR proves that a lean final image can be composed; it does not produce or package
that image as a component .so.

2. Export ownership and intermix safety are not defined

The project correctly uses hidden visibility by default at root CMakeLists.txt:42-46, but public C
entry points override this through RJ_API_EXPORT.

Both existing shared libraries still assemble overlapping analysis/code/ISA object sets:

• rocjitsu_shared: root CMakeLists.txt:267-298
• rocjitsu_hooks: hooks/CMakeLists.txt:25-49

A future component DSO that embeds rocjitsu_dbt_internal could likewise expose rj_code_* definitions
unless the final target applies an export map or hides archive symbols. Safe intermixing requires
exactly one owner for every exported symbol—either a common core DSO or disjoint, module-specific
API surfaces.

There is currently no:

• ELF version script/export allowlist
• nm -D or readelf --dyn-syms assertion
• ABI version or SOVERSION
• installed CMake export/package for component targets

3. Model/exec separation is link-time composition, not a runtime plugin ABI

The callback slots in instruction.h:325-338 are hidden, image-local C++ state. That is correct when:

• a DBT DSO embeds model objects only; or
• a simulator DSO embeds model and execution objects together.

It would not support loading model.so first and later loading an independent exec.so and expecting
the latter to populate the former’s callback slots. If execution providers are intended to be
independently loadable, they need an explicit exported registration ABI plus provider lifetime/
unload rules.

4. The model boundary still has private compile-time dependencies

Although the model-only link excludes VM implementation symbols, generated model sources still
include private VM headers. Examples include:

• gfx1250/vop3_alu.cpp:7-16
• gfx1250/vbuffer.cpp:7-18
• gfx1250/vds.cpp:7-18
• gfx1250/isa.h:7-13

So the boundary is currently clean at link time, but not yet a self-contained installed-header or
out-of-tree compilation boundary. The next iteration should give model and execution outputs
separate include sets and move the minimal model-facing types into an explicitly supported internal/
public-model header layer.

5. “Small in isolation” is not measured yet

The symbol denylist is valuable, but it operates on a GTest executable rather than a component .so.
There is no test for:

• final .so file size
• .text/.rodata budgets
• permitted DT_NEEDED dependencies
• dynamic export count
• accidental simulator dependency growth

6. Only gfx1250 has the structural split

Gfx1250Profile.split_execution_sources is the only enabled profile. This is a good reference
implementation, but the broader modular goal still requires applying the pattern to the other ISA
families or defining which component owns their combined model/execution code.

Recommended next phase

1. Create actual component SHARED targets with stable C APIs or uniquely named plugin descriptors.

2. Give each exported symbol exactly one owning DSO.

3. Apply per-DSO version scripts/export maps and hide all implementation archives.

4. Add nm -D, readelf --dyn-syms, and readelf -d allowlist tests.

5. Load multiple component DSOs with RTLD_GLOBAL in both orders and exercise each API and opaque
   handle.

6. Add file-size, section-size, dependency, and export-count budgets.

7. Separate model and execution include sets so the model target no longer requires private VM
   headers.

Test and inspection assessment

Current required CI is green: release, ASan/UBSan, GCC sanitizer, TSan, formatting, CodeQL, and
TheRock package builds passed.

The current head was not available as a local Git object, so I inspected full current-head content
through read-only GitHub blobs and did not locally regenerate or compile it. The worktree was left
untouched.

Bottom line: PR 8933 is clean and is meaningful prerequisite work. It creates the correct source and
object-level seam for small component DSOs. The actual DSO ABI, export discipline, coexistence
guarantees, packaging, and size enforcement remain the next phase.

Opus 4.8 expert review:

MAJOR

1. Registration design isn't .so-split-safe (instruction.h:315-322; gfx1250 operand.h execution_backend()) — details above. This is the central gap vs. your goal. Fix: decide the boundary contract now — make the cross-boundary slot/table an RJ_API_EXPORTed ABI keyed
   by a runtime id (not a per-type template local static), and make the model side assert/throw on an unregistered backend at first use instead of returning null. At minimum, document in instruction.h that the slot assumes model+exec are co-linked into one image and is
   NOT .so-split-safe — otherwise the next person splits the targets and ships a silently-broken simulator.

2. Model TUs still #include the VM/simulator headers (sopp.cpp:8-9, sop1.cpp:8-11, vds.cpp:9-14, vglobal.cpp:10-13, smem.cpp:10-12). The generator passes one cpp_includes list to both model and exec file sets, so every per-encoding model file pulls
   wavefront.h/compute_unit.h/execute_shared.h etc. — even though it references no VM symbols. The nm test passes (no forbidden symbols emitted), but the source-level simulator coupling the PR claims to sever is still there; the model can't be compiled without the VM
   tree present. Only operand.cpp got a trimmed include list. Fix: give the model file set its own narrow include list, as already done for operand.cpp.

MINOR

• gfx1250 operand machinery diverges + duplicates shared SIMD code (operand.h, instruction_compute_unit_view.h:77): gfx1250 now derives from IsaOperand instead of the shared AmdgpuIsaOperand, and the generator re-emits ~20 *_exec bodies that duplicate the
  shared SIMD logic in isa_operand_simd_inl.h verbatim. A future fix to the shared path silently won't reach gfx1250. Consolidate into shared free functions both paths call.
• Registration trio under-documented (instruction.h:305-322): no @tparam/@returns and, critically, no statement of the static-init-before-any-decode invariant that is the whole correctness argument. Add it.
• Non-split ISAs run a redundant _exec write + stale-removal pass per encoding (_generator.py ~L8102-8123). No-op today (zero churn confirmed) but deletes any future arch legitimately named _exec.cpp. Guard with if profile.split_execution_sources:.

NIT

• execute is documented as "must not be called when null" but no assert(inst->execute) guards any call site — a debug assert would also catch the silent-null .so-split case loudly.
• _generator.py: import os re-imported inside many methods (pre-existing; new one at L8137).

Verified non-issues (things that are correct)

• No static-init-order hazard in the full simulator build: no global/static Instruction instances, all decode runs after main(); exec-TU registrars run before it. Confirmed.
• Registration is race-free: slots/tables are written only from namespace-scope static initializers (single-threaded, pre-main()); runtime is read-only → happens-before holds, plain store is safe.
• Model object set is symbol-clean: model .cpp bodies reference zero VM/execute symbols (only ISA enum constants); rocjitsu_dbt_internal + rocjitsu_isa_gfx1250_model links standalone, and the nm test has a non-vacuous required-symbol guard.
• Other ISAs unchanged — zero regen churn across all other AMDGPU arches + RISC-V.
• addr_calc.cpp correctly exec-only (uses register/memory state; on the model denylist) and generated files carry the AUTO-GENERATED / DO-NOT-EDIT banner — split is generator-driven, not hand-edited.

Bottom line: cleaner and more robust than v1 for a single-.so build (the ODR hazard and the load-bearing --gc-sections trick are both gone, and thread-safety/static-init are sound). But for your specific goal — intermixable .sos with no symbol-resolution surprises —
the new runtime-registration indirection is the weak point: it silently breaks across a model/exec .so boundary rather than failing loudly. I'd treat MAJOR #1 as the thing to resolve (or at least explicitly document and defer alongside #8939) before building the real
.so split on top of it. Nothing was posted to GitHub.

[amd-arosa]

some tests are failing on my mi300x box. doing investigation.

[kuhar]

@atgutier addressed most comments
@amd-arosa maybe nuke the build dir and start over?

[amd-arosa]

found some real issues that need to be fixed, but they aren't caused by this change and can go in separate prs.