[Kernel/Docker/CMake] Add CCCL support for CUDA 13 in CMake and update Dockerfile - #20588
NorthSecond wants to merge 2 commits into
Conversation
…t variables in Dockerfile
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the handling of CUDA 13's CCCL header changes by moving a previously Docker-specific workaround into the CMake build system. The change ensures that projects dependent on Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a build issue with CUDA 13 by moving the fix from a Docker-specific workaround to the CMake build system. This makes the build process more robust and works for non-Docker environments as well. The changes involve adding logic to CMakeLists.txt to link against CCCL for CUDA 13+ and removing the now-redundant include path workaround from the Dockerfile. The implementation is a clean backport of the upstream fix in mscclpp. I have one minor suggestion to improve consistency in the CMake version check.
|
Thanks @NorthSecond! Two things changed under this PR: |
Related: #14066 #17600 #18800
Motivation
CUDA 13 introduced CCCL 3.0, which moved several headers from:
${CTK_ROOT}/include/cuda/${CTK_ROOT}/include/cub/${CTK_ROOT}/include/thrust/to:
${CTK_ROOT}/include/cccl/cuda/${CTK_ROOT}/include/cccl/cub/${CTK_ROOT}/include/cccl/thrust/Per NVIDIA's migration guide, this can break builds for translation units compiled by the host compiler alone, because unlike
nvcc, host compilers do not automatically pick up the new CCCL include layout. One common symptom is:fatal error: cuda/atomic: No such file or directorymscclppis one of the affected dependencies. Themscclpprevision currently pinned insgl-kernelpredates the upstream CUDA 13 fix, so its CMake logic does not yet know thatCCCL::CCCLmust be linked for CUDA 13+ builds.As a result, source builds of
sgl-kernelon CUDA 13 currently rely on a Docker-specific include path workaround instead of handling the issue in the build system itself.Problem with the current workaround
The current workaround injects CCCL-related include behavior from
sgl-kernel/Dockerfile.(https://github.com/sgl-project/sglang/blob/25e38216b6af2472949f17b1ec0f4e476768d334/sgl-kernel/Dockerfile#L28-29)This has a few drawbacks:
There is also an ongoing Docker-layer attempt in #18800, but that still addresses the symptom at the container/environment level rather than in the build configuration of the affected target.
Modifications
This PR moves the workaround to the correct layer by applying a minimal CMake-side fix:
mscclppsubdirectory, try to locate CCCL via:find_package(CCCL QUIET PATHS "${CUDAToolkit_LIBRARY_DIR}/cmake/cccl")CCCLis found and the CUDA major version is greater than 12, linkCCCL::CCCLtomscclpp_objThis keeps the fix scoped to the actual dependency that needs it, and makes both Docker and non-Docker source builds use the same build logic.
Why this approach
Upstream
mscclpphas already addressed this in commit51a86630ff8ffafd9b044581b383cf26658b4e6f("Build fixes (#696)"), which adds CUDA 13 CCCL handling in CMake.However, the pinned
mscclpprevision insgl-kernelis significantly older. Bumping the dependency directly would pull in a much larger set of upstream changes and would require broader validation.Instead, this PR backports only the relevant CUDA 13 CCCL CMake fix in a minimal and readable form. This keeps the diff small, limits behavioral change, and aligns with the upstream direction without taking on the risk of a full dependency upgrade.
Scope and impact
CCCL::CCCLis available.mscclpprevision.Once
sgl-kernelupgrades to anmscclppversion that already includes proper CUDA 13 support, this local backport may become unnecessary.Scope and impact
CCCL::CCCLis available.mscclpprevision.Once
sgl-kernelupgrades to anmscclppversion that already includes proper CUDA 13 support, this local backport may become unnecessary.Accuracy Tests
N/A. This PR only changes build configuration and does not modify kernel math or runtime execution logic.
Benchmarking and Profiling
N/A. This PR does not change runtime code paths or performance-sensitive kernels.
References
https://nvidia.github.io/cccl/unstable/cccl/3.0_migration_guide.html
mscclppCUDA 13 CMake fix:microsoft/mscclpp@51a8663
Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ci