Repository navigation
[rocjitsu] [perf] Let VOPD instructions use the decoded-instruction cache - #14801
Conversation
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the VOPD encoding-ownership and cache-reuse changes.
No open issues 🎉
What changed in this PR
This PR lets decoded VOPD instructions own their encoding, enabling reuse by the compute unit’s decoded-instruction cache without weakening its borrowed-buffer safeguards.
Changes:
- Update the generator to copy instruction bytes into internal storage.
- Regenerate VOPD implementations for RDNA3, RDNA3.5, RDNA4 and CDNA5.
- Test encoding ownership and cache reuse for 8-byte and 12-byte literal forms, while preserving coverage of borrowed-encoding rejection.
| File | Description |
|---|---|
emulation/rocjitsu/tests/instruction_cache_test.cpp |
Tests owned VOPD reuse and borrowed-buffer rejection. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vopd.h |
Adds internal encoding storage for RDNA4. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna4/vopd.cpp |
Copies RDNA4 encoding into owned storage. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/vopd.h |
Adds internal encoding storage for RDNA3. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3/vopd.cpp |
Copies RDNA3 encoding into owned storage. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/vopd.h |
Adds internal encoding storage for RDNA3.5. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/rdna3_5/vopd.cpp |
Copies RDNA3.5 encoding into owned storage. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vopd.h |
Adds internal encoding storage for CDNA5. |
emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/generated/cdna5/vopd.cpp |
Copies CDNA5 encoding into owned storage. |
emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py |
Generates owned encoding storage and initialization. |
Review effort: Balanced
yashjha123
changed the title
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
It would be useful to update the comment at emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/decoder.cpp:97: it still names combined encodings as borrowers, but the generated VOPD classes now own their bytes. Mentioning external decoders alone would make the explanation match the current implementation. The rebinding and cache guard should stay, since external decoders can still borrow. This is a small documentation follow-up.
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
We should update the two VOPD3 prefix assertions in emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py:7894 and :8042 for the word0 rename. The gfx1250 test now fails in the amdisa suite run by the corpus workflow. The RDNA4 test still passes because it checks for the absence of the old spelling.
For gfx1250, could we check (word0 >> 24) == 0xCF inside Vopd::Vopd? The generated validate_encoding contains the same condition, so a whole-file search would still pass if the constructor lost its VOPD3 branch.
VOPD constructors pointed raw_encoding_ at the caller's fetch words. The CU's decoded-instruction cache cannot keep an instruction that borrows the fetch window, so every VOPD issue on RDNA3, RDNA3.5, RDNA4 and gfx1250 paid a full decode and heap allocation. Copy the encoding into the instruction, as other generated encodings already do, so VOPD reuses its decode like any other ALU instruction. The cache's borrowed-encoding guard keeps its coverage through a test-only decoder. End to end against 9f68654 (region time, median of 5 paired rounds, fresh process per run, warm-up per build, budget 1, Ryzen 9950X3D2): flaggems amax-fp32-65536x4096 gfx1250 11.38 -> 8.20 s 1.387x torch batch-norm-train-128x256x4096 gfx1250 29.96 -> 28.51 s 1.051x torch cumsum-fp32-8192x4096 gfx1250 12.08 -> 11.64 s 1.038x rocprim merge-sort-u32-4m gfx1201 22.67 -> 21.86 s 1.037x torch batch-norm-train (no VOPD) gfx950 9.25 -> 9.29 s 0.996x
The VOPD constructor is the only reader of word0_, word1_ and word2_; the owned raw_words_ now holds the encoding for the instruction's lifetime, so cached instructions no longer carry the parsing scratch. Also say that only external decoders retain the input pointer in Decoder::decode_window(), since the generated VOPD classes own their bytes, and name the word that follows the 8-byte VOPD in the reuse test.
The constructor's encoding words are now locals named word0..word2, so the gfx1250 and RDNA4 generator checks still looked for the old word0_ spelling: the gfx1250 check failed and the RDNA4 one passed without testing anything. Use the new spelling, and look for the gfx1250 VOPD3 prefix in the constructor body: validate_encoding() tests the same prefix, so a whole-file search would pass even if the constructor lost its VOPD3 branch.
gfx1250 VOPD3 is 12 bytes without a literal: the third word holds register fields, and perfsim reads all three words from raw_encoding(). Copying only 8 bytes unless a literal is present passed the existing VOPD cache test but returned zero for that word on a cache hit; this test catches it.
Scalar and vector ALU encodings already have generator tests that their constructors copy size_ bytes into raw_words_. VOPD had the same copy but no such check: a generator that copied a fixed 8 bytes passed the whole amdisa suite. Generate both VOPD templates (cdna5 with VOPD3, rdna4) and check that the copy follows every size_ assignment and that raw_encoding_ points at the copy.
e0e477f to
5ce4a09
Compare
rocjitsu post-commit fuzz passedThe 32-worker multiplexer campaigns for commit
|
A quick win: VOPD instructions were never reused by the CU's decoded-instruction cache. Their constructor pointed raw_encoding_ at the caller's fetch words, and the cache does not keep instructions that borrow the fetch buffer, a deliberate exclusion from #12632. So every VOPD issue on RDNA3, RDNA3.5, RDNA4 and gfx1250 paid a full decode plus a heap allocation.
This PR copies the encoding into the instruction (raw_words_ + memcpy), as other generated encodings already do, which lifts that exclusion.
Full details for agents and humans
Cause and fix
Vopd::raw_encoding_raw_words_inside the instructionborrows_fetch_buffer)The change is in the generator template (
_generator.py). The regenerated output touches only the eightvopd.{h,cpp}files for rdna3, rdna3_5, rdna4 and cdna5. The copy happens oncesize_is known: 8 bytes, or 12 with a literal or in the VOPD3 form.Tests
Changed.
BorrowedVopdEncodingIsNotRetainedAcrossIssuesasserted the old borrowing behaviour, so it now covers two things separately:BorrowedEncodingIsNotRetainedAcrossIssueskeeps the cache's borrowed-encoding guard covered with a test-only decoder whose instruction borrows its words. This is the same pattern asdecoder_window_test.cpp.VopdOwnsItsEncodingAndIsReusedAcrossIssueschecks all four VOPD targets, in the 8-byte form and the 12-byte literal form. The encoding is owned, the second issue reuses the decode (one decoder call), and the encoding is intact after the first fetch buffer is cleared.Mutation check. Restoring
raw_encoding_ = wordsin the RDNA4 output alone fails the new test for RDNA4 only, in both forms.Suite. Full
ctestpasses. On the test host,hsa_init_testdoesn't link against its older system HSA runtime, and that is unrelated to this change.pre-commitis clean.Performance
Method. Region time from the workload's own markers. Each run is a fresh process. There is one warm-up run per build, then 5 paired rounds in alternating order. Each run has a thread budget of 1 and is pinned to 4 cores / 8 threads of an otherwise idle Ryzen 9950X3D2. Base is
9f686546da7. Base run-to-run variation is 0.1–1.0% per workload.Where it applies. VOPD only exists on RDNA3, RDNA3.5, RDNA4 and gfx1250, so other targets are unaffected (see the gfx950 control). The gain scales with the workload's VOPD share. Reductions, argmax/amax, RNG, sort and norm kernels on gfx1250 use VOPD the most.
Review focus
raw_words_) preferable to making the cache accept borrowers by rebindingraw_encoding_on return?raw_words_duplicatesword0_..word2_. Shouldword*_read fromraw_words_instead, or is the small duplication fine in generated code?