feat(windows): add Vulkan GPU detection for Intel Arc and other non-CUDA GPUs#930
feat(windows): add Vulkan GPU detection for Intel Arc and other non-CUDA GPUs#930darthcav wants to merge 2 commits into
Conversation
…UDA GPUs CanUseGPU on Windows only checked NVIDIA CUDA (amd64) and Qualcomm Adreno OpenCL (arm64), so Intel Arc and other Vulkan-capable GPUs were silently ignored and fell back to the CPU llama.cpp variant. Add hasVulkanCapableGPU (PCI-based, excludes NVIDIA and Adreno which are handled by their own backends) and hasVulkan (probes vulkan-1.dll, mirroring the existing OpenCL.dll probe). Update CanUseGPU to call hasVulkan on amd64 when no CUDA GPU is found, and wire up a "vulkan" variant in ensureLatestLlamaCpp with priority CUDA > Vulkan > CPU. A TODO marks the point where a "vulkan" image variant of docker/docker-model-backend-llamacpp needs to be published to Docker Hub to complete the end-to-end fix. Fixes docker#925 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
hasVulkanCapableGPU, accessinggpu.DeviceInfo.Vendor.Nameandgpu.DeviceInfo.Product.Nameassumes these nested fields are always non-nil; consider adding nil checks to avoid panics on unexpectedghwdata. - The Adreno/Qualcomm detection in
hasVulkanCapableGPUis case-sensitive while the vendor comparison is lowercased; normalizingproduct(and possibly using vendor IDs) would make the heuristic more robust and avoid misclassifying GPUs as Vulkan-capable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `hasVulkanCapableGPU`, accessing `gpu.DeviceInfo.Vendor.Name` and `gpu.DeviceInfo.Product.Name` assumes these nested fields are always non-nil; consider adding nil checks to avoid panics on unexpected `ghw` data.
- The Adreno/Qualcomm detection in `hasVulkanCapableGPU` is case-sensitive while the vendor comparison is lowercased; normalizing `product` (and possibly using vendor IDs) would make the heuristic more robust and avoid misclassifying GPUs as Vulkan-capable.
## Individual Comments
### Comment 1
<location path="pkg/inference/backends/llamacpp/gpuinfo_windows.go" line_range="111-113" />
<code_context>
+ if err != nil {
+ return false, err
+ }
+ for _, gpu := range gpus.GraphicsCards {
+ vendor := strings.ToLower(gpu.DeviceInfo.Vendor.Name)
+ product := gpu.DeviceInfo.Product.Name
+ isNVIDIA := vendor == "nvidia"
+ isAdreno := strings.Contains(product, "Adreno") || strings.Contains(product, "Qualcomm")
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against nil DeviceInfo/Product to avoid potential panics from ghw.GPU()
ghw may return GraphicsCards where `DeviceInfo`, `Vendor`, or `Product` is nil. Directly accessing `gpu.DeviceInfo.Vendor.Name` and `gpu.DeviceInfo.Product.Name` can therefore panic. Please add nil checks (e.g., skip entries with missing DeviceInfo/Product) before dereferencing these fields.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces Vulkan GPU detection for Windows amd64 systems as a fallback when CUDA is not available. It includes logic to identify Vulkan-capable hardware and verify the presence of the Vulkan runtime library. A critical review comment identifies potential nil pointer dereferences in the hardware discovery logic, recommending defensive checks to ensure stability.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@darthcav go may not be your coding language but this PR is perfectly clean :) The important thing is that you tested and confirmed this worked. Did you test it? |
|
@ericcurtin The problem is that I cannot do a full compilation in my laptop and check it against the local GPU. I would need someone to compile that... |
Well you could also use the coding agent you used for this to figure out how to build it. I don't have an Intel Arc machine, so even if I wanted to, I can't test this. |
Summary
Fixes #925 — Docker Model Runner does not use Intel Arc GPU via Vulkan on Windows.
Root cause
CanUseGPUingpuinfo_windows.goonly detects NVIDIA CUDA (amd64) and Qualcomm Adreno OpenCL (arm64). There is no Vulkan path, so Intel Arc and other Vulkan-capable GPUs (AMD without CUDA, etc.) are silently ignored and fall back to thecpullama.cpp variant.Changes
gpuinfo_windows.gohasVulkanCapableGPU()— uses PCI enumeration (viaghw) to find GPUs that are neither NVIDIA nor Adreno; Intel Arc, AMD GPUs, and similar fall into this category.hasVulkan()— probesvulkan-1.dllviasyscall.LoadLibrary, mirroring the existinghasOpenCL/OpenCL.dllpattern. Returnstrueonly when a Vulkan-capable GPU is present and the Vulkan runtime is loadable.CanUseGPU()— on amd64, after the CUDA check, now also callshasVulkan().hasNVIDIAGPU(),hasSupportedAdrenoGPU(),hasVulkanCapableGPU()— added nil guards forDeviceInfo,Vendor, andProductfields before dereferencing.Name, preventing panics whenghwfails to retrieve full PCI details for a card.download_windows.gocanUseVulkandetection inensureLatestLlamaCpp.desiredVariant = "vulkan"when Vulkan is detected.Known limitation / follow-up required
No
vulkanimage variant ofdocker/docker-model-backend-llamacppcurrently exists on Docker Hub (tags available:cpu,cuda,opencl,rocm,metal,generic). When thevulkantag is not found,downloadLatestLlamaCpplogs a warning and falls back to the vendored Docker Desktop binary — which already shipsggml-vulkan.dll. A TODO comment indownload_windows.gomarks this gap.A follow-up task to publish
docker/docker-model-backend-llamacpp:latest-vulkan(a Windows build with the Vulkan backend compiled in) is needed to complete the end-to-end fix.Test plan
GOOS=windows GOARCH=amd64✓go test ./pkg/inference/backends/llamacpp/...) ✓docker model run ai/smollm2 "Test"should show Vulkan backend in logs once thevulkanDocker Hub image variant is published.🤖 Generated with Claude Code