Skip to content

[Build] Build bundled DeepGEMM _C per-Python so the wheel imports on every CPython - #41516

Merged
mgoin merged 8 commits into
vllm-project:mainfrom
mgoin:deepgemm-multi-python-build
May 12, 2026
Merged

[Build] Build bundled DeepGEMM _C per-Python so the wheel imports on every CPython#41516
mgoin merged 8 commits into
vllm-project:mainfrom
mgoin:deepgemm-multi-python-build

[Build] Address review: respect $CXX, explicit CUDA_HOME check

84aa4ad
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 2, 2026 in 14m 3s

Code review found 1 important issue

Found 5 candidates, confirmed 4. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important .buildkite/test_areas/kernels.yaml:122 Bare python in kernels.yaml will fail (no python symlink in test image)
🟡 Nit tools/check_wheel_deepgemm.py:19 check_wheel_deepgemm imports the package, masking its own diagnostic
🟡 Nit cmake/external_projects/deepgemm.cmake:61-65 Empty DEEPGEMM_PYTHON_INTERPRETERS silently skips the build

Annotations

Check failure on line 122 in .buildkite/test_areas/kernels.yaml

See this annotation in the file changed.

@claude claude / Claude Code Review

Bare `python` in kernels.yaml will fail (no `python` symlink in test image)

The new check command uses bare `python` instead of `python3`, which will fail on the H100 deepgemm CI step. The vllm-base test image only sets up `/usr/bin/python3` (via update-alternatives) and never creates a `python` symlink, so the very first command of this step will exit with `python: command not found`, defeating the wheel verification this PR adds. Trivial fix: change `python` to `python3` on line 122.

Check warning on line 19 in tools/check_wheel_deepgemm.py

See this annotation in the file changed.

@claude claude / Claude Code Review

check_wheel_deepgemm imports the package, masking its own diagnostic

The script's whole purpose is to print a clear 'deepgemm _C: found {...}, required [...], missing [3.X]' diagnostic when a per-Python `_C.cpython-X.Y-*.so` is missing from the wheel — exactly the regression #41476/#41512 was about. But `import vllm.third_party.deep_gemm as pkg` at line 19 executes DeepGEMM's vendored `__init__.py`, which eagerly does `from . import _C` and then `from ._C import (set_num_sms, ...)`. If the running interpreter's `_C.so` is the one missing, that import raises Impor

Check warning on line 65 in cmake/external_projects/deepgemm.cmake

See this annotation in the file changed.

@claude claude / Claude Code Review

Empty DEEPGEMM_PYTHON_INTERPRETERS silently skips the build

The CMake guard `if(DEFINED ENV{DEEPGEMM_PYTHON_INTERPRETERS})` is true for an empty string, so an empty `/tmp/dg_pythons.txt` would silently skip the per-Python build (foreach runs 0 times → `add_custom_target(_deep_gemm_C ALL DEPENDS)` with no deps → no `_C*.so` shipped) — exactly the regression this PR is meant to prevent. Trigger is narrow today (the bash script uses `set -euo pipefail` and current `requires-python` is non-empty), and `tools/check_wheel_deepgemm.py` catches it on the H100 st