Skip to content

[ROCm] Refactor tuned gemms - #55001

Open
afriedri wants to merge 6 commits into
vllm-project:mainfrom
afriedri:refactor_tuned_gemms
Open

afriedri wants to merge 6 commits into
vllm-project:mainfrom
afriedri:refactor_tuned_gemms

Conversation

@afriedri

@afriedri afriedri commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Purpose

vllm/_aiter_ops.py had hardcoded GEMM shapes and pandas scraping of aiter config CSVs. All of it had to be hand-maintained as aiter retunes. aiter already answers this directly, so this PR deletes the hardcoded data and calls get_gemm_config (returns is_tuned) and get_GEMM_config_with_quant_type (returns None when untuned) instead.

Also fixes three latent bugs found while verifying:

  • MXFP4 list was written in aiter's filename units (K_file = 4 × packed bytes) while the caller passes weight.shape[1] = packed bytes, producing false positives
  • CSV scrape ignored the gfx and cu_num columns, so on gfx950 it claimed 3 gfx1250-only shapes were tuned.
  • hardcoded lists were out of date.

Testing

Ensure that all hardcoded tuned shapes are recognized by new code.

Triton — old hardcoded lists (gfx950):

List Result
is_triton_gemm_w8a8_tuned gfx950 set 11/11 preserved
is_triton_gemm_afp4wfp4_presh_ws_tuned (20 entries) 20/20 preserved

CK — every shape the old code accepted, on its own arch:

per-token    fp8_e4m3fn    gfx950/256:  37/37
per-token    int8          gfx942/80:     2/2
per-token    int8          gfx950/256:  36/36
bpreshuffle  fp8_e4m3fn    gfx1250/256:   4/4
bpreshuffle  fp8_e4m3fn    gfx950/256:  66/66
bpreshuffle  fp8_e4m3fnuz  gfx942/80:   49/49
bpreshuffle  int8          gfx942/80:     8/8
bpreshuffle  int8          gfx942/304:    2/2
                                   TOTAL 204/204

Serving benchmarkamd/Llama-3.3-70B-Instruct-MXFP4-Preview, gfx950, TP=1, VLLM_ROCM_USE_AITER=1, random 1024→128, 200 prompts, concurrency 16:

Metric Baseline This PR Δ
Output tok/s 617.6 727.6 +17.8%
Total tok/s 5557.9 6548.4 +17.8%
Mean TPOT (ms) 19.69 15.90 −19.2%
Median TPOT (ms) 19.14 15.30 −20.1%
Mean TTFT (ms) 707.3 704.9 ~flat

AI assistance (Claude) was used for this change. Changes reviewed by Andy.

Summary by CodeRabbit

  • Bug Fixes

    • Improved ROCm GEMM tuning detection for FP4 and W8A8 workloads, helping select optimized configurations more reliably.
    • Corrected FP4 shape handling for packed data layouts.
  • Performance

    • Added caching for repeated ROCm tuning checks to reduce lookup overhead.
  • Tests

    • Updated ROCm FP4 coverage to reflect supported tuned shapes and packed-byte dimensions.

Signed-off-by: Andy Friedrich <afriedri@amd.com>
Signed-off-by: Andy Friedrich <afriedri@amd.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 06225658-1e15-4bdd-b266-396ad75b3315

📥 Commits

Reviewing files that changed from the base of the PR and between 488e6fd and e2c1ad9.

📒 Files selected for processing (2)
  • tests/kernels/quantization/test_rocm_mxfp4.py
  • vllm/_aiter_ops.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change replaces pandas and hardcoded ROCm GEMM tuning data with aiter configuration lookups. Tuned-shape checks are cached and handle lookup errors. The ROCm FP4 test updates a preshuffled shape and passes packed-byte width.

Changes

Aiter GEMM tuning checks

Layer / File(s) Summary
Aiter configuration lookup helpers
vllm/_aiter_ops.py
Removes pandas-based CSV loading. Adds aiter configuration probes for Triton and Composable Kernel GEMM shapes.
Tuned-shape predicates and FP4 validation
vllm/_aiter_ops.py, tests/kernels/quantization/test_rocm_mxfp4.py
Caches tuned-shape predicates and updates them to use aiter lookups. The FP4 test uses packed-byte width and changes one preshuffled shape.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e2c1a

This localized refactor replaces stale GEMM tuning data with the existing aiter configuration lookup and corrects packed FP4 shape handling; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: shen-shanshan

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the ROCm scope and the main change: refactoring tuned GEMM detection.
Description check ✅ Passed The description directly explains the aiter-based GEMM tuning refactor, related bug fixes, testing, and performance results.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify mergify Bot added the rocm Related to AMD ROCm label Sep 2, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Sep 2, 2026
@afriedri afriedri changed the title Refactor tuned gemms [ROCm] Refactor tuned gemms Sep 3, 2026
@mergify

mergify Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @afriedri.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 4, 2026
Signed-off-by: afriedri <afriedri@amd.com>
@mergify mergify Bot removed the needs-rebase label Sep 9, 2026
@dllehr-amd
dllehr-amd self-requested a review September 9, 2026 17:17

@dllehr-amd dllehr-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@dllehr-amd dllehr-amd added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

@afriedri, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@afriedri

afriedri commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87968 for commit f430412700c4.

@dllehr-amd

Copy link
Copy Markdown
Collaborator

/amd-ci retry

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

✅ Queued 3 failed job(s) for retry in Buildkite AMD CI #12781.

@mergify

mergify Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @afriedri.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 10, 2026
Signed-off-by: afriedri <afriedri@amd.com>
@afriedri

Copy link
Copy Markdown
Contributor Author

need to fix various things because of conflicting #51692 ; wait for next commit

@mergify

mergify Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Hi @afriedri, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Signed-off-by: Andy Friedrich <afriedri@amd.com>
@mergify mergify Bot removed the needs-rebase label Sep 10, 2026
@afriedri

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88851 for commit 03de3c62363b.

@afriedri

Copy link
Copy Markdown
Contributor Author

/amd-ci retry

@github-actions

Copy link
Copy Markdown

✅ Queued 1 failed job(s) for retry in Buildkite AMD CI #12849.

Comment thread vllm/_aiter_ops.py
(8192, 1024),
(8192, 32768),
}
rdna4_tuned = gfx950_tuned | {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the RDNA4 configs now live in AITER?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants