fix(cmake): improve CUDA C++ standard for compatibility with gcc-14 #5036
Conversation
…gcc-14 Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
There was a problem hiding this comment.
Pull Request Overview
This PR addresses NVCC compilation errors when using gcc-14 as the host compiler with C++11 by adjusting the CUDA C++ standard requirements. The changes ensure that the CUDA standard is set to at least C++14 when gcc-14 or higher is used, and uses set_if_higher to prevent downgrading the C++ standard when multiple requirements exist.
- Adds a check for gcc-14+ to require CUDA C++14 standard minimum
- Replaces direct assignment with
set_if_higherto prevent downgrading C++ standards - Includes documentation link to the known issue
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 Walkthrough<source_lib_src_gpu_cmakeLists_txt> WalkthroughIntroduces a conditional to raise CUDA C++ standard to 14 when the host C++ compiler is GCC/clang version >= 14, using set_if_higher to avoid lowering an already-set standard. Alters CUDA 13.0+ path to set the CUDA C++ standard using set_if_higher(17) instead of unconditionally setting to 17, preserving higher standards if already specified. Changes
<estimated_code_review_effort> Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
</estimated_code_review_effort> Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
source/lib/src/gpu/CMakeLists.txt (2)
48-59: Clarify the message when usingset_if_higher().The status message at line 57 ("setting C++ standard to 17") may be misleading since
set_if_higher()is non-deterministic—it only sets the standard to 17 if the current value is lower. If already set to 14 by the GCC-14 check (line 49), the message would be inaccurate.Consider updating the message to reflect the actual behavior:
if(CMAKE_CUDA_COMPILER_VERSION VERSION_GREATER_EQUAL "13.0") set_if_higher(CMAKE_CUDA_STANDARD 17) message( STATUS - "CUDA ${CMAKE_CUDA_COMPILER_VERSION} detected, setting C++ standard to 17" + "CUDA ${CMAKE_CUDA_COMPILER_VERSION} detected, C++ standard set to at least 17" ) endif()
52-59: Clarify the status message when usingset_if_higher().The status message at line 57 ("setting C++ standard to 17") may be inaccurate since
set_if_higher(CMAKE_CUDA_STANDARD 17)is conditional—it only raises to 17 if currently lower. If GCC-14 already set it to 14 (line 49), the message becomes misleading.Update the message to reflect actual behavior:
if(CMAKE_CUDA_COMPILER_VERSION VERSION_GREATER_EQUAL "13.0") set_if_higher(CMAKE_CUDA_STANDARD 17) message( STATUS - "CUDA ${CMAKE_CUDA_COMPILER_VERSION} detected, setting C++ standard to 17" + "CUDA ${CMAKE_CUDA_COMPILER_VERSION} detected, C++ standard set to at least 17" ) endif()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/lib/src/gpu/CMakeLists.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T11:37:10.532Z
Learnt from: CR
Repo: deepmodeling/deepmd-kit PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-18T11:37:10.532Z
Learning: Applies to source/CMakeLists.txt : Keep C++ build configuration up to date in CMake when changing C++ components (library, ops, APIs)
Applied to files:
source/lib/src/gpu/CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: Test C++ (false)
- GitHub Check: Test Python (3, 3.9)
- GitHub Check: Test Python (6, 3.12)
- GitHub Check: Test Python (5, 3.12)
- GitHub Check: Test Python (3, 3.12)
- GitHub Check: Test Python (1, 3.12)
- GitHub Check: Test Python (5, 3.9)
- GitHub Check: Test Python (6, 3.9)
- GitHub Check: Test Python (4, 3.12)
- GitHub Check: Test Python (2, 3.12)
- GitHub Check: Test Python (4, 3.9)
- GitHub Check: Test Python (2, 3.9)
- GitHub Check: Test Python (1, 3.9)
- GitHub Check: Test C++ (true)
- GitHub Check: Build C++ (cuda, cuda)
- GitHub Check: Build C++ (rocm, rocm)
- GitHub Check: Build C++ (clang, clang)
- GitHub Check: Analyze (python)
- GitHub Check: Build C++ (cpu, cpu)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Build C++ (cuda120, cuda)
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build wheels for cp310-manylinux_aarch64
- GitHub Check: Build wheels for cp311-win_amd64
- GitHub Check: Build wheels for cp311-macosx_arm64
- GitHub Check: Build wheels for cp311-macosx_x86_64
- GitHub Check: Build wheels for cp311-manylinux_x86_64
- GitHub Check: Build C library (2.18, libdeepmd_c.tar.gz)
- GitHub Check: Build C library (2.14, >=2.5.0,<2.15, libdeepmd_c_cu11.tar.gz)
🔇 Additional comments (4)
source/lib/src/gpu/CMakeLists.txt (4)
46-59: Approved: GCC-14 compatibility fix is well-structured.The approach to fix GCC-14 + NVCC incompatibility by conditionally raising the C++ standard is sound. The logic flow (11 → 14 for GCC-14 → 17 for CUDA 13.0+) using
set_if_higherto preserve higher standards is appropriate.Verification needed:
- Confirm
set_if_higher()is defined in the project's CMake infrastructure.- Test the build on a GCC-14 + CUDA 11 or 12 environment to ensure no regressions.
46-59: Approved: GCC-14 compatibility fix logic is sound.The approach to address NVCC compilation errors with GCC-14 is well-structured. Conditionally raising the C++ standard (11 → 14 for GCC≥14 → 17 for CUDA≥13.0) using
set_if_higherto preserve already-set higher standards is the right strategy.This aligns with the learning to keep C++ build configuration up-to-date in CMake. Based on learnings.
46-50: No issues found—set_if_higher()is properly defined and correctly used.The custom macro
set_if_higher()is defined atsource/CMakeLists.txt:21and implements the conservative logic of only setting a variable if the new value is higher. The code at lines 46-50 correctly applies this macro to handle the GCC-14 compatibility issue, following the same pattern used elsewhere in the codebase.
46-50: Verification complete—no issues found.The
set_if_higher()macro is properly defined insource/CMakeLists.txt:21-28as a custom CMake function that only increases values when they are LESS than the new value, preventing downgrades. The usage at lines 49 and 54 is correct, and the message accurately reflects the behavior: when CUDA 13.0+ is detected, the standard is set to 17 (or kept higher if already set). The conditional logic flows correctly from the initial standard of 11 to 14 (if GCC ≥ 14) to 17 (if CUDA ≥ 13.0).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devel #5036 +/- ##
==========================================
- Coverage 84.23% 84.23% -0.01%
==========================================
Files 709 709
Lines 70073 70075 +2
Branches 3620 3618 -2
==========================================
- Hits 59029 59027 -2
- Misses 9879 9881 +2
- Partials 1165 1167 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…eepmodeling#5036) NVCC compilation errors with gcc-14 and C++11. Cases in other repos: https://gitlab.archlinux.org/archlinux/packaging/packages/cuda/-/issues/12 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved GPU build configuration to better support newer compiler versions (GCC/Clang 14+) and preserve user-specified C++ standards during compilation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
NVCC compilation errors with gcc-14 and C++11.
Cases in other repos: https://gitlab.archlinux.org/archlinux/packaging/packages/cuda/-/issues/12
Summary by CodeRabbit