Skip to content

Conversation

zhewenl
Copy link
Collaborator

@zhewenl zhewenl commented Sep 30, 2025

Purpose

We are seeing failure on AMD due to __syncwarp is exclusively on CUDA but not on HIP

__vllm_cpp_lib_hipify_gen__/out/csrc/cache_kernels.hip:541:3: error: use of undeclared identifier '__syncwarp'; did you mean '__sync_swap'?
  541 |   __syncwarp();
      |   ^~~~~~~~~~
      |   __sync_swap

Test Plan

CI

Signed-off-by: zhewenli <[email protected]>
@zhewenl zhewenl marked this pull request as ready for review September 30, 2025 22:38
@mergify mergify bot added the rocm Related to AMD ROCm label Sep 30, 2025
@zhewenl zhewenl added the deepseek Related to DeepSeek models label Sep 30, 2025
@simon-mo simon-mo added this to the v0.11.0 Cherry Picks milestone Sep 30, 2025
Copy link
Collaborator

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

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

Ok given this fix amd build. Without this op the accuracy will drop in nvidia

@simon-mo simon-mo enabled auto-merge (squash) September 30, 2025 22:45
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 30, 2025
@mxz297
Copy link
Contributor

mxz297 commented Sep 30, 2025

AMD gpu does not support __syncwarp. So to have AMD properly support this, the first thing to try would be __syncthreads, but it would be better to be done under the context of actual adding dsv32 support on AMD. So, this fix at the moment should be good.

@zhewenl
Copy link
Collaborator Author

zhewenl commented Oct 1, 2025

checking the failing CIs:

@simon-mo could you help force merge this PR? thanks!

@zhewenl zhewenl requested a review from simon-mo October 1, 2025 02:23
@simon-mo simon-mo merged commit e952eee into vllm-project:main Oct 1, 2025
86 of 89 checks passed
@zhewenl zhewenl deleted the fix-rocm-syncwarp branch October 1, 2025 04:35
simon-mo pushed a commit that referenced this pull request Oct 1, 2025
@HAIAI
Copy link
Collaborator

HAIAI commented Oct 2, 2025

It is fine for temporary.

pdasigi pushed a commit to pdasigi/vllm that referenced this pull request Oct 2, 2025
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deepseek Related to DeepSeek models 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.

4 participants