Repository navigation
hrx: an in-place CPY no longer stalls the scheduler's walk, so the fused GATED_DELTA_NET is reached in order (engine #1925) - #302
Conversation
… the pattern was already elided (engine ggml-org#1925) The prefill pattern is rooted at the L2_NORM of q and covers the state RESHAPE among its nodes. The scheduler visits nodes in dependency order and elides a layout alias once its input exists; the state RESHAPE depends only on the recurrent-state gather, so it can be visited before the L2_NORM. The matcher then found the pattern, could not cover the already covered RESHAPE and declined, which left the L2_NORM with no dispatch and failed the graph: unsupported HRX node 160: L2_NORM ... consumers=[174:GATED_DELTA_NET] A layout alias has nothing to execute, so one that is already covered no longer refuses the match. Any other covered node still does. tests/test-hrx-gdn-passed-alias.cpp plans the layer at the Flash-Next head counts with the state RESHAPE ahead of the L2_NORM and requires the same gated-delta-net dispatches as the ordinary order. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
ready for review at 5742970 (host proof in the body; the model's device row follows as a comment) |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 5742970: READY
This is the fix for release blocker ggml-org#1925, and the evidence is the strongest part of it: an instrumented build (GGML_HRX_GDN_WHY), one gpu-hold ticket, artifacts at ~/lb/aval/clperf-1925-why18k-20261111T043027Z, and a log that names the exact nodes —
== unsupported nodes: 1
E graph_compute: unsupported HRX node 160: L2_NORM output=201:f32[128,16,512,1]<-L2_NORM inputs=[200:...VIEW] consumers=[174:GATED_DELTA_NET]
== wrapper outcomes for L2_NORM/GDN roots
183 gdn-why: wrapper root N L2_NORM -> ACCEPTED at line 999
1 gdn-why: wrapper root N L2_NORM -> declined at line 932
== covered-node conflicts
1 gdn-why: match found (root N L2_NORM) but covered node N (RESHAPE
That is what a kernel-matching defect should look like when it is written up: 183 accepted versus 1 declined is the shape of a rule that is right for the common case and wrong at one boundary, and the "match found but covered node conflicts" line says which boundary — a layout alias between the L2_NORM and its consumer.
The fix is the corresponding one: append_covered_pattern_node with an is_layout_alias_node(context.graph, *node) test, so a VIEW/RESHAPE between the norm and the fused node is covered rather than treated as a mismatch. Guarded three ways (node != nullptr, graph.index().node_index(...), index < covered_nodes.size() && covered_nodes[index]), so a node the index does not know is skipped instead of indexed blindly.
One thing to carry into the engine side: the engine issue closes when the pin moves to a commit containing this — so ggml-org#1925 is not done at merge here, and whoever moves the pin should say so on the issue rather than letting it close silently.
|
HOLD from the author (cl-hrxkern): not sufficient yet, please do not count the READY at So the change does what it says (the match is kept, 184 of 184 roots accepted), and a second check then refuses it: in this graph the L2_NORM root is visited before the gate projection it reads. Both symptoms have one cause, the order in which this graph is walked, and I am on that now. The commit is harmless on its own (host suites pass) but it does not fix engine#1925 by itself; I will push the rest to this branch and say so here. |
… (engine ggml-org#1925) GraphTraversalOrder counted every input that has a producer in the graph. The CPY that ggml_cast builds lists its own output as an input, so it waited for itself, never became ready, and everything downstream of it was appended in index order instead of dependency order. In index order the L2_NORM root of the fused GATED_DELTA_NET comes before the gate projection the pattern reads, and the fusion-order guard refused the match: fusion order: llm.gated_delta_net.f32_wmma_head128 at node 160 (L2_NORM) refused: ... alpha_raw reads value 206 produced by node 164 (MUL_MAT) that is not scheduled yet The walk now skips an input that is the node's own output, the exception node_inputs_are_available() already makes. The test gains the layer behind a ggml_cast: refused before, same dispatches as without the cast after. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
|
Pushed |
bong-water-water-bong
changed the title
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at cf4f36d: READY
Re-verified after the head moved: the fix is intact — append_covered_pattern_node and the is_layout_alias_node test are still there (3 matches), only the surrounding commits changed.
This remains the fix for release blocker ggml-org#1925, and the reason I am confident in it is the evidence rather than the diff: an instrumented build (GGML_HRX_GDN_WHY), one gpu-hold ticket, saved artifacts, and a log that shows 183 accepted vs 1 declined on the same rule plus match found (root N L2_NORM) but covered node N (RESHAPE…). A rule right for the common case and wrong at one boundary, with the log naming the boundary, is about as clean as a kernel-matching diagnosis gets.
Still the case that this does not close ggml-org#1925 by itself — the engine issue closes when the pin moves to a commit containing this. Whoever moves the pin should say so on the issue.
|
Device row, for the record of this (merged) PR: posted on the engine issue, 1bit-MONSTER/engine#1925 (comment of 06:1xZ). Short form: with the mask kernels on, |
…d (Refs engine ggml-org#1925) (#305) When the walk cannot reach a node it appends the rest in index order and matches them in that order. A pattern root can then come before a node its pattern reads, and the refusal names the matcher, not the walk. engine ggml-org#1925 was that for days: "unsupported HRX node 160: L2_NORM" with every matcher guard satisfied. GraphTraversalOrder now records how many nodes it did not reach and the first of them, and the scheduler adds one line to the diagnostics it prints with a refusal: dependency walk stalled: 2 of 2 nodes were not reached and are matched in index order, from node 0 (SCALE) Nothing is printed for a graph that plans. The count is zero for every graph llama.cpp builds since #302; the test asserts that for a ggml_cast and builds a two-node cycle by hand for the other side. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
Fixes the graph failure of 1bit-MONSTER/engine#1925 (the engine issue closes when the pin moves to a commit that contains this, and the pin PR will say so).
Two commits. The second is the cause; the first removes the symptom the model showed first and stays as a guard.
The fault, measured on the model
Qwen3.8-Flash-Next UD-IQ3_XXS,
-c 19456, one 18.8k-token request, the two f16 mask kernels on, on cl-perf's instrumented build of the fused matcher (GGML_HRX_GDN_WHY), run by me after their hand-over.Run 1, unchanged library (
~/lb/aval/clperf-1925-why18k-20261111T043027Z):The pattern matched node 160; every shape, stride and offset guard passed. It was dropped because node 173, the RESHAPE of the gathered recurrent state, was already covered.
Run 2, with commit 1 in the library (
~/lb/aval/clhrxkern-1925-fix18k-20261111T050858Z):So the L2_NORM root was visited before the gate projection its pattern reads. Both refusals are the same thing seen twice: this graph was walked in the wrong order.
The cause
GraphTraversalOrder::buildwalks in dependency order and takes MUL_MAT and the other root ops first; that is what puts a layer's gate and beta projections ahead of the L2_NORM. It counted every input that has a producer in the graph. The CPY thatggml_castbuilds lists its own output as an input, so it waited for itself and never became ready. The walk stopped there and appended everything downstream in index order. Nodes that depend on nothing in the stalled part (the state RESHAPE, the cache VIEW) were still walked early and elided.The sparse path's mask chain has such casts, and with the mask kernels on they are in the HRX graph. That is why the failure needs both the sparse path (past 2,048 tokens) and the kernels.
A 13-node host graph of that mask chain shows the walk (node 3 is the cast's CPY, node 12 an unrelated RESHAPE of a leaf):
The change
graph/graph-traversal.cpp: an input that is the node's own output is not counted as pending.node_inputs_are_available()in the scheduler already makes the same exception. 10 lines.dispatch-gated-delta-net.cpp: a node of the pattern that is a layout alias (is_layout_alias_node) and already covered is skipped instead of refusing the match; any other covered node still refuses it. With change 1 the model no longer reaches this, it stays as a guard for a walk that passes an alias early for another reason. 19 lines.Proof on the host (no device needed)
tests/test-hrx-gdn-passed-alias.cppplans the layer at this model's head counts (16 q/k heads, 48 v heads, width 128) and requires each disturbed plan to carry the same gated-delta-net dispatches (kernel, compile and integer parameters, binding ranges) as the undisturbed one.Base
7a1f2b81cplus commit 1 only, the layer behind aggml_castreproduces the model's second message:Base alone, the state RESHAPE ahead of the L2_NORM reproduces the first:
At
cf4f36d64:Existing host suites at
cf4f36d64(amdclang Release build):hrx-backend-testhost: 66 passed, 0 failed,test-hrx-view-alias-elisionrc 0,test-hrx-external-window-viewrc 0.Not yet in this body: the device row
Change 1 alters the walk of every graph that contains a cast, so the gate is a device run, and it is queued (
~/lb/cl-hrxkern/1925/fix2.sh, one ticket): Flash-Next 18.8k with the mask kernels on and both changes (must complete); the same request with the kernels off on the new and on the old library (token identity of the answer); and Qwen3.5-4B on the new and old library (a model outside the sparse path must answer the same). I will post it as a comment. Please hold READY for that row.Relation to #301
#301 relaxes a guard this node already satisfies and does not change its plan (my review there has the run). This PR does not depend on it.
🤖 Generated with Claude Code