fix: Windows (MSVC) native and CUDA builds - #1
Conversation
Three build-script fixes, each a hard build failure on Windows: - canonicalize_llama_cpp_dir returned \?\-prefixed verbatim paths, which cl.exe cannot resolve in -I include dirs (fatal error C1083 on ggml-backend.h). Strip the verbatim prefix after canonicalizing. - The target_os link-lib match had no windows arm; ggml-cpu's CPU detection calls RegOpenKeyExA/RegQueryValueExA/RegCloseKey, leaving three unresolved __imp_Reg* externals at link time. Link advapi32. - emit_cuda_link_flags was Linux-only and its lib set (cudart_static/culibos/ dl/rt/pthread) is Unix-specific, so --features cuda compiled every kernel then failed to link. Add a Windows variant emitting the cudart/cublas/ cublasLt/cuda import libs plus the lib\x64 search path, which cuda_library_dir misses when the bare lib parent exists. Validated on Windows 11 (MSVC 14.51, CUDA 12.2, RTX 4090) against mainline llama.cpp b10054: CPU and CUDA builds link and run; CPU vs GPU eval results on Bonsai-27B-Q1_0 are identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
canonicalize() can return \\?\UNC\server\share\...; stripping only \\?\ left a relative UNC\... path. Map that form to \\server\share\... and plain \\?\C:\... to C:\... so MSVC/CMake include paths stay absolute.
smanaton
changed the title
bitnom
left a comment
bitnom
left a comment
There was a problem hiding this comment.
Please address these before merge:
-
build.rs:255-258unconditionally linkscublasLton Windows. Current llama.cpp only linksCUDA::cudartandCUDA::cublasfor the Windows static backend, and CUDA 10.0 does not provide cuBLASLt. This can causeLNK1104on an otherwise supported toolkit. Omit the WindowscublasLtlink or gate it on a verified toolkit capability/version. -
build.rs:246-248assumes anx64subdirectory whenever the discovered CUDA directory has one. The build script already receives the target architecture, but this path is not architecture-aware and can select the wrong import-library directory for Windows ARM64. Either select the directory from the target architecture or explicitly reject unsupported Windows CUDA targets. -
Please add focused automated coverage for the Windows path/link decisions, including CPU-only MSVC and CUDA-enabled MSVC, and at least the path normalization cases. The current validation is a single local x64 environment and there are no CI checks on this PR.
The advapi32 addition and drive/UNC normalization are directionally correct, but these issues affect whether the foundational build script works across supported Windows/CUDA environments.
|
Superseded by draft replacement PR #3: #3 The corrected implementation is on the upstream |
|
Closing as superseded by draft PR #3. The corrected work remains on the |
Summary
Three
build.rsfixes — each a hard build/link failure on Windows:canonicalize()returns\\?\...paths that MSVCcl.execannot use in-Idirs. Strip carefully:\\?\C:\foo→C:\foo\\?\UNC\server\share→\\server\sharecudart,cublas,cublasLt,cuda) pluslib\x64search path when CUDA is enabled.Test plan
Notes
Fork staging: smanaton#1
Recommend merging before the prefer-mainline-llama PR.