Skip to content

Conversation

@hholtmann
Copy link
Contributor

@hholtmann hholtmann commented Nov 29, 2025

fix(cuda): add device guard and runtime SM dispatch to cutlass_scaled_fp4_mm

Purpose

Currently, the fp4 scaled_mm function doesn't work for the 5090 GPU, resulting in a RuntimeError: Internal Error. See #21274 and #22783 for more informatio

Test Plan

pytest -v -s tests/kernels/quantization/test_nvfp4_scaled_mm.py

Test Result

All passed.

@chatgpt-codex-connector
Copy link

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
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 introduces runtime SM dispatch for cutlass_scaled_fp4_mm, which is a solid improvement to support multiple GPU architectures and fixes issues on newer hardware like the 5090 series. The change from compile-time to runtime dispatch also corrects a latent critical bug where the previous implementation would attempt to return a value from a void function. The new logic is more robust and the improved error message is a good addition. I have one suggestion to refactor the dispatch logic to improve its long-term maintainability and make it less error-prone when adding support for future architectures.

@ApostaC
Copy link
Collaborator

ApostaC commented Dec 1, 2025

Hey @hholtmann , should this PR be merged to the main branch instead of releases/v0.11.2?

In the mean time, cc @mgoin @tlrmchlsmth

@mergify
Copy link

mergify bot commented Dec 1, 2025

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

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 Dec 1, 2025
@mgoin mgoin added this to the v0.12.0 milestone Dec 1, 2025
Copy link
Collaborator

@ProExpertProg ProExpertProg left a comment

Choose a reason for hiding this comment

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

The diff seems polluted, can you rebase/merge from main?

Signed-off-by: mgoin <mgoin64@gmail.com>
@mgoin
Copy link
Member

mgoin commented Dec 1, 2025

@ProExpertProg I'm working on it

@mgoin mgoin added the bug Something isn't working label Dec 1, 2025
@bbrowning
Copy link
Contributor

I tested this on my DGX Spark (sm121), was able to reproduce the original issue, and confirm that this fixes the failing tests in test_nvfp4_scaled_mm.py. I normally just compile vLLM for my specific architecture, so had to first recompile locally to trigger the original issue with a multi-arch CUDA binary.

Reproduction Steps

# Enable builds for multiple architectures
export TORCH_CUDA_ARCH_LIST="10.0f;11.0f;12.0f;12.1a"

# Clear my cmake bits and set cmake up again, since I adjusted architectures
rm -rf cmake-build-release/

# Setup cmake for new architectures
cmake --preset release

# Rebuild vLLM
cmake --build --preset release --target install

pytest -v -s tests/kernels/quantization/test_nvfp4_scaled_mm.py

After taking the steps above, all of the tests in that file failed.

Verification Steps

# Apply the patch from this PR
cmake --build --preset release --target install

pytest -v -s tests/kernels/quantization/test_nvfp4_scaled_mm.py

After applying the fix from this PR and rebuilding, all tests passed.

@mgoin
Copy link
Member

mgoin commented Dec 2, 2025

Excellent work @bbrowning, thank you for validating 🙏

@vllm-bot vllm-bot merged commit c0dfc89 into vllm-project:main Dec 2, 2025
90 of 94 checks passed
@github-project-automation github-project-automation bot moved this to Done in NVIDIA Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci/build nvidia ready ONLY add when PR is ready to merge/full CI is needed v1

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants