ROCm / ROCm/aiter

[Issue]: Workspace Buffer Pointer Arithmetic in pa_v1 Ignores MTP

Open
#2,176 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.