Repository navigation
[rocjitsu] Model RDNA4 floating-point SWMMAC - #13813
Conversation
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
I think we should make the shared operand mapping comments explicit about the sparse contract in emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/mma_exec.h:
- At line 1936, replace “In this model” with the gfx1201 observation: sparse B modifiers follow the compressed product slot, not the selected register half. The captured
0xcc504a18/0xeeddc988test returns0x422e0000; using the register half would give 19.5 instead of 43.5. Recording the measurement explains whybl.sub_elementmust not select this modifier. - At lines 1879–1880, describe both WMMA and SWMMAC, and use lowercase
kfor the product index. UppercaseKelsewhere denotes the matrix dimension, so the current wording conflates two different quantities.
851d81f to
05c0fd2
Compare
|
Addressed the review's regression, test organization, formatting, and contract-comment requests. The sparse B modifier comment now states the gfx1201 observation directly; the mapping helper describes both WMMA and SWMMAC and distinguishes product index k from dimension K. The optional index-hoisting optimization is deferred as explained in its thread. Validation at this commit: 14 focused C++ tests, 175 matrix generator tests, and all applicable pre-commit hooks pass. The earlier emulation job's reported failures also occur in the base develop job; it stopped with a custom-container error. Fresh CI is running on the updated commit. |
05c0fd2 to
df638f3
Compare
kuhar
left a comment
kuhar
left a comment
There was a problem hiding this comment.
I think it would help to document the supported shapes beside the executors in emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/isa/arch/amdgpu/shared/mma_exec.h:
- At line 1884, explain that this mapping serves dense 16x16x16 and sparse 16x16x32 forms, both with 16 stored products. That makes the sparse K=32 mapping and the shared
k < 16loops easier to read together. - At line 3260 and the related generic sparse-float helpers, record that their remaining generated callers are CDNA5 wave32; measured RDNA4 execution goes through
exec_gfx12_swmmac_dot4/dot8. The retained runtimewave_sizeparameter otherwise suggests broader numerical support than these callers exercise.
A brief contract comment is enough here; this does not require generalizing the fixed-shape kernels or changing the generic signatures.
We should retain two more decoded cases in emulation/rocjitsu/tests/rdna_swmmac_float_test.cpp:
- Near line 27, add a nonuniform wave64 F16 case with index key 2. Restoring the old 16-bit index-set selection still passes the existing float and integer suites, although hardware selects four 8-bit sets. With
float_input(64, 16, false), MODE0xf0, and words{0xcc505018u, 0x1c821100u}, the gfx1201 capture gives output word 0 =0xcc879066and word 255 =0x47832038; the old selection gives word 0 =0x4babaf5f. This protects the upper index-key bit independently of the current FP8 key-3 case. - Near line 19, add one packed F16 case. The packed BF16 witness exercises a different narrowing specialization, and the current accumulator-modifier cases produce F32. A compact captured F16 witness uses
{0xcc524818u, 0x9c821100u}, MODE0xf0, anduniform_input({0x3c003c00u, 0x40004000u, 0, 0xb800b800u, 0xeeddc988u}): all 128 output words are0x50105010in both wave sizes. This covers packed F16 dispatch and accumulator NEG; it does not replace the BF16 per-step narrowing test.
The current implementation matches both captures. These additions preserve behavior that the checked-in regressions do not distinguish.
|
Addressed the review: documented the dense/sparse stored-product mapping and the remaining CDNA5 wave32 callers of the generic sparse-float helpers; added the supplied wave64 F16 key-2 and packed F16 NEG witnesses; renamed the Python rejection test. The focused matrix C++ and Python tests pass. |
df638f3 to
af0e3fc
Compare
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
af0e3fc to
963214a
Compare
|
🎉 All checks passed! This PR is ready for review. |
Reuse native four- and eight-product arithmetic for sparse matrix instructions, with the RDNA index slices and source modifiers. Narrow packed accumulators at each arithmetic step. Add representative decoded cases for sparse index keys, source modifiers, and packed and mixed-format outputs. Co-authored-by: Benoit Jacob <jacob.benoit.1@gmail.com>
963214a to
dd90bda
Compare
Originally by @kuhar, with cleanup by @bjacob . Add focused decoded WMMA regressions for RDNA3 packed destination halves, RDNA4 subnormal accumulators, and BF16 factor encoding. Reuse the matrix input generator and runner, with a small sample of original expected outputs. This commit only adds tests; the arithmetic implementations are unchanged. Validation: GCC Release build and 22 focused C++ tests pass at the four-commit stack tip; generator-profile tests also pass. Stacked on #13813. Progress towards #13670. Co-authored-by: Jakub Kuderski <jakub@nod-labs.com>
TheRock Submodule Bump ActivityNewest first
|
Originally by @kuhar, with cleanup by @bjacob .
Route RDNA4 floating-point SWMMAC through the shared four- and eight-product arithmetic, with sparse index selection and source modifiers.
Share operand mapping across FP16/BF16 and FP8/BF8, and narrow packed accumulators after each arithmetic step.
Add focused regressions for sparse indices, destination overlap, modifiers, and packed and mixed-format outputs.
Validation: GCC Release build and 22 focused C++ tests pass at the four-commit stack tip; generator-profile tests also pass.
Stacked on #13812.
Progress towards #13670.