Repository navigation
third_party: bump llama.cpp to 6e42864a6 (3 fork commits; supersedes #1073, which re-bumps an already-merged pin) - #1075
Conversation
…ince 817d0815b 817d0815ba -> 6e42864a6 (fast-forward on 1bit/hrx-vulkan-patched), three commits: 6e42864a6 convert/model: store Zamba2's shared transformer block once per block (engine #1036, fork #203) b390c2db1 hrx: log a residency decline, so a budget that reshapes the graph is visible (engine #674, fork #174) c08e9510b src: opt-in per-layer attention q/k dump (LLAMA_ZHA_DUMP_DIR) (fork #179) None touches the architecture registry, so unlike #1039 there is no registry_build/device_route consequence (checked: 0 files under registry/ in the three commits). Each commit carries its own build wiring: #203 and #174 only touch existing files plus one new header, and #179 adds src/llama-zha-dump.{h,cpp} together with its src/CMakeLists.txt entry. Behaviour a reader bumping this pin should know about: * #203 changes what a converted Zamba2 GGUF contains -- a re-convert now writes the shared transformer block once per block and the loader aliases the rest (ffn_up stays per layer because of the per-layer MLP adapter). An existing Zamba2 file keeps working on the old path; a new one is smaller. * #174 adds the hrx placement: decline log (GGML_HRX_HOST_WEIGHT_LIMIT_MB), eight distinct (weight, op) pairs then one suppression line, plus a verbosity note in the comment. * #179 is gated on LLAMA_ZHA_DUMP_DIR and does nothing when it is unset. Closes #1073 as superseded: that PR bumps to 817d0815ba, which main already has (#1039, 518cd93), so its merge was a no-op.
tools/registry_build.py --check-pins failed the bump (registry_pins, test 43): a pin moved without regenerating the registry. Ran it with third_party/llama.cpp at 6e42864a6: hrx 297, npu 7, architectures 297, mapped 297 -- unchanged from the recorded count, so the three commits add no architecture. The only change is the recorded pin: - "llama.cpp (hrx)": "817d0815ba5af..." + "llama.cpp (hrx)": "6e42864a628af..." --check-pins: "records the committed pins"; --check-gaps: "none of the 28 reviewed significant classes is mapped without a reason".
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at d06afd89 — CI is green (build SUCCESS, the two pin guards SUCCESS, CodeQL/CodeQL-analyzers SUCCESS, hw_gate skipped), and this replaces #1073.
I authored this one, so the "verdict" is a record of the checks rather than an independent review; the review that matters is the three fork commits, which I had already given READY verdicts as fork PRs #203, #174 and #179. Two files:
third_party/llama.cpp 817d0815ba -> 6e42864a628af8e97bc13b8e3ddb3dc7a72fc6d4
registry/architectures.json "llama.cpp (hrx)": 817d0815ba5af... -> 6e42864a628af...
The second file is the part the first version of this PR missed, and registry_pins caught it exactly as designed (a pin moved without regenerating the registry). With third_party/llama.cpp at the new pin, tools/registry_build.py reports hrx 297, npu 7, architectures 297, mapped 297 — unchanged counts, so the three commits add no architecture; only the recorded pin moves. --check-pins and --check-gaps both pass.
What the bump brings, for the record: #203 (a converted Zamba2 GGUF now stores the shared block once per block — a re-convert changes the file, an existing file still loads), #174 (the hrx placement: decline log, gated on GGML_HRX_HOST_WEIGHT_LIMIT_MB), #179 (per-layer q/k dump, gated on LLAMA_ZHA_DUMP_DIR, inert when unset).
I do not merge: the merge decision is the orchestrator's. My recommendation is in ~/briefs/TPRS-STATE.md's MERGE ORDER block.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Follow-up: I labelled this PR hw-gate (it was SKIPPED — see #1077), and the gate has now done the verification that nothing else in CI does: it compiled the HRX backend against the new pin and ran the matrix.
From the self-hosted runner's own results.tsv for this run:
build PASS 184s
ops PASS 99s 1959/1959 tests passed (MUL_MAT, MUL_MAT_ID, FLASH_ATTN_EXT,
GATED_DELTA_NET, SSM_CONV, GET_ROWS, ROPE, RMS_NORM, SOFT_MAX)
fa-decode-q8 PASS 1s 6/6 (K q8_0 x V f16 and q8_0; Hq/Hkv 16/8 and 32/8; n_kv 1031, 2048, 4103)
kat qwen3-0.6b PASS 19s 9 cases, 3 warnings
needle qwen3-0.6b PASS 7082-token prompt: expected 7341, got '7341'
placement qwen3-0.6b PASS all on HRX in 170 graphs (largest 986 nodes)
kld qwen3-0.6b PASS mean KLD 0.000001, p99 0.00004, same top 100.00%
kat zaya1-8b PASS 47s 9 cases, 0 warnings
needle zaya1-8b PASS 7403-token prompt: expected 7341, got '7341'
placement zaya1-8b PASS all on HRX in 193 graphs (largest 6449 nodes)
kld zaya1-8b PASS mean KLD -0.000000, p99 0.00004, same top 100.00%
model qwen3.8-27b FAIL 641s could not run: the memory pre-check refused it (rc 76)
What that settles for this PR: build PASS is the HRX compile — GGML_HRX is off in ci.yml, so this is the only job that compiles ggml/src/ggml-hrx, and it is where fork #174's host-weight-placement.{h,cpp} gets built. With ops 1959/1959 and the kat/needle/placement/kld rows green on two models, the pin bump is verified on both halves, not just the non-HRX one.
What it does not settle: the 27B row, and not because of this PR — the memory pre-check refused the step ten times over 641 s (rc 76), with the box at 88 GB used / 33 GB available at the time (other lanes' models loaded). That is an environment refusal, so the 27B coverage for this pin is unmeasured, and re-running it needs a window where the box is quiet. Worth knowing before anyone reads "the gate passed" as 27B-inclusive.
This is also the demonstration for #1077: the mechanism works, it just was not being asked to run — the previous pin bump (#1039) shipped with the same hw_gate: SKIPPED.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
One detail that makes the red readable: the gate exited 2, not 1 — and that is its own convention for "infrastructure refused this, not the code":
# scripts/hw-gate.sh
[ $rc = 76 ] && { note "::error::memory pre-check refused the step ${tries} times"; infra=1; }
…
[ "$infra" = 1 ] && exit 2and the run's own last line is Process completed with exit code 2. So the red on this PR means the box did not have the memory for the 27B row (88 GB used / 33 GB available at the time, refused ten times over 641 s), while build (the HRX compile), ops (1959/1959) and the kat/needle/placement/kld rows on 0.6B and zaya1-8b all passed.
The 27B coverage for this pin therefore remains unmeasured rather than failed — it needs a re-run on a quiet box, and I would rather it be re-run than read as a pass. That is also a small point in the gate's favour: the exit code distinguishes the two cases, so nobody has to guess from the log which kind of red this is.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at d06afd89 — with two things the merger must know, one of them about a duplicate.
What the change is. third_party/llama.cpp 817d0815ba → 6e42864a6 (3 commits: fork #203 Zamba2 shared block, #174 residency-decline log, #179 qwen/k dump behind an env var) plus the one line that records the pin in registry/architectures.json. 2 files, +2/−2.
The gate failure that was mine, and its proof now. My first push moved the gitlink and left the registry alone, so registry_pins (ctest 43) failed exactly the way it failed on #1039: "a pin moved without regenerating the registry". The three fork commits touch 0 files under registry/, which is not the same claim as the pin not needing a registry update — the pin is recorded there. Fixed in d06afd89 by regenerating with third_party/llama.cpp checked out at 6e42864a6:
hrx 297, npu 7, architectures 297, mapped 297 <- unchanged counts: no new architecture
- "llama.cpp (hrx)": "817d0815ba5af..." + "llama.cpp (hrx)": "6e42864a628af..."
--check-pins -> records the committed pins ; --check-gaps -> none of the 28 classes unmapped without a reason
The HRX half is compiled by a real gate, not assumed. hw_gate: SKIPPED is the default for a pin bump (engine #1077: GGML_HRX is OFF in ci.yml, so no PR-time job compiles ggml/src/ggml-hrx at all). I labelled this PR hw-gate, which started run 41983120230 on the strixhalo-engine self-hosted runner, and its own results.tsv reads:
build PASS 184s <- the HRX compile; the only job that builds ggml/src/ggml-hrx against the pinned pair
ops PASS 99s 1959/1959 tests passed (MUL_MAT, MUL_MAT_ID, FLASH_ATTN_EXT, GATED_DELTA_NET,
SSM_CONV, GET_ROWS, ROPE, RMS_NORM, SOFT_MAX)
fa-decode-q8 PASS 1s 6/6
kat/needle/placement/kld qwen3-0.6b PASS · zaya1-8b PASS (placement: all on HRX in 170 / 193 graphs)
model qwen3.8-27b FAIL 641s "could not run: the memory pre-check refused it (rc 76)" x10
Read the red correctly: it is not the pin. The run exited 2, and scripts/hw-gate.sh uses exit 2 for its own infra signal ([ $rc = 76 ] && infra=1 … [ "$infra" = 1 ] && exit 2). The 27B step was refused by the memory pre-check with the box at 88 GB used / 33 GB available (other lanes' models loaded), ten times over 641 s. So 27B for this pin is unmeasured, not failed — it wants a quiet window, and it is not a statement about 6e42864a6. Everything that did run passed.
The duplicate. Engine #1082 (1bit/pin-6e42864, 3895affd) is the same bump and its patch is byte-identical to this one — git diff of both PRs hashes to the same md5 (c4ffc80d…), 2 files +2/−2 either way. One of the two must be closed, and only one may be merged: the second squash is an empty merge. My recommendation is this PR, because its HRX compile and ops matrix are verified above; #1082 carries no gate and its hw_gate has never run. If the red hw_gate on this PR is inconvenient for the merge UI, merging #1082 lands the identical bytes and this one should then be closed as superseded — the outcome for main is the same, the evidence is not.
Not verified by me: a model run at the new pin (the PR's own logit-equality table at -dev none / -dev HRX0 is the author's, and I did not re-take it), and the 27B placement row above. Nothing in the three commits touches the registry content, the kernel corpus or the HRX runtime.
… of them inside the split's file, and the moved block survives it main went bd7d451 -> cb5866e -> 05da4d8 while the five gate rows were being re-measured, and one of the twelve commits edits the file the split is built around: npu/forward/src/q4nx_forward.cpp +38/-5 (#1087 rope-scaling refusal by name, #1072 the batch phase counters zeroed BEFORE BatchPhases snapshots them) npu/forward/include/q4nx_rope.h +72 (new: q4nx_rope_declared_type / q4nx_rope_type_supported) npu/pack_import.cpp/.h +97 (#844 final-norm import from the .fconst tail envelope) app/, config/, docs/, tests/, tools/, os/ (laya-first #1069, determinism tool #1068, serve logging #1074, onnx pricing #1076, puppy vm-check #1080, release-check #1079/#1071) third_party/llama.cpp pin bump 6e42864a6 (#1075) Textually clean (`git merge-tree --write-tree` produced no CONFLICT), which is exactly the case that was NOT clean the last time main landed in that file — so the merge was checked by what the compiler and the tests say, not by "auto-merging" messages: - #1072 moved `BatchPhases bph` / `BatchPhaseScope _batch_scope` / `tp_bs` so the counters are zeroed before the snapshot is taken. Those symbols live in npu/forward/include/q4nx_lh.h in this branch (the move this PR exists for); the statement order is main's and is carried over verbatim, so the reordering lands on the header's definitions unchanged. - The moved set is still defined exactly once: `inline StepPhases g_ph` (q4nx_lh.h:116), `phprof_on`/`ph_ms` (q4nx_lh.h, one each), `inline bool g_in_batch` (:131), `struct BatchPhaseScope` (:132), `lh_dump_layer_host` — and q4nx_forward.cpp no longer defines any of them. - `BatchPhases` remains a local struct in q4nx_forward.cpp (one definition), which is where main keeps it. Build: `cmake --build build-p4 --target onebit -j4` under ~/lb/thermal-run.sh -> rc 0, 16 objects rebuilt (warm tree; the pin bump and the two touched TUs). Tests: `ctest` **87/87, 0 failed** (85 passed, 2 skipped: mtp_batch_all, gdn_batch_order — the same two that skip on main). Consequence, stated rather than implied: the five gate rows measured on A=bd7d451a are VOID and are being re-run against A built at this main; this commit carries no numeric claim of its own.
third_party/llama.cpp:817d0815ba→6e42864a6— the 3 fork commits merged since the current pin. Supersedes #1073, which bumps to817d0815ba(already onmainvia #1039,518cd93), so that PR's merge was a no-op.The three commits (fast-forward on
1bit/hrx-vulkan-patched)What I checked before opening this
device_route.shrow): 0 files underregistry/in the three commits, soregistry_build.py --check-gaps/--check-pinsare unaffected.host-weight-placement.h), and npu: GGUF-on-NPU serving route (Qwen2.5-7B, MiniCPM4-8B, MiniCPM5-1B answer ' Paris' on the NPU) #179 addssrc/llama-zha-dump.{h,cpp}together with itssrc/CMakeLists.txtentry. Nothing is missing for a build to pick up.git rev-parse origin/1bit/hrx-vulkan-patched=6e42864a628af8e97bc13b8e3ddb3dc7a72fc6d4), not a mid-branch commit.Behaviour a reader bumping this pin should know
ffn_upstays per layer because it carries the per-layer MLP adapter. An existing Zamba2 file keeps working on the old path (every layer is its own source, so every alias is the identity); a new one is smaller.hrx placement:decline log (GGML_HRX_HOST_WEIGHT_LIMIT_MB): eight distinct (weight, op) pairs then one suppression line, with the verbosity note in the comment.LLAMA_ZHA_DUMP_DIRand does nothing when it is unset.Scope
Gitlink only — one line, no source change of ours. I am the PR-review lane (pi-prs): I opened this because #1073 asked for a bump and the useful bump was 3 commits further on; I do not merge, and the pin owners should treat the three commit notes above as the review.
The step I missed, now in this PR (and worth writing down for the next bump)
registry_pins(ctest 43) failed the first version of this PR:So a gitlink bump is two files, not one: the pin and the registry record it is checked against. The second commit here is that regeneration — run with
third_party/llama.cppat6e42864a6:unchanged counts, so the three commits add no architecture — which is the same conclusion my "0 files under registry/" check reached, arrived at the way the gate wants it.
--check-pins→ "records the committed pins",--check-gaps→ "none of the 28 reviewed significant classes is mapped without a reason".