Skip to content

fix: gate kv_inplace_write GPU copy on a GPU backend build - #2113

Merged
inureyes merged 2 commits into
mainfrom
fix/issue-2108-cpu-link-kv-inplace
Oct 5, 2026
Merged

inureyes merged 2 commits into
mainfrom
fix/issue-2108-cpu-link-kv-inplace

Conversation

@inureyes

@inureyes inureyes commented Oct 5, 2026

Copy link
Copy Markdown
Member

Summary

kv_inplace_write.cpp is compiled for every backend, but its eval_gpu calls mlx::core::copy_gpu_inplace, which MLX only defines when it builds a GPU backend, so a Linux build with no GPU feature failed to link. build.rs now defines MLXCEL_BRIDGE_GPU_BACKEND when the resolved Metal backend, the cuda feature or the rocm feature holds (gpu_backend_enabled in build_support/metal_backend.rs, with unit tests). The copy.h include and the eval_gpu body sit behind it; without it eval_gpu throws like eval_cpu. That throw is unreachable: every Rust caller and inplace_slice_write itself already require a GPU default device. The existing Metal and ROCm defines were not reused because the condition is their union and CUDA had none. macOS with MLXCEL_BUILD_METAL=OFF had the same failure and is covered by one of the new tests.

Validation (GB10)

  • CPU-only, separate target/cpu: cargo test -p mlxcel-core --profile test-fast --lib --no-run links (2m24s cold, run beside a sibling CUDA build; 6s warm). Restoring main's .cpp reproduces the failure: two undefined reference to copy_gpu_inplace, nothing else. cargo build --release links (9m48s cold). inplace (4) and build-policy (7) tests pass.
  • CUDA: cargo build --release --features cuda --bin mlxcel-server builds. Under gpu-lock with --test-threads=1, mlxcel-core lib filters inplace (4), cache:: (541) and metal_backend_build_policy (7) pass; the full lib suite was not run. The CUDA kv_inplace_write.o still references copy_gpu_inplace and lacks the CPU-only error string; the CPU-only object is the reverse.
  • cargo clippy --release --features cuda --lib --tests -- -D warnings and cargo fmt --check are clean.
  • Metal and ROCm cannot be built here. Both already link main's unconditional file (Metal is the primary platform; update(rocm): port paged attention (v1, v2 partial, merge) to HIP #2103 ran on gfx1151 with it), and both set the new define, so their preprocessed source is unchanged.

CI

ci.yml is untouched because #2111 edits it in the same run. Recommended for #2111: a GB10 step with its own persistent target dir (pattern at ci.yml:657-661) running cargo test -p mlxcel-core --profile test-fast --lib --no-run, about 2.5 min cold and 6s warm. Until then the command is in docs/installation.md ("CPU-only link check").

Closes #2108

@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:medium Medium priority area:core mlxcel-core: MLX FFI, primitives, KV cache, layers platform:linux Linux (CUDA / packaging) specific status:review Under review status:done Completed and removed status:review Under review labels Oct 5, 2026
A Linux build with no cuda, metal or rocm feature failed to link: InplaceSliceWrite::eval_gpu in kv_inplace_write.cpp calls mlx::core::copy_gpu_inplace, which MLX defines only when it builds a GPU backend. No CI job links that configuration.

build.rs now defines MLXCEL_BRIDGE_GPU_BACKEND when the resolved Metal backend, the cuda feature or the rocm feature holds (gpu_backend_enabled in build_support, with unit tests). The copy.h include and the eval_gpu body sit behind it; without it eval_gpu throws like eval_cpu. Every Rust caller already requires a GPU default device, so the throw is unreachable. GPU builds compile the same source as before.

docs/installation.md documents the CPU-only link check command and its measured cost.

Refs #2108
@inureyes
inureyes force-pushed the fix/issue-2108-cpu-link-kv-inplace branch from 1d32937 to 925881a Compare October 5, 2026 11:29
@inureyes
inureyes merged commit 6ddbb57 into main Oct 5, 2026
25 checks passed
@inureyes
inureyes deleted the fix/issue-2108-cpu-link-kv-inplace branch October 5, 2026 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core mlxcel-core: MLX FFI, primitives, KV cache, layers platform:linux Linux (CUDA / packaging) specific priority:medium Medium priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: CPU-only Linux build fails to link kv_inplace_write

1 participant