Skip to content

Fix CPU-only build by skipping CUDA, rmm, raft deps#146

Open
hcho3 wants to merge 6 commits into
rapidsai:mainfrom
hcho3:fix_cpu_only_build
Open

Fix CPU-only build by skipping CUDA, rmm, raft deps#146
hcho3 wants to merge 6 commits into
rapidsai:mainfrom
hcho3:fix_cpu_only_build

Conversation

@hcho3

@hcho3 hcho3 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Closes #145

nvForest uses RMM and RAFT as dependencies, but they are not needed when building nvForest in the CPU-only mode.

@hcho3 hcho3 requested a review from a team as a code owner June 13, 2026 01:18
@hcho3 hcho3 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jun 13, 2026
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 75615fc5-d158-4a51-8b19-02486a711a3e

📥 Commits

Reviewing files that changed from the base of the PR and between 3cfaed4 and edf0dcc.

📒 Files selected for processing (1)
  • cpp/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (1)
  • cpp/CMakeLists.txt

📝 Walkthrough

Summary by CodeRabbit

  • Chores

    • GPU (CUDA) support is now optional: CUDA toolchain/runtime configuration and GPU-specific compilation settings are applied only when GPU support is enabled, keeping CPU-only builds free of CUDA requirements.
    • The core library’s GPU runtime linkage is now conditionally included for GPU builds, while common logging support remains available.
  • Tests

    • Test build/link settings are GPU-aware: GPU runtime libraries and CUDA language requirements are applied only for GPU-enabled builds.
    • GPU-specific test executables are created only when GPU support is enabled.

Walkthrough

CMake build files now gate all CUDA initialization, runtime setup, third-party dependencies, and GPU-specific target properties behind the NVFOREST_ENABLE_GPU flag. This enables CPU-only builds to avoid CUDA language requirements and GPU library linkage.

Changes

Conditional CUDA build support

Layer / File(s) Summary
Conditional project CUDA initialization
cpp/CMakeLists.txt
CUDA is removed from initial project() language list; CUDA architecture initialization and language enablement are made conditional on NVFOREST_ENABLE_GPU, preventing CUDA language setup in CPU-only builds.
CUDA runtime and third-party dependency setup
cpp/CMakeLists.txt
CUDA runtime initialization, CUDAToolkit discovery (ConfigureCUDA.cmake), ccache launcher for CUDA, and RMM/RAFT third-party fetching are gated behind NVFOREST_ENABLE_GPU, skipping GPU setup when not needed.
nvforest++ library CUDA properties and linking
cpp/CMakeLists.txt
Library target CUDA properties (CUDA_STANDARD, CUDA_STANDARD_REQUIRED) and GPU library linkage (rmm::rmm, raft::raft, CUDA::cudart_static) are applied only when GPU is enabled; position-independent code remains unconditional.
Test target CUDA configuration
cpp/tests/CMakeLists.txt
Test linking conditionally adds GPU dependencies only for GPU builds; test language standard properties (CUDA_STANDARD) are applied only when NVFOREST_ENABLE_GPU is true; DEVICE_BUFFER_TEST executable is registered only for GPU-enabled builds.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • rapidsai/nvforest#128: Both PRs modify the CUDA/C++ language-standard CMake wiring in cpp/CMakeLists.txt and cpp/tests/CMakeLists.txt—main PR changes how/when CUDA_STANDARD/CUDA_STANDARD_REQUIRED are applied under NVFOREST_ENABLE_GPU, while retrieved PR changes those standards' values to C++20/CUDA 20.

Suggested reviewers

  • dantegd
  • bdice
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing CPU-only builds by making CUDA, RMM, and RAFT dependencies conditional on GPU enablement.
Description check ✅ Passed The description directly addresses the changeset by explaining that RMM and RAFT dependencies are skipped in CPU-only mode, which aligns with the file changes that gate these dependencies on NVFOREST_ENABLE_GPU.
Linked Issues check ✅ Passed The code changes fully address issue #145 by conditionally gating CUDA, RMM, and RAFT dependencies on NVFOREST_ENABLE_GPU, enabling CPU-only builds when consumed via CPM.
Out of Scope Changes check ✅ Passed All changes in cpp/CMakeLists.txt and cpp/tests/CMakeLists.txt are directly scoped to conditionally managing CUDA and GPU-related dependencies, with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
cpp/CMakeLists.txt (1)

318-318: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

HIGH: CUDA-specific linker script applied in CPU-only builds

Issue: fatbin.ld contains CUDA-specific ELF sections (.nvFatBinSegment, .nv_fatbin) but is unconditionally passed to the linker.
Why: May cause linker warnings or errors in CPU-only builds when these sections are not recognized.
Impact: Potential build failure or linker noise in CPU-only mode.

Consider making this conditional:

if(NVFOREST_ENABLE_GPU)
  target_link_options(${NVFOREST_CPP_TARGET} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/fatbin.ld")
endif()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/CMakeLists.txt` at line 318, The CMake target_link_options call
unconditionally adds the CUDA-specific linker script fatbin.ld to
${NVFOREST_CPP_TARGET}, which can cause linker warnings/failures in CPU-only
builds; wrap the target_link_options("${CMAKE_CURRENT_BINARY_DIR}/fatbin.ld")
invocation in a conditional that checks the NVFOREST_ENABLE_GPU option (e.g.,
only add the fatbin.ld link option when NVFOREST_ENABLE_GPU is true) so the
CUDA-specific linker script is applied only for GPU-enabled builds.
cpp/tests/CMakeLists.txt (1)

86-86: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

CRITICAL: CUDA test built unconditionally in CPU-only mode

Issue: DEVICE_BUFFER_TEST includes buffer.cu source file but is not gated on NVFOREST_ENABLE_GPU.
Why: In CPU-only builds, CUDA language is not enabled, so CMake cannot compile .cu files.
Impact: Build failure with "No known features for CUDA compiler" or similar error when NVFOREST_ENABLE_GPU=OFF.

Suggested fix:

ConfigureTest(NAME HOST_BUFFER_TEST raft_proto/buffer.cpp)
if(NVFOREST_ENABLE_GPU)
  ConfigureTest(NAME DEVICE_BUFFER_TEST raft_proto/buffer.cu)
endif()
ConfigureTest(NAME FOREST_TRAVERSAL_TEST forest/traversal_forest.cpp)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/CMakeLists.txt` at line 86, The ConfigureTest invocation for
DEVICE_BUFFER_TEST is adding a .cu source unconditionally, which breaks CPU-only
builds; update the CMake so that ConfigureTest(NAME DEVICE_BUFFER_TEST
raft_proto/buffer.cu) is wrapped in an NVFOREST_ENABLE_GPU guard (use the
NVFOREST_ENABLE_GPU variable) and add a CPU fallback (e.g., a HOST_BUFFER_TEST
using raft_proto/buffer.cpp) so CUDA sources are only configured when
NVFOREST_ENABLE_GPU is ON; adjust nearby ConfigureTest calls (e.g.,
FOREST_TRAVERSAL_TEST) accordingly to keep test ordering/coverage.
🧹 Nitpick comments (1)
cpp/CMakeLists.txt (1)

171-175: ⚡ Quick win

MEDIUM: CMAKE_CUDA_COMPILER_LAUNCHER set without CUDA language guard

Issue: Line 174 sets CMAKE_CUDA_COMPILER_LAUNCHER even when CUDA language may not be enabled (CPU-only builds).
Why: Inconsistent with the PR's goal to fully gate CUDA configuration; may cause warnings depending on CMake version.

Consider wrapping in NVFOREST_ENABLE_GPU guard for consistency:

if(USE_CCACHE)
  set(CMAKE_C_COMPILER_LAUNCHER ccache)
  set(CMAKE_CXX_COMPILER_LAUNCHER ccache)
  if(NVFOREST_ENABLE_GPU)
    set(CMAKE_CUDA_COMPILER_LAUNCHER ccache)
  endif()
endif()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/CMakeLists.txt` around lines 171 - 175, The current USE_CCACHE block sets
CMAKE_CUDA_COMPILER_LAUNCHER unconditionally which can trigger warnings for
CPU-only builds; update the block so that CMAKE_CUDA_COMPILER_LAUNCHER is only
set when the CUDA/GPU feature is enabled by guarding it with
NVFOREST_ENABLE_GPU. Locate the USE_CCACHE conditional and keep setting
CMAKE_C_COMPILER_LAUNCHER and CMAKE_CXX_COMPILER_LAUNCHER as-is, but wrap the
CMAKE_CUDA_COMPILER_LAUNCHER assignment in an if(NVFOREST_ENABLE_GPU) ...
endif() so the CUDA launcher is only configured when NVFOREST_ENABLE_GPU is
true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cpp/CMakeLists.txt`:
- Line 318: The CMake target_link_options call unconditionally adds the
CUDA-specific linker script fatbin.ld to ${NVFOREST_CPP_TARGET}, which can cause
linker warnings/failures in CPU-only builds; wrap the
target_link_options("${CMAKE_CURRENT_BINARY_DIR}/fatbin.ld") invocation in a
conditional that checks the NVFOREST_ENABLE_GPU option (e.g., only add the
fatbin.ld link option when NVFOREST_ENABLE_GPU is true) so the CUDA-specific
linker script is applied only for GPU-enabled builds.

In `@cpp/tests/CMakeLists.txt`:
- Line 86: The ConfigureTest invocation for DEVICE_BUFFER_TEST is adding a .cu
source unconditionally, which breaks CPU-only builds; update the CMake so that
ConfigureTest(NAME DEVICE_BUFFER_TEST raft_proto/buffer.cu) is wrapped in an
NVFOREST_ENABLE_GPU guard (use the NVFOREST_ENABLE_GPU variable) and add a CPU
fallback (e.g., a HOST_BUFFER_TEST using raft_proto/buffer.cpp) so CUDA sources
are only configured when NVFOREST_ENABLE_GPU is ON; adjust nearby ConfigureTest
calls (e.g., FOREST_TRAVERSAL_TEST) accordingly to keep test ordering/coverage.

---

Nitpick comments:
In `@cpp/CMakeLists.txt`:
- Around line 171-175: The current USE_CCACHE block sets
CMAKE_CUDA_COMPILER_LAUNCHER unconditionally which can trigger warnings for
CPU-only builds; update the block so that CMAKE_CUDA_COMPILER_LAUNCHER is only
set when the CUDA/GPU feature is enabled by guarding it with
NVFOREST_ENABLE_GPU. Locate the USE_CCACHE conditional and keep setting
CMAKE_C_COMPILER_LAUNCHER and CMAKE_CXX_COMPILER_LAUNCHER as-is, but wrap the
CMAKE_CUDA_COMPILER_LAUNCHER assignment in an if(NVFOREST_ENABLE_GPU) ...
endif() so the CUDA launcher is only configured when NVFOREST_ENABLE_GPU is
true.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2722a09a-162b-48c2-878a-7c01e8a3639f

📥 Commits

Reviewing files that changed from the base of the PR and between 520be40 and a889a16.

📒 Files selected for processing (2)
  • cpp/CMakeLists.txt
  • cpp/tests/CMakeLists.txt

@hcho3

hcho3 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CMake CUDA/C++ improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] nvForest cannot be built with CUDA disabled when consumed via CPM

1 participant