Skip to content

hrx: do not fuse away a value that something outside the programme reads (engine #533) - #146

Closed
bong-water-water-bong wants to merge 3 commits into
1bit/hrx-vulkan-patchedfrom
1bit/533-fusion-guard
Closed

bong-water-water-bong wants to merge 3 commits into
1bit/hrx-vulkan-patchedfrom
1bit/533-fusion-guard

Conversation

@bong-water-water-bong

Copy link
Copy Markdown

hrx: do not fuse away a value that something outside the programme reads (engine ggml-org#533)

Stacked on #133 (f7071dde). Two changes, both about the same failure mode: a fused programme that takes over a
node while a reader of that node's output stays outside it.

  • Matcher guard (dispatch-gated-delta-net.cpp): before the selected-snapshot programme upgrades its last
    dispatch to the fused kernel, every consumer of the snapshot value and of the cache must be covered by the
    programme (consumers_covered_by). If one is not, the fusion is refused and the unfused programme is used. A
    reader scheduled before the fused kernel that writes the value is exactly what the command-program verifier
    rejects with "reads transient value N before write".
  • Test assertion (hrx-backend-test.cpp): run_gdn_selected_rms_q8_dispatch_checks forbade any
    ggml_copy_f32 in the plan. The snapshot copy is a legitimate part of the fused programme, so the case now
    requires that the copy precedes the fused kernel and feeds one of its bindings, and still forbids the fallback
    kernels (ggml_get_rows_f32, ggml_rmsnorm_gate_f32_q8_1_x4).

Measured, and it does not close the issue. With #133 + this, at 2ab3114966-era hrx-system 82804197bc,
host planner, GGML_HRX=ON:

host: 66 case(s) — 59 passed, 1 failed, 6 known-fail, 0 unexpected-pass
[FAIL] gdn_selected_rms_q8_dispatch
       hrx-backend-test.cpp:10255: command_program_verifies(commands)
       VERIFY-ERROR: command 4 kind=Kernel bindings=2 deps=1 binding source value=10
                     origin=Transient access=Read range=[0, 3145728) reads transient value 10 before write

So gdn_selected_rms_q8_dispatch still fails, on the snapshot copy: value 10 has no producer before command 4
even with the guard in place, which means the copy's consumer node is already inside the programme's covered set
while its dispatch is not in the list — the same shape as the bug #133 fixed for the first dispatch, one level
down. The next step is to make the covered/dropped pair impossible rather than to detect it: when the fused
kernel replaces a dispatch, the nodes that dispatch covered must be dropped from covered_nodes too, and the
verifier's "reads before write" is what will confirm it.

#533 therefore needs one more change beyond #133 and this PR; the #533 marker stays on the row, and the two
sibling cases (qwen_attention_postprocess_dispatch, qwen_routed_gate_up_dispatch) are untouched here.

Host and device run details are in the PR comments; nothing about this is claimed beyond what the verifier output
shows.

🤖 Generated with Claude Code

bong-water-water-bong and others added 3 commits October 23, 2026 12:39
…-only value (engine ggml-org#533 verify)

The fused postops export declares its bias and residual bindings with read access whether or not
the kernel uses them, and the kernel only loads them when has_bias / has_residual is set. For an
absent slot the dispatcher bound the command's own output transient, which the command-program
verifier rejects:

  VERIFY-ERROR: command 2 kind=Kernel bindings=10 deps=1 binding bias value=5 origin=Transient
                access=Read range=[0, 16384) reads transient value 5 before write

Bind a value the programme already reads instead - the weight - for those placeholder slots. The
kernel never loads it (has_bias / has_residual guard the read), so nothing changes numerically.

  before: [KNOWN-FAIL ggml-org#533] common_mul_mat_id_postops_dispatch
              VERIFY-ERROR: command 2 ... binding bias value=5 ... reads transient value 5 before write
  after:  [PASS] common_mul_mat_id_postops_dispatch  (tag retired in tests/hrx-backend-test.cpp)

Model check (Qwen3-Coder-30B-A3B Q4_K_M, the postops+next_rmsnorm user): the family enabled vs
GGML_HRX_DISABLE_DISPATCH=common.mul_mat_id_postops.f32_f32_wmma gives identical greedy text and
identical per-token top-5 logprobs on three prompts (128/126/29 tokens, max |dlogprob| = 0), and an
identical second pass on the same server (cold vs warm). 90 W cap, PCM pad (23 Oct).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r before the kernel (engine ggml-org#533)

match_gated_delta_net_rmsnorm_q8 replaces the last dispatch of the matched programme with the fused
selected_snapshot_projection_rms_gate_q8 kernel, then cleared the whole dispatch list and pushed that
one back. For a one-token graph the programme has two dispatches, so clearing dropped the first (the
projections / gate producer) even though its nodes stay in covered_nodes - the fused kernel then reads
raw_gate from a command that is emitted later, and the command-program verifier rejects the plan:

  VERIFY-ERROR: command 6 kind=Kernel bindings=5 deps=1 binding raw_gate value=33 origin=Transient
                access=Read range=[0, 24576) reads transient value 33 before write by command 7

Replace only the dispatch the fused kernel supersedes (back()) instead of clearing the list, so the
producer stays ahead of its consumer in the programme. The planner orders commands by append position
and gives each a linear dependency (dispatch/command-program.cpp), which is what makes the reversal
fatal rather than merely untidy.

Model check (Qwen3.8-27B UD-Q4_K_XL --mtp, the GDN selected-snapshot user): the family enabled vs
GGML_HRX_DISABLE_DISPATCH=llm.gated_delta_net.selected_snapshot gives identical greedy text and
identical per-token top-5 logprobs on three prompts (128/128/26 tokens, max |dlogprob| = 0), and an
identical second pass on the same server (cold vs warm). 90 W cap, PCM pad (23 Oct).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ads (engine ggml-org#533)

Stacked on #133 (`f7071dde`). Two changes, both about the same failure mode: a fused programme that takes over a
node while a reader of that node's output stays outside it.

- **Matcher guard** (`dispatch-gated-delta-net.cpp`): before the selected-snapshot programme upgrades its last
  dispatch to the fused kernel, every consumer of the snapshot value and of the cache must be covered by the
  programme (`consumers_covered_by`). If one is not, the fusion is refused and the unfused programme is used. A
  reader scheduled before the fused kernel that writes the value is exactly what the command-program verifier
  rejects with "reads transient value N before write".
- **Test assertion** (`hrx-backend-test.cpp`): `run_gdn_selected_rms_q8_dispatch_checks` forbade *any*
  `ggml_copy_f32` in the plan. The snapshot copy is a legitimate part of the fused programme, so the case now
  requires that the copy precedes the fused kernel and feeds one of its bindings, and still forbids the fallback
  kernels (`ggml_get_rows_f32`, `ggml_rmsnorm_gate_f32_q8_1_x4`).

**Measured, and it does not close the issue.** With #133 + this, at `2ab3114966`-era hrx-system `82804197bc`,
host planner, `GGML_HRX=ON`:

```
host: 66 case(s) — 59 passed, 1 failed, 6 known-fail, 0 unexpected-pass
[FAIL] gdn_selected_rms_q8_dispatch
       hrx-backend-test.cpp:10255: command_program_verifies(commands)
       VERIFY-ERROR: command 4 kind=Kernel bindings=2 deps=1 binding source value=10
                     origin=Transient access=Read range=[0, 3145728) reads transient value 10 before write
```

So `gdn_selected_rms_q8_dispatch` still fails, on the snapshot copy: value 10 has no producer before command 4
even with the guard in place, which means the copy's consumer node is already inside the programme's covered set
while its dispatch is not in the list — the same shape as the bug #133 fixed for the first dispatch, one level
down. The next step is to make the covered/dropped pair impossible rather than to detect it: when the fused
kernel replaces a dispatch, the nodes that dispatch covered must be dropped from `covered_nodes` too, and the
verifier's "reads before write" is what will confirm it.

`ggml-org#533` therefore needs one more change beyond #133 and this PR; the `ggml-org#533` marker stays on the row, and the two
sibling cases (`qwen_attention_postprocess_dispatch`, `qwen_routed_gate_up_dispatch`) are untouched here.

Host and device run details are in the PR comments; nothing about this is claimed beyond what the verifier output
shows.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@bong-water-water-bong

Copy link
Copy Markdown
Author

Holding #133 + #146 unmerged until gdn_selected_rms_q8_dispatch passes the command-program verifier: the matcher guard and the corrected test assertion are right, but the remaining 'reads transient value before write' in the selected-snapshot programme is the actual ggml-org#533 defect. Merge both together once that case is green on the host planner and on HRX0.

@bong-water-water-bong

Copy link
Copy Markdown
Author

Dump taken, cause identified, and it is not fixed by the guard in this branch — so #133 + #146 do not close
ggml-org#533.

format_command_program() for the failing case (8 commands; temporary print, reverted):

cmd kernel bindings that matter
0..3 ggml_copy_f32 copy graph values into transients 1, 3, 5, 7
4 ggml_copy_f32 reads transient value 10 (3,145,728 B) → writes graph value 29
5 fused …_selected_snapshot_projection_rms_gate_q8 (14 bindings) reads q/k/v = 1, alpha 3, beta 5, state 8, raw_gate 7 … and writes value 29 itself (snapshot_cache, Write), plus dst 35 and q8_output 40
6, 7 mul_mat, copy consume 40 → 38 → 39

Cause: value 29 is the snapshot cache. Command 4 copies transient 10 into it; the fused kernel also writes
29 directly, so the copy has become redundant — but it is emitted before the fused kernel (its node precedes
the GDN root in graph order) and its source, transient 10, was the pre-fusion GDN snapshot output, which no
command writes any more. Hence "reads transient value 10 before write".

Why the guard here does not help: consumers_covered_by asks whether the readers of the snapshot binding and
of the cache are inside the programme. They are — the defect is ordering, not coverage: a reader that was
emitted before the writer.

Next step, and it is a matcher decision, not a scheduler one: refuse the fusion when the pre-fusion snapshot
value (selected.dispatches.back().bindings[token_count == 1 ? 7 : 8], captured before the replacement) has a
consumer whose node index is lower than the programme root — that consumer is already emitted and would be
redundant. The cost is that such a graph takes the unfused path, so test.fused in
run_gdn_selected_rms_q8_dispatch_checks has to expect false for the shape that carries the cpy.

State written to ~/briefs/TISSUES-B-STATE.md (transient id 10, producer: none after fusion; consumer: command 4;
hypothesis and the three candidate fixes) so a fresh agent can take it from there.

Device run of the suite at this branch (HRX0, one ≤20 min hold):
host 59 passed / 1 failed / 6 known-fail, device 11 passed / 0 failed / 0 known-fail — the GDN row fails in
the planner on both halves, as expected.

@bong-water-water-bong

Copy link
Copy Markdown
Author

Option 1 tested — it does not work for this case, and the reason is worth recording. Branch is unchanged
(9a6ce858); the experiment was reverted.

What I tried: in match_selected_gated_delta_net_dispatch, refuse the fusion when a consumer of the pre-fusion
snapshot value has a node index lower than that value's producer (i.e. the reader is already emitted).

Result: the guard never fires — the case still reports the fused kernel in the plan and the same
command 4 … reads transient value 10 before write. The reason is that dispatch order is by programme root, not
by node index
: the copy's programme is rooted at the GET_ROWS node (early in the graph), while the fused
programme is rooted at the GDN node (later). So the copy is emitted first even though its node sits after the
GDN node, and no node-index comparison can see that.

So ggml-org#533's remaining failure is a scheduler ordering/withdrawal problem, not a matcher decision:

  • when a fusion covers nodes whose dispatches are already in the plan, those dispatches are redundant by
    construction and should be withdrawn (the programme's covered_nodes already says so), or
  • emission should be dependency-ordered rather than root-index-ordered.

Filed as engine ggml-org#570 with this dump, the two sibling rows that show the same class
(qwen_attention_postprocess_dispatch 12 bindings, qwen_routed_gate_up_dispatch 4 bindings), and the three
affected matchers.

Verdict for ggml-org#533: #133 fixes row 1 (common_mul_mat_id_postops_dispatch); #146 adds a correct guard (a value
fused away must not have outside readers) but neither closes row 2, which needs ggml-org#570. Host totals at this branch:
59 passed, 1 failed, 6 known-fail; device half 11 passed, 0 failed.

bong-water-water-bong added a commit that referenced this pull request Oct 25, 2026
…(engine ggml-org#533, ggml-org#570) (#150)

Registry-level fusion-order guard (dispatch-fusion-order.{h,cpp}): a match whose non-write-only binding reads transient data that no earlier dispatch (in the plan or in the match) wrote is refused and falls through to the next registration; GGML_HRX_FUSION_ORDER_GUARD=0 disables, GGML_HRX_LOG_FUSION_ORDER=1 logs. Also: routed-ffn gate_up_swiglu_f16_wmma only when every GLU consumer is a Q4_K/Q6_K routed down projection (a real defect: other down weights read an unwritten f32 value). hrx-backend-test host 60/0/6 -> 63/0/3, device 11/11, test-backend-ops 314/314, KLD identical on Qwen3.5-4B / ZAYA1-8B / Qwen3.8-27B with 0 refusals, 27B draft-mtp completions identical, hw-gate PASS. Supersedes #133 and #146.

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

Copy link
Copy Markdown
Author

Superseded by #150 (merged): a registry-level read-before-write guard closes engine ggml-org#533 / ggml-org#570 for all matchers; #133's kept copy read a value nothing writes in the fused programme (the 'value 10' error), and #146's consumers_covered_by is covered by the guard's checks 1 and 3.

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