[lua1235]

Motivation

Add another rewrite validator similar to the idempotence check. This confirms that no registered rewrite condition remains after rewrites are applied. (Idempotence would not, for example, detect a rewrite that happened to be the identity rewrite).

Technical Details

• refactor rewrites into central registry shared by both the existing rewrite system and the new validator.
  -Include operand-driven rewrites in the refactored registry, since they were not representable by the existing opcode-keyed table.
• Implement read only pass over translated binary to check for remaining rewrite sites.

Issue Tracking

N/A

Test Plan

Enable this checking pass over the existing rewrite tests + add tests for remaining added interface.

Test Result

Tests pass

Submission Checklist

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

[therock-pr-bot]

❌ PR Check — Action Required

Check	Status	Details
📝 PR Description	❌ Fail	Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword. Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples: • JIRA ID : TESTAUTO-6039 • JIRA ID - #330 • JIRA ID #330 • JIRA ID (on separate line) ROCM-25757 • ISSUE ID : TESTUTO-3334 • ISSUE ID #3334 • ISSUE ID - TESTAUTO-3433 • ISSUE ID (on separate line) AIRUNTIME-2352 • ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234 • Closes #10 • Fixes octo-org/octo-repo#100 • Resolves: #123 • #123 • https://github.com/<org_name>/<repo_name>/issues/123 Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

• ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[therock-pr-bot]

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

• ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

[atgutier]

BLOCKER

[BLOCKER] emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/binary_translator.cpp:1570 —
External entries are split as executable roots but are not guaranteed to be emitted or relocated

Reasoning: text_external_block_leaders() adds global/weak STT_FUNC and STT_NOTYPE symbols and
address-taken text entries to the CFG leader set. However, kernel_translation_scopes() at lines 791-
833 still roots emitted translation scopes only at kernel descriptors. An externally visible
function that is not descriptor-reachable may therefore receive no output placement.

The patcher’s existing contract confirms the mismatch: code_object_patcher.cpp:449-525 only requires
relocation-referenced text symbols to have mappings. An unreferenced symbol without a mapping
retains its old value. The final verifier rediscovers that stale symbol at lines 1381-1390 and
rejects it at lines 1392-1398 because it is not a decoded output block start. Without verification,
the symbol can instead silently point into unrelated translated bytes.

Evidence: Release corpus CI rejects two previously expected-success code objects with:

error: residual-rewrite: rewrite-discharge verification found an invalid final executable entry

Translation and materialization succeeded; the failure occurs only when reconstructing entries from
the produced ELF, demonstrating that the source and final entry models disagree.

Suggested fix: Establish one authoritative external-entry model across reachability, emission, text-
offset relocation, and verification. Either:

• emit supported externally addressable functions as standalone translation roots and require an
  output mapping for each; or

• restrict text_external_block_leaders() to entry forms the translator can actually preserve,
  instead of treating symbol visibility alone as sufficient.

Add a regression with an unreferenced global/weak text function and verify that its body and symbol
either survive at a relocated address or cause translation to fail before materialization.

Confidence: high.

MAJOR

[MAJOR] emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/code/dbt/binary_translator.cpp:1570-1589 —
Adding every symbol entry to full indirect-CFG discovery makes ordinary translation time out

Reasoning: The new external-leader set is passed to BasicBlock::build() even when
verify_rewrite_discharge is disabled. That routine runs the complete indirect-branch recovery/
dataflow pipeline. In indirect_branch_discovery.cpp:1251-1260, every analysis block linearly
searches every external leader; this is O(blocks × external_entries) and is repeated during fixed-
point discovery. Large packaged objects can contain many blocks and many function symbols.

The optional final verifier repeats another complete BasicBlock::build() at
binary_translator.cpp:1392, and idempotence can add another full translation, further multiplying
the cost.

Evidence:

• ASan/UBSan uses the ordinary translator without the new verification option and times out two
  expected-success objects after 90 seconds.

• GCC sanitizer times out the same large object after 180 seconds.

• Release verification times out two objects after 60 seconds.

• These are deterministic corpus regressions; unit tests, TSan, formatting, and TheRock checks pass.

This also affects runtime HSA-tool translation because the expensive external-leader analysis is
unconditional, not limited to the offline audit.

Suggested fix:

• Replace the repeated linear external-entry lookup with an offset set/bitmap or block-index map.

• Separate CFG split boundaries from indirect-analysis roots so symbol boundaries do not
  unnecessarily expand heavyweight dataflow.

• Reuse a compact final boundary/entry map for discharge verification instead of rerunning indirect-
  target discovery over the materialized ELF.

• Add large-object performance coverage using the corpus objects that currently exceed 60/90/180
  seconds.

Confidence: high.

[kuhar]

A few comments

[kuhar]

The external-entry model still needs one shared policy across entry recovery and text-symbol relocation (binary_translator.cpp:348, code_object_patcher.cpp:531). Entry recovery only collects R_AMDGPU_ABS64 references to STT_FUNC/STT_NOTYPE, while the patcher requires a mapping for any allocated zero-addend SHT_RELA reference to a non-STT_SECTION .text symbol. A targeted probe at a non-emitted .text offset was caught early only for STT_FUNC + ABS64; STT_OBJECT + ABS64 and STT_FUNC + types 1/2/12 completed translation before the patcher rejected them, and STT_OBJECT at a mapped offset was accepted where STT_FUNC was rejected. We should define the supported relocation/symbol predicate once and use it in both paths, with regressions for the non-ABS64 and symbol-type cases.

[kuhar]

LGTM % one comment

[kuhar]

@lua1235 can you rebase and address #9560 (comment) ?

[kuhar]

Left an inline reply and 2 new comments

[kuhar]

Left a few inline comments

[lua1235]

Gfx1250F32K128WmmaAcceptsOperandsThePackedF16PathRejects now fails rewrite-discharge verification due to a (small) upstream inconsistency. The translated scaled-WMMA form preserves an SGPR SRC2, but the final-stream decoder currently rejects that. Will investigate.

[kuhar]

LGTM % the failure you noted above

[kuhar]

LGTM