ggml-cuda: stop scheduling compute onto the host buffer on HIP integrated GPUs - #145
ggml-cuda: stop scheduling compute onto the host buffer on HIP integrated GPUs#145danielhanchen wants to merge 1 commit into
Conversation
…ated GPUs Carries ggml-org#25863. On HIP the device info sets integrated = prop.integrated, while the CUDA branch of the same #if keeps it false 'due to issues with corrupted output'. That asymmetry re-enables direct host-buffer compute on APUs, which is the path the gfx1151 corruption reports bisect to. Pinned staging stays available; only compute directly out of the host buffer is refused.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Closing this in favour of #147. I had the shipping model wrong when I opened it. The nightly tree is the upstream tag plus #147 pins ggml-org#25863 itself, which is the same change, open and MERGEABLE upstream at The evidence and the reasoning are all restated on #147. |
Overview
Carries ggml-org#25863 so our ROCm prebuilts stop scheduling compute onto the pinned host buffer on HIP integrated GPUs.
The asymmetry is one
#ifinggml-cuda.cu:CUDA disables the flag because it produced corrupted output. HIP does not. On an APU that turns on a host-buffer path where the scheduler can place a compute input in pinned host memory, and an H2D input write can then race a graph that is still running.
Why now
This is the defect behind the largest group of Strix Halo reports, and all of them are on builds we ship:
c7d87229("ggml-cuda : restore prop.integrated on HIP builds", ggml-org#24233), with a clean parent, on gfx1151. Symptom:-np 4 --kv-unifiedreturns another request's response verbatim.The rule those threads established is that corruption tracks
n_ubatch < n_batchand is clean whenever the whole prompt lands in a single ubatch. Makingintegratedruntime-switchable on an otherwise unmodified tree gives PPL 850,121 with it on and 72.80 with it off, from the same binary.Two points worth carrying into how we test this. One reporter ran a greedy completion canary clean for forty minutes against a backend that was numerically broken throughout, so tok/s and HTTP 200 are not evidence of correctness here. The same reporter measured Vulkan at 24% faster decode on this chip, so there is no throughput argument for staying on the ROCm build while this is open.
What the change does
Three call sites, matching ggml-org#25863:
ggml_backend_cuda_host_buffer_supported()factors out the existingGGML_CUDA_NO_PINNEDtest.ggml_backend_cuda_device_supports_cuda_host_buft()returns false for HIP integrated devices.ggml_backend_cuda_device_supports_buft()consults it, so the scheduler stops placing compute inputs on the host buffer type.Pinned host memory stays available for staging. Only compute directly out of it is refused, which is the narrowest change that closes the reports. Discrete HIP and every CUDA path are untouched, since
integratedis already false there.Alternatives considered
ggml-org#27311 is the general fix, a ring buffer for input tensors that makes host buffers correct rather than unavailable. It is the better end state and is what we should follow, but it is a scheduler change with four pending reviews and no approvals. ggml-org#25863 is small enough to carry now and is measured at PPL 63.67 against a Vulkan reference of 64.10 where the base gives 872,006.
Verification
Not built or measured here, and I want to be plain about that. The gfx1151 CI runner available to me has no compiler and this needs the ROCm toolchain, so the prebuilt legs in this repo are the first real check. Before merging I would want:
llama-perplexityover wikitext-2 on gfx1151 at-b 2048 -ub 512, before and after, expecting the base to be the broken number and the head to match a Vulkan reference,-np 4 --kv-unifiedrun against Eval bug: server -np 4 --kv-unified returns other requests' responses verbatim on integrated HIP GPU (gfx1151) — bisected to c7d87229 ggml-org/llama.cpp#25992's nonce-checked harness.I can drive the second and third on the Strix Halo runner once there is a build.