Skip to content

CustomOp: Unify aiter impl into GroupedTopk#31221

Merged
ProExpertProg merged 1 commit intovllm-project:mainfrom
xinyu-intel:dev/xinyu/unify-hip-grouped-topk
Dec 26, 2025
Merged

CustomOp: Unify aiter impl into GroupedTopk#31221
ProExpertProg merged 1 commit intovllm-project:mainfrom
xinyu-intel:dev/xinyu/unify-hip-grouped-topk

Conversation

@xinyu-intel
Copy link
Copy Markdown
Contributor

@xinyu-intel xinyu-intel commented Dec 23, 2025

Purpose

a follow-up of #29575 (comment)

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the ROCm AITer implementation for GroupedTopk by encapsulating the platform-specific logic within the GroupedTopk custom operator. This is achieved by introducing a forward_hip method in GroupedTopk and moving the AITer-related checks and function calls there. Consequently, the FusedMoE.select_experts method is simplified, removing conditional logic and direct calls to rocm_aiter_grouped_topk. The changes improve code structure and encapsulation, making the FusedMoE layer more platform-agnostic. The implementation appears correct and is a good improvement.

Signed-off-by: Xinyu Chen <xinyu1.chen@intel.com>
@xinyu-intel
Copy link
Copy Markdown
Contributor Author

@MengqingCao please take a review, thx.

@zejunchen-zejun
Copy link
Copy Markdown
Contributor

LGTM
Hi @tjtanaavllm Could you help this PR? Thank you

Copy link
Copy Markdown
Contributor

@MengqingCao MengqingCao left a comment

Choose a reason for hiding this comment

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

LGTM, thx!

@jikunshang jikunshang added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 24, 2025
@tjtanaa tjtanaa added the rocm Related to AMD ROCm label Dec 26, 2025
@ProExpertProg ProExpertProg merged commit 87f1b8c into vllm-project:main Dec 26, 2025
56 checks passed
yiliu30 pushed a commit to yiliu30/vllm-fork that referenced this pull request Dec 30, 2025
Signed-off-by: Xinyu Chen <xinyu1.chen@intel.com>
dsuhinin pushed a commit to dsuhinin/vllm that referenced this pull request Jan 21, 2026
Signed-off-by: Xinyu Chen <xinyu1.chen@intel.com>
Signed-off-by: dsuhinin <suhinin.dmitriy@gmail.com>
ItzDEXX pushed a commit to ItzDEXX/vllm that referenced this pull request Feb 19, 2026
Signed-off-by: Xinyu Chen <xinyu1.chen@intel.com>
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

None yet

Development

Successfully merging this pull request may close these issues.

6 participants