[Issue]: Workspace Buffer Pointer Arithmetic in pa_v1 Ignores MTP
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 565
- Forks
- 585
- Avg merge
- 3d 4h
- Merged PRs (30d)
- 366
Description
Problem Description
The pa_v1 C++ kernel computes workspace pointers without considering MTP
https://github.com/ROCm/aiter/blob/d2f5f2791336a9839c65241b77a88f0a8407a054/csrc/cpp_itfs/pa/pa_v1.cpp.jinja#L69-L72
This allocates based on num_seqs only. However, the main kernel writes to index seq_idx + mtp * MTP_PARALLEL_THREADS,
which for mtp > 0 can write beyond the allocated buffer, causing memory corruption.
While the Python test code correctly sizes the buffer with mtp:
https://github.com/ROCm/aiter/blob/d2f5f2791336a9839c65241b77a88f0a8407a054/op_tests/test_pa_v1.py#L308-L319
Also note that there is zero test coverage for MTP > 1:
The test file (test_pa_v1.py) never tests with mtp > 1
Operating System
Ubuntu 22.04.5 LTS (Jammy Jellyfish)
CPU
AMD EPYC 9575F 64-Core Processor
GPU
8 x AMD Instinct MI355X
ROCm Version
ROCm version: 7.0.51831-a3e329ad8
ROCm Component
No response
Steps to Reproduce
Replicator script:
paged_attention_v1.issue3.mi355.py
Logs from running replicator script:
paged_attention_v1.issue3.mi355.log
(Optional for Linux users) Output of /opt/rocm/bin/rocminfo --support
No response
Additional Information
My goal is to enable CUDA graph support for speculative decoding in the ROCM_AITER_FA attention backend by adding spec_as_decode support and changing _cudagraph_support from UNIFORM_SINGLE_TOKEN_DECODE to UNIFORM_BATCH. This is expected to speed up speculative decoding: removes graph breaks and allows routing speculative tokens through the more efficient decode path instead of the extend path. The AITER PA API appears to support MTP>1, but the implementation seems to be buggy.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with csrc/cpp_itfs/pa/pa_v1.cpp.jinja lines 69-72 and compare its workspace sizing with the indexing described in the issue. Review op_tests/test_pa_v1.py lines 308-319, add coverage for mtp > 1, and run the pa_v1 tests to verify the workspace remains large enough without memory corruption.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- backend, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100