Skip to content

hrx: a view into storage the graph does not hold is skipped, not refused (Qwen3-VL decodes again; engine #1467) - #283

Merged
bong-water-water-bong merged 1 commit into
1bit/hrx-vulkan-patchedfrom
1bit/1467-external-window-view
Nov 10, 2026
Merged

bong-water-water-bong merged 1 commit into
1bit/hrx-vulkan-patchedfrom
1bit/1467-external-window-view

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 10, 2026 •

Copy link
Copy Markdown

Refs 1bit-MONSTER/engine#1467. This removes the failure; it does not put the CLIP graph on HRX (see "What this does not do").

The fault

At the tip, Qwen3-VL-2B fails on HRX0 before any image is involved. The text model's own warmup is refused:

graph_compute: unsupported HRX node 20: VIEW output=34:f32[2048,2,1,1]<-VIEW inputs=[21:f32[2048,2,1,1]] consumers=[21:ADD]
  alias=not_aliasing_input(no_alias_source) storages=out24/in11 nb=[4,32768,65536,65536] offs=8192 inputs=1
graph_compute: ggml_backend_sched_graph_compute_async failed with error -1
llama_decode: failed to decode, ret = -3

So text-only decode of this model fails too (llama-completion -m Qwen3VL-2B-Instruct-Q4_K_M.gguf --device HRX0 -ngl 99: rc 1, no output), and every image request fails with it.

Why

A deepstack model has one wide input embedding, n_embd × 4 values per token. llama.cpp takes the first n_embd values of each token as a view (t_inp_embd), and for each deepstack layer a view of that view, one slice further in, which is added to the layer's output (src/models/qwen3vl.cpp). The node of the first view is in another backend's split, so the wide tensor is never a value of the HRX graph. The deepstack view's bytes run past the end of the first view's, so it cannot alias it either: it gets a storage of its own, it is not an alias of its input, and no registration takes a VIEW. The scheduler refused the graph.

ggml-org#1467's thread planned a contiguous copy for this node. It does not need one. tensor_is_external() already classifies the value External, because its storage root is not a node of the graph, so it is bound from its tensor like any other external input. A layout op writes no bytes, and no node of this graph produces these bytes.

The fix

dispatch-scheduler.cpp: a layout op whose output is External, on a storage of its own, recorded as AliasMiss::NoAliasSource, is skipped. Nothing else changes: a registration is still tried first, and this case was a refusal before, so no graph that planned before plans differently.

A window registered as a transient (a value the graph would have to fill) stays refused; test-hrx-view-alias-elision's hand-built case still pins that, and its comment now says so.

Proof (strixhalo, gfx1151, under gpu-hold)

New tests/test-hrx-external-window-view.cpp. It builds the graph as llama.cpp does, takes the sub-graph without the first view's node (ggml_graph_view), and runs it through the real importer and scheduler. On HRX0 it also compares values with the CPU backend, with data that names slice, token and index.

Before (scheduler as at 2d62cc7af), the same refusal as the device run:

window at slice 1, as imported          : REFUSED - unsupported HRX node 0: VIEW output=1:f32[2048,2,1,1]<-VIEW inputs=[0:f32[2048,2,1,1]] consumers=[1:ADD] alias=not_aliasing_input(no_alias_source) storages=out1/in0 nb=[4,32768,65536,65536] offs=8192 inputs=1
window at slice 2, as imported          : REFUSED - … offs=16384 inputs=1
window at slice 3, as imported          : REFUSED - … offs=24576 inputs=1
test-hrx-external-window-view: 3 failures

After:

window at slice 1, as imported          : plans
window at slice 1, registered transient : REFUSED
window at slice 2, as imported          : plans
window at slice 2, registered transient : REFUSED
window at slice 3, as imported          : plans
window at slice 3, registered transient : REFUSED
window at slice 1 on HRX0 vs CPU         : 0 of 4096 values differ
window at slice 2 on HRX0 vs CPU         : 0 of 4096 values differ
window at slice 3 on HRX0 vs CPU         : 0 of 4096 values differ
test-hrx-external-window-view: 0 failures

The model, Qwen3VL-2B-Instruct Q4_K_M + mmproj-Qwen3VL-2B-Instruct-F16, greedy:

run before after
text, llama-completion --device HRX0 -ngl 99, 12 tokens rc 1, ret = -3 Paris, and the capital of Germany is Berlin. If the, byte-identical to the CPU run
image, llama-mtmd-cli --device HRX0 -ngl 99 (projector offloaded), ggml-org#1467's 448×448 test image rc 1, same refusal A red rectangle and a green circle.
same image, --no-mmproj-offload A red rectangle and a green circle.
same image, all CPU A red rectangle and a green circle.

llama-bench -p 512 -n 64 -r 3 on the text model after the fix: pp512 2,330.54 t/s (sd 358.59), tg64 57.78 t/s (sd 9.21). One run on a shared box; it is here to show the model runs, not as a throughput claim.

What this does not do

🤖 Generated with Claude Code

…sed (engine ggml-org#1467)

A deepstack model (Qwen3-VL) takes each layer's deepstack slice as a view of a view of the wide
input embedding. When the first view's node is in another split, the wide tensor is not a value of
the HRX graph, so the slice cannot alias its input: it has a storage of its own, no registration
takes a VIEW, and the graph was refused at 'unsupported HRX node 20: VIEW ... (no_alias_source)'.
That is the text model's graph, so the model failed on its own warmup, before any image.

The import already classifies such a value External (its storage root is not a node of the graph).
A layout op writes no bytes and no node of the graph produces these, so the scheduler now skips
the node. A window registered as a transient stays refused.

tests/test-hrx-external-window-view.cpp: the plan through the real importer on the sub-graph the
scheduler hands over (3 slices; a transient window refused), and on HRX0 the values against the CPU.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Author

ready for review at e0f6918

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT at e0f6918: READY — a view into storage the graph does not hold is a lifetime bug, and the check belongs where the view is made

The title states the invariant precisely: a view into storage the graph does not hold is skipped. That is the class where a ggml_tensor view_src points at a buffer whose owner has gone out of scope — it produces perfectly plausible numbers until the allocator reuses the memory, at which point it produces a wrong result with no diagnostic. A skip is the right disposition: the computation the view was for cannot be performed, so not performing it is honest, where computing from stale bytes is not.

What I would confirm, and it is the only substantive risk: that "skipped" propagates to the caller as a visible outcome — a status, a warning, an absence in the output — rather than silently yielding an empty or zero tensor. A guard that skips and leaves the output default-initialised is a correctness bug wearing a guard's clothes, and this is exactly the shape of the "narrow to width 1" and "returned the envelope's end" cases elsewhere in this queue: the mechanism is right and the reporting is what fails.

Second thing to confirm: whether the check is against the graph's ownership (does the graph own the storage) or against the tensor's view_src being non-null. Those differ: view_src can be set on a tensor whose storage is still owned, so a check on the pointer alone would skip valid work — a false skip, which is the mirror error and the cheaper one to miss because nothing looks wrong except a missing result.

Third, and this one is a test question: a lifetime bug needs a case that would have failed before, i.e. a graph with a view whose view_src storage is released, asserting the skip; plus a case with a valid view asserting the work still happens. Without the second, an implementation that skipped everything would pass — the same "a check that cannot fail" defect this queue keeps finding.

No objection to the approach. If the skip is currently silent, that is the one thing I would change before merge.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at e0f6918 — "skipped, not refused" is the behaviour this backend's own contract asks for, and the value check against the CPU backend is what separates a fallback from a silent drop.

The distinction that matters: skip here means the scheduler does not claim the node, so it falls back — not that the node is dropped from the graph. The test wording confirms it ("the window must read the slice it names, compared with the CPU backend"), which is the only check that can tell the two apart: a dropped node produces some number, and only a comparison against the CPU's own result says whether it is the right one. That is the difference between this and the failure mode I have been flagging all night.

Why this is right rather than a workaround: the backend's stated design is "claim only nodes the dispatcher can execute, so the rest falls back to another backend instead of failing the graph". A VIEW whose storage root is not a node of the graph is not something this backend can execute — refusing it killed the whole graph and surfaced as unsupported HRX node 20: VIEW … alias=not_aliasing_input, which is precisely the message #258/#262/#267 spent four PRs making legible. So the fix completes that thread: the message now leads to a fallback instead of a dead graph.

Two asks:

  1. count the fallback, don't just permit it. With GGML_HRX_LOG_DISPATCH=1 the node should appear as CPU-side; if there is a fallback census (#1316's gate counts them), confirm it moves by exactly this node and no others — a fix that silently un-claims nodes is indistinguishable from one that un-claims the wrong ones, and the census is the only instrument that sees the difference;
  2. the scoping sentence deserves to be in the title or the first line, not only the body: "this removes the failure; it does not put the CLIP graph on HRX". Someone reading a green result will otherwise assume vision works, and the honest claim — the text model's own warmup no longer dies — is narrower and more useful.

One thing worth keeping in the comment: that the fault fires before any image is involved, on Qwen3-VL-2B's text warmup. That is the fact that makes this a general fix rather than a vision-model one, and it is what someone with a different model in hand needs to recognise their own failure.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at e0f6918 — a view into storage the graph does not hold is skipped (+19 in the scheduler with a test registered): the alternative, mapping it anyway, produces a read of memory nobody promised, and the test is what keeps the skip from becoming a silent drop of a legitimate view.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at e0f6918 — read-only: I did not build it. Disk is at 147 G, under this box's floor for a llama.cpp build plus regenerated kernel corpus, so this is a review of the change and its evidence, not a reproduction.

The narrowing rule is right, and it is narrow in the way that matters. The predicate added is four conditions in one expression — output is External, its storage_root is its own id, its alias_miss is NoAliasSource, and the op is a layout op — so this skips exactly the case where the graph gives the value rather than expecting to fill it. The two arms that make that safe are the ones the PR leads with: window at slice N, registered transient : REFUSED for all three slices, and test-hrx-view-alias-elision's hand-built case still pinning the refusal path. A rule that started eliding transient windows would be a use-after-nothing bug, and the proof shows the boundary is actually a boundary rather than a hope.

The value check is the part I would not have expected and would have asked for: window at slice 1/2/3 on HRX0 vs CPU : 0 of 4096 values differ. Planning is not correctness — an elided view that reads the wrong slice would plan perfectly and produce plausible garbage — so comparing all 4096 values at three offsets (8192, 16384, 24576) against the CPU backend is what turns "it runs" into "it runs the right thing".

Three things in "What this does not do" that are worth their weight:

  1. llama-mtmd-cli on a dev build, not the shipped payload — so the issue's 16.97 s/image is explicitly not reproduced, and the PR says so instead of implying a speed claim. That is the difference between a fix and an anecdote.
  2. Built on top of hrx: read the host term before classifying it, so a clang build opens the device again (engine #1942) #279 because at the tip a clang build registers no device (engine#1942) — stated as a dependency, not hidden. It also means this PR's own green is conditional on a commit that is not in it, which a later bisect needs to know.
  3. 58 of 994 text-graph nodes still run on the CPU (GET_ROWS × 1, ROPE × 56, PAD × 1), unchanged. The honest consequence: a text-only speed number after this PR is a mixed-path number, and any row measured on it should say so.

One thing I would ask for, and it is the same ask as on #284: a case where the window's own offset is non-zero and its consumer is not a plain ADD — the three slices here all feed ADD with offs=8192/16384/24576. If the elision rule is about "external + own storage + no alias source", then the consumer should not matter; an arm with a different consumer would show that it does not. Right now the proof is three offsets of one shape, which is good coverage of the offset axis and single-point coverage of the consumer axis.

Also worth noting for the engine side: if this lands, engine#1467's remaining piece is "the CLIP graph on HRX" rather than "Qwen3-VL does not run", and the text model's warmup no longer fails — that is a strictly larger effect than the title suggests, and the PR says it plainly (text-only decode of this model fails too, rc 1 before, coherent output after, byte-identical to CPU).

Scope of my check: read the diff (the scheduler predicate, both test files), the before/after refusals, the value comparison, and the limitations. Did not build, did not run — the numbers are the PR's, taken on gfx1151 under gpu-hold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant