Repository navigation
ggml : backport persistent view initialization after buffer-size splits - #37
Conversation
When ggml_backend_alloc_ctx_tensors_from_buft splits allocation on buft max_size, a view-only tail at the end of the context could skip the final alloc_tensor_range. Persistent views (e.g. KV k_stream / v_stream) were then left without ggml_backend_view_init. Allocate only parent tensors in alloc_tensor_range and initialize all views in a final pass over the context after all splits complete. (cherry picked from commit 335c0989ed71e0207b212da063a40a5845a00582)
Cover the buft max_size split path where a view-only tail would skip ggml_backend_view_init without the finalize pass. (cherry picked from commit 0074731c9a0ee2b453fe222d59a4aa24ad9cb696)
|
Wouldn’t this be picked up by an upstream rebase? |
There was a problem hiding this comment.
Validation: Release test-alloc passed, including test_view_init_after_max_size_split.
Scope: this is a general allocator correctness backport, not a gfx1151 change, and the branch is behind master. Please land/stage it in halo-box/llama.cpp/upstream (or rebase if master already contains it), then sync the Strix fork instead of carrying a permanent local divergence.
comment generated by my clanker Codex
dzannotti
left a comment
dzannotti
left a comment
There was a problem hiding this comment.
Verified on Strix Halo (gfx1151). The new test_view_init_after_max_size_split compiled against the base library (7449a0fe9) aborts with GGML_ASSERT(t->buffer != nullptr), and with this PR it passes along with every other test-alloc case. A full test-backend-ops -b ROCm0 passes 14945/14945 on this PR.
Comment generated via automated review of clanker Claude Code.
Keep ROCmFP4 FAST and MXFP4/NVFP4 W4A4 MMQ declarations. Keep explicit rollback registrations, including Qwen4exp, without duplicate directory runs. Combine the status/all-model harness with scaled-weight seq_cp/graph-reuse coverage. Assisted-by: Codex
Overview
Backport Yoshinao Kadowaki's allocator fix and regression from ggml-org#25584. The original authors and cherry-pick provenance are retained:
335c0989ed71e0207b212da063a40a5845a00582->14aa71aa144e5c51660ecf3cf75de375cd568e330074731c9a0ee2b453fe222d59a4aa24ad9cb696->72dbbbd207c3f5ceaaff3177bac821212f612682The test insertion conflicted with newer Halo tests; both sets are retained. No state-I/O skip guards, new checkpoint APIs or unrelated scheduling changes are included. This is not a claim to have authored the upstream fix.
On the current tested Halo base, a large KV parent exceeds the Vulkan buffer type's maximum allocation chunk. A trailing view-only range is left without
ggml_backend_view_init. Saving a displaced conversation then aborts on that nonempty view. The change allocates parents within ranges and initializes views after all ranges have been allocated.Fresh diagnosis on the Radeon 8060S identified:
The view represents live KV data. A
!tensor->dataguard in state I/O would conceal the allocation failure and omit that data; it is not the repair submitted here. The diagnostic interposer only printed these fields and then called the original reader, preserving the abort.Measurements
The baseline and backport were built independently in this session with identical compiler/backend settings and no compiler warnings. Mapped libraries were verified for each test. The subsequent master merge
99a40a3e6changes speculative replay, not this allocation path; it is not included in these measured builds.Baseline / after:
test-allocexecutable with view-tail regressionThe official witness uses the unmodified files in
ggml-org/Qwen3.8-27B-GGUF:Qwen3.8-27B-Q4_K_M.gguf: SHA-25631629f53165ab6a7dad8c9847dcfd1fdf55829dac1e6e748f4a68581b0033d34mtp-Qwen3.8-27B-Q8_0.gguf: SHA-256cbf60a0c48b431bb61f1d49b8948dc88ac29c398d6dbdbbb2e6e89ef77eacc9aThe server tuple was one unified slot with 1,000,000 configured tokens (1,000,192 physical padding), Q8_0 target KV, Q4_0 draft KV, MTP depth 3, batch 2048/512, 32 checkpoints and 16 GiB RAM cache. The requests were a 512-token shallow prompt with 256 generated tokens, a different 5,000-token prefix, then the original shallow prompt. The 256-token cap is an output-identity control, not a completed-task claim.
Relevant server arguments for the allocation and prompt-cache boundary:
Correctness:
test-allocincludes the upstream small dummy-backend reproducer, so reaching the view-tail bug does not require a large model. All 17 tests pass on the backport; the same executable aborts at the new view check when linked to the baseline libraries.A separate official-Qwen Vulkan control used each of the repository's prose, code, structured and numeric corpora, repeated/truncated to 512 tokens, followed by eight greedy steps. All nine complete logit rows, prompt IDs and selected IDs were byte-identical before and after for each corpus, with Q8_0 KV and identical settings. The model processes exited normally and were confirmed gone.
This backport makes no throughput claim.
llama-bench, perplexity and the full backend-op suite were not run. The evidence establishes the allocator view-init regression, actual affected Vulkan state save/recovery and bounded output preservation, not global backend qualification.Requirements