Repository navigation
ggml-cuda : Minimal pr 27311 version - #29450
mapatel-amd wants to merge 1 commit into
Conversation
Assisted-by: Claude
github-actions
Bot
added
ggml
|
Here are some relevant tables with results from tests I performed using Claude. The markdown for the tables was made using Claude, but the table explanations themselves were written myself: Hardware: Strix Halo gfx1151 (Radeon 8060S, 128 GB), ROCm 10.0, HIP backend. Base master 171e884.
Perplexity (wikitext-2, 4 chunks)
This shows how well the model is predicting, lower is better. In this case, the score seen on master is the expected score and with only the
This is just llama.cpp's own tests, they are now all passing. llama-server needle test (Qwen3-30B-A3B Q4_K_M,
Another test where we hide a secret code in a long document and ask the model to find it, 120 times, with 4 conversations running in parallel. Master was already failing on one of the tests, which is the model's own fault. We were able to get this back up to 119 with the sync. Speed, experts on CPU (Qwen3-30B-A3B Q4_K_M,
Testing the speed improvements with this PR when part of the model sits on the CPU. Fair bit of improvement imo. Speed, full offload (Qwen3-30B-A3B Q4_K_M,
Testing the speed when the model is fully on the GPU. Master and this PR as essentially the same and the differences are due to noise. Speed, long context (Qwen3.5-122B-A10B UD-Q4_K_XL,
Long context testing with about 98,000 tokens worth of context, about ~9-10% improvement. Discrete GPU check (RX 7900 XTX gfx1100, Qwen3-0.6B F16,
The sync that we do is only for iGPUs/APUs. So realistically it shouldn't make a difference for dGPUs and in this case, we can see that it doesn't.
Tested all the math operations in llama.cpp, they all passed. Comparison with #27311Same hardware, ROCm 10.0. Base
Size
My changes are far more minimal. Though I did extensive testing on AMD hardware, I have NOT done any testing on NVIDIA hardware. I may need assistance with this. Perplexity (wikitext-2, 4 chunks)
#27311 gives identical results to my fix. Speed, experts on CPU (Qwen3-30B-A3B Q4_K_M,
The differences in speed when part of model is on CPU is very minimal, can be attributed to noise. Speed, full offload (Qwen3-30B-A3B Q4_K_M,
When the model is fully on the GPU, it's the same thing, very minimal noise which can be attributed to noise. Speed, small model (Qwen3-0.6B F16,
Even for smaller models, this pr's fix has minimal speed difference. Race detector (#27311's
These results show that my sync can possibly replace the ring buffer fixes. #27311's other scheduler changes, with this fix's sync in place (#27311 head + this fix's sync, ring off,
Either way with pinning on and off, it made little to no difference. It turns out pinning only protects the same inputs this pr's sync already protects, so it's redundant. Race detector under llama-server load (Qwen3-30B-A3B Q4_K_M,
Without my sync, we have far more races when doing the needle tests, thus resulting in more failing needle tests. |
|
|
||
| // the previous graph_compute_async may still be reading the inputs that are about to be overwritten | ||
| if (sync_host_inputs) { | ||
| ggml_backend_sched_synchronize(sched.get()); |
There was a problem hiding this comment.
This likely breaks pipeline parallel on many setups (performance reduction).
As stated in #27311 (comment) , supporting prop.integrated is low priority and there are very high barriers to get something like merged, due to the complexity of the scheduling code and the need to test many many setups. (dGPUs, UMA devices, various multi-GPU setups).
So before this moves forward, expect #27258 and #25319 to likely be first in line.
Assisted-by: Claude
Overview
The CPU and GPU in APUs share memory. When llama.cpp knows the device is an iGPU, it lets GPU read the model's inputs directly from the CPU instead of copying those inputs to the GPU's memory first. The problem with this is that the GPU's work is queued asynchronously, so when llama.cpp starts writing the next chunk's worth of inputs, it overwrites the previous inputs. This can happen while the GPU is in the process of reading those previous inputs, which produces very bad outputs, leading the PPL to be pretty high.
We suspected AMD's backend to be the issue because the HIP_LAUNCH_BLOCKING=1 flag makes the CPU wait after each GPU launch, forcing the CPU to wait and thereby preventing the overwrite. The HIP runtime, amdgpu driver and firmware weren't at fault, and this issue wasn't just showing up in APUs. It showed up in a gfx1036(iGPU)+gfx1100(dGPU) setup too.
The fix I have is that, first, we turn the integrated flag back on for HIP. Second, when llama.cpp sets up, check whether any GPU can read the CPU input buffer directly. Only in that case, wait for the previous GPU step to finish before writing new inputs. This concept already exists in llama.cpp as it is used in multi-GPU pipelines. This will also significantly speed up things compared to just setting the blocking flag.
Additional information
The goal of this PR is to be a minimal version of #27311.
Requirements