RoPE kernel for latency test: issue #166 (issue #94), standalone 8-core (4x2) - #194
Conversation
…ing with Granite-8b scale - Grid [4,2], TILE_SEQ=256: seq-tile loop (4 iter) - head loop (20 iter). - Half-layout RoPE (LLaMA convention): y[0:D/2] = x[0:D/2]*cos - x[D/2:D]*sin; memory-bound at AI ~0.73 FLOPs/byte. - Tests: correctness, latency scaling (memory/simd/comm), MLIR frontend adapt. Signed-off-by: Hao Yu <yuh@us.ibm.com>
Signed-off-by: Hao Yu <yuh@us.ibm.com>
a2b64bd to
c9a3e26
Compare
Notes on addressing a loop-interchange rewriting of the standalone RMSNorm kernel.Lars left a comment somewhere about a potential poor-forming of the mlir file. Does that have any grounding in reality? In Gist: It translates that 'loop interchange' is the obvious path to avoid unnecessary reload sin-cos constants for RMSNorm. Actions: update the kernel with following design thoughts: interchanging those 2 loops is convincing now
I wrongfully assumed that MLIR has a way to pass part of the array to inside the loop. There was 'preload the needed sin/cos to my core' outside the loop nest. Yet MLIR can not link that load to the 'slicing-relayout/reload' of cos/sin. Now will follow your suggestion, |
lasch
left a comment
There was a problem hiding this comment.
/lgtm
Optional item:
G1 (minor, cheap). test_rope_fwd_zero_input provides little independent signal: with x = 0, both y_first and y_second reduce to 0 * cos ± 0 * sin = 0 regardless of whether cos/sin are loaded or indexed correctly, so this test would still pass even if the cos/sin addressing were completely broken. The main test_rope_fwd_correctness test already covers the general case well via random input compared against a full-tensor NumPy reference across all 8 cores, so this isn't a large gap — but a more targeted edge case (e.g. an input that isolates the sin/cos cross-term, or a boundary-head/boundary-tile-specific check) would add more real signal than the zero-input check does today.
The RoPE kernel (torch-spyre#194) landed on main after this branch renamed `#ktdp.spyre_memory_space<HBM>` to `#ktdp.memory_space<global>`, so the merge of main brought in four occurrences of the old spelling. It no longer parses under the bumped ktir-mlir-frontend pin, which is why the frontend job failed on the merge commit while the regex-parser job passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Yu Chin Fabian Lim <flim@sg.ibm.com>
Issues addressiog
What's done?
Add standalone 8-core (4x2) RoPE kernel (issue #166) aligning with Granite-8b scale