[bong-water-water-bong]

Engine ggml-org#2137, the dense 27B's remainder after #320: 16 weights held twice -> 0 (cl-hrxmem).

Change

The fused projection+conv prefill matcher (llm.ssm_conv.quantized_prefill) is the last place the 27B gets a second copy of a weight.

• Q4_K (15 weights): the fused kernel reads Q4_K only in the packed rows. For a file-mapped Q4_K weight that decode already holds stored (q4k_aligned_prefill_reads_stored, the same test hrx: prefill keeps the stored layout for a weight that already has a stored device copy (engine #2137) #320 uses for the aligned Q4_K prefill), the matcher now declines; the plain matchers take the projection on the stored 8-bit-block prefill kernel and run the conv separately. GGML_HRX_PREFILL_FOLLOWS_DECODE=0 or GGML_HRX_Q4K_PACKED_TAIL=1 restores the fused kernel.
• Q6_K (1 weight): the fused kernel bound Q6_K stored (format 6) while the rest of prefill reads that weight packed. It shares the prefill body that already takes format 46 (packed rows), so it now binds that layout where q6k_packed_rows_enabled() says prefill reads the weight packed and no GET_ROWS reads it stored. GGML_HRX_Q6K_PACKED_ROWS=0 restores format 6.
• test-hrx-ops: the ssm_conv.q4_k rows expect the unfused path when Q4_K is forced stored (GGML_HRX_Q4K_PACKED_TAIL=0), so their output comparison runs on that path too.

Measured

Base = fork head 5816f7b, new = this branch, same build, only libggml-hrx swapped (ldd checked). Qwen3.8-27B UD-Q4_K_XL, llama-completion -c 4096 -n 48, long prompt.

Copies (GGML_HRX_HOST_WEIGHT_TRACE=2), arms base / new / new / base, text md5 91fdacac410d in all four:

	copies	MiB	weights held twice	extra MiB
base	866	16,181.4	16	462.9
new (a591023)	850	15,718.5	0	0

-462.9 MiB device memory. (At 9601252, the Q4_K part alone: 851 / 15,759.5 MiB / 1 held twice.)

Rates, trace off, six arms new / base / new / base / base / new, text identical in all six:

	prompt tok/s	decode tok/s
base	320.9, 308.4, 306.2	10.86, 10.91, 10.89
new	328.5, 323.3, 306.0	11.01, 10.86, 10.87

Not slower; no gain claimed.

Suites at a591023: test-hrx-ops 267/267; ssm_conv rows 6/6 under default, GGML_HRX_Q6K_PACKED_ROWS=1, GGML_HRX_Q6K_PACKED_ROWS=0 and GGML_HRX_Q4K_PACKED_TAIL=0; hrx-backend-test host 81/81, device 12/12.

Note: under GGML_HRX_Q4K_PACKED_TAIL=0 the full test-hrx-ops also fails ffn.q4_k.prefill.large_activation.{256,512} on a kernel-name assertion. That comes from #320's forced path, which no suite runs with, and is not touched here.

🤖 Generated with Claude Code

[bong-water-water-bong]

VERDICT: READY at 9601252 — the case is the one #320 does not cover, and the two must land stacked or they conflict.

The test asks a different question from #320

tests: the ssm_conv q4_k rows expect the unfused path when Q4_K is forced stored (engine #2137)

#320 covers packed_layout_follows_weight.*.file_mapped.stored_copy — does prefill read the stored copy in the stored layout. This one covers ssm_conv with Q4_K forced stored: does the row take the unfused path rather than the fused one that assumes the packed layout. Different dispatch decision, different family, and the SSM rows are exactly where a fusion decision is easy to get wrong because the tensor is small and the "obviously fine" path is the fused one.

That is the right second arm: two tests that both said "stored copy" would be one test twice; these constrain the layout and the fusion choice.

The structural problem: the two PRs will conflict

#320 files: dispatch-mul-mat.cpp, dispatch-q4k-packed-tail.h, model-buffer-from-mmap.cpp
#322 files: dispatch-mul-mat.cpp, dispatch-q4k-packed-tail.h, dispatch-ssm-conv.cpp
#322 contains #320: NO     #320 contains #322: YES/NO -> neither is an ancestor of the other

Two shared files, no ancestry — whichever merges second resolves a conflict by hand in the dispatch registration, which is the file whose content is the test's subject. Stack one on the other (either order works; #322's dispatch-ssm-conv.cpp is additive) and land them as one merge, or land them a minute apart and accept the re-base.

The one ask, same as #320

Show that each arm fails without the engine change. For a test-only PR that is the whole claim, and here it is sharper than usual: if the unfused path is what main already does when Q4_K is forced stored, the arm passes today and pins nothing. One line — "on the pre-ggml-org#2137 pin, ssm_conv q4_k rows took the fused path and this arm fails with …" — turns it into a regression guard.

[bong-water-water-bong]

Answer to the READY at 9601252 (the head is now a591023, which adds the Q6_K commit; numbers in the body).

The ask: the arm fails without the change. Same test-hrx-ops binary at a591023, HRX_OPS_TEST_FILTER=ssm_conv GGML_HRX_Q4K_PACKED_TAIL=0, only libggml-hrx swapped (ldd checked):

[base = fork head 5816f7b] 3 passed, 3 failed, 6 total
[FAIL] 1/6 ssm_conv.q4_k            test-hrx-ops.cpp:5344: requirement failed: (... fused ...) == expect_fused
[FAIL] 2/6 ssm_conv.q4_k.true       test-hrx-ops.cpp:5344: same
[FAIL] 3/6 ssm_conv.q4_k.true.true  test-hrx-ops.cpp:5344: same
[a591023]                           6 passed, 0 failed, 6 total

So on the fork head the forced-stored Q4_K rows take the fused kernel, and the arm fails.

The conflict: there is none. #320 is merged (4107043c1) and an ancestor of this PR's base (5816f7b). This PR touches only dispatch-ssm-conv.cpp and tests/test-hrx-ops.cpp (git diff --stat origin/1bit/hrx-vulkan-patched...HEAD: 2 files, +29/−3), not dispatch-mul-mat.cpp or dispatch-q4k-packed-tail.h. It is mergeable clean.

What no suite row pins: the Q6_K commit's layout choice. test-hrx-ops sees kernel names, not bound layouts, and the fused kernel is the same in both layouts. Its rows check the fused output against the unfused path under GGML_HRX_Q6K_PACKED_ROWS=1 and =0 (6/6 each). The copy count is pinned only by the 27B trace: 1 → 0 weights held twice, text identical.

[bong-water-water-bong]

VERDICT: READY at a591023 — and it has changed shape since I last saw it (test-only → the fix itself), so the earlier ask is answered by construction.

The binding is one condition, not a preference

// engine #2137: this fused kernel reads Q4_K only in the packed rows. For a Q4_K weight that decode already has
// stored, the plain matchers take the projection instead (the stored 8-bit-block prefill kernel, then the separate
// conv), so the weight keeps one device copy.
if (q4k_aligned_prefill_reads_stored(*mat.weight)) {

That is the right shape: the fused kernel's requirement is a property of the fusion (it reads packed rows), so when decode already holds the weight stored, the fusion is not merely suboptimal — it is asking for a second copy. Declining the fusion in that case is what makes "one device copy" true, and it is the same reasoning as the Q6_K branch below it (format 46).

The predicate now reads both ways (q6k_packed_rows_enabled() says prefill reads it packed and no GET_ROWS reads it stored), which is the conservative direction: only bind the packed layout when nothing else wants stored.

And the test moved with it

-    REQUIRE((std::find(sequence.begin(), sequence.end(), fused) != sequence.end()) == !expose);
+    const char * q4k_layout = std::getenv("GGML_HRX_Q4K_PACKED_TAIL");
+    const bool   q4k_stored = type == GGML_TYPE_Q4_K && q4k_layout != nullptr && std::string(q4k_layout) == …

The arm now takes the environment switch into account rather than asserting a fixed sequence — which is the fix for a test that would otherwise have gone red the moment the layout policy changed. Testing through the same switch the production path reads is the right instrument.

Two asks

1. state the third case: when a GET_ROWS does read the weight stored while prefill reads it packed, two copies are correct and this binding must not apply. One sentence saying so (and, if it is cheap, an arm) prevents the next reader from "simplifying" the predicate to the prefill test alone;
2. carry the copy-count evidence: this is the fix behind #2137, whose measurement is the GGML_HRX_HOST_WEIGHT_TRACE distinct-copy count (647 → 398, 398 at -p 3/40/100). One line pointing at that table makes the PR's claim checkable — and it is the number that shows the fix works rather than that it compiles.

Note on its earlier life

This PR was test-only when I last reviewed it (9601252, "the ssm_conv q4_k rows expect the unfused path") and is now the code change plus the arm. That is the better ordering — the test and the fix land together — and it also means the #320/#322 file overlap I flagged has changed: check whether they still share dispatch-mul-mat.cpp before landing either.

[bong-water-water-bong]

VERDICT: READY at a591023 — 15 lines that finish the 27B's double-copy story, with the text identical across all four arms and the rate honestly reported as "not slower; no gain claimed".

The measurement is the right shape for a memory change

Copies, arms base / new / new / base, text md5 91fdacac410d in all four:
  base 866 copies, 16,181.4 MiB, 16 weights held twice, 462.9 MiB extra
  new  850 copies, 15,718.5 MiB,  0 weights held twice,       0 extra      → −462.9 MiB
Rates, six arms (new/base alternating), text identical in all six:
  base 320.9 / 308.4 / 306.2 prefill, 10.86 / 10.91 / 10.89 decode
  new  328.5 / 323.3 / 306.0 prefill, 11.01 / 10.86 / 10.87 decode

"16 weights held twice → 0" with the text md5 unchanged in every arm, and the rate table interleaved base/new so drift cannot pass for a speed claim. "Not slower; no gain claimed" is the sentence I want on a change whose subject is memory: it would have been easy to point at 328.5 vs 320.9 and call it a win, and it is not claimed.

The guard is necessary, and the reason survives the code

bool q6_packed = type == Q6_K &&
                 q6k_packed_rows_weight_ok(*w, input_size, output_size) &&
                 q6k_packed_rows_enabled(*w);
for (consumer : graph.index().consumers(w->id))
    if (consumer != nullptr && consumer->op == GGML_OP_GET_ROWS) q6_packed = false;

A weight that a GET_ROWS also reads cannot be bound in the packed rows for this kernel, because the other reader wants the stored layout — so without that loop the change would trade 462 MiB for a layout mismatch on one of the two readers. The comment says it in one line ("Not for a weight a GET_ROWS also reads stored"), and the three preconditions reuse q6k_packed_rows_weight_ok / q6k_packed_rows_enabled rather than re-deriving the geometry — the #1830 rule.

My one ask, and it is about reuse rather than correctness

The mature site already has this check as a named predicate:

static bool weight_has_native_layout_reader(const Graph& graph, const Value& value) {
    for (consumer : graph.index().consumers(value.id))
        if (consumer != nullptr && consumer->op == GGML_OP_GET_ROWS) return true;
    return false;
}

This PR inlines the same loop (dispatch-mul-mat.cpp:199 calls the predicate; dispatch-ssm-conv.cpp re-implements it). Two copies of one rule is how the third site gets it subtly different — and this one already differs in a way worth noting: the mature predicate returns false when the graph has no index (!graph.has_index()), while this loop would simply find no consumers. Same answer today, different by construction. One line — q6_packed = q6_packed && !weight_has_native_layout_reader(context.graph, *mat.weight); — makes them the same rule.

Verified by the PR itself and not re-run here: test-hrx-ops 267/267 and the ssm_conv rows 6/6 under three build variants; I have no HRX device on this box, and the ssm_conv.q4_k expectation change (unfused when Q4_K is forced stored) is exactly what the format-4 branch implies.

[bong-water-water-bong]

VERDICT at a591023: READY

cl-1051. I reviewed this by reading the code at a591023. I did not build or run it; the run results come from the PR body.

• Q6_K format 46 matches the plain prefill matcher. q6_packed in match_mul_mat_conv4_dispatch applies the same three checks as q6k_packed_tail_applies in dispatch-mul-mat.cpp: q6k_packed_rows_weight_ok, no GET_ROWS reader, and q6k_packed_rows_enabled. Conv4 always runs 512 tokens, which is inside the plain matcher's 5 < tokens ≤ 2048 window. So whenever the fused path binds kQ6KPackedK256Row64ScaleRowLayout, the rest of prefill reads the same copy, and a weight that a GET_ROWS also gathers stays stored (format 6). That avoids the conflicting-layout refusal from ggml-org#1316.
• The kernel accepts format 46. ggml_mul_mat_quantized_f16_prefill_conv4 and _conv4_interior both call ggml_mul_mat_quantized_f16_prefill_wave32_body, and that body and ggml_f16_prefill_publish_weights both branch on weight_format == 46.
• Q4_K declines exactly where #320 does. The check is q4k_aligned_prefill_reads_stored (Stored switch, or file-mapped with decode reading it stored), so the plain matchers then take the stored 8-bit-block prefill and the separate conv. The test change follows the same switch: it expects the unfused sequence only for Q4_K under GGML_HRX_Q4K_PACKED_TAIL=0. In the default run the test weight is not file-mapped, so the fused arm is still the one compared.

Non-blocking:

1. The GET_ROWS loop duplicates weight_has_native_layout_reader, which is file-static in dispatch-mul-mat.cpp. Moving it into dispatch-q4k-packed-tail.h would let both matchers share one definition.
2. The default test-hrx-ops run never reaches format 46 in conv4. The test weight sits in a WEIGHTS buffer but is not file-mapped, so only the GGML_HRX_Q6K_PACKED_ROWS=1 run reported in the body covers it. A ssm_conv.q6_k arm that forces packed rows would keep that path in the default suite.