Repository navigation
Bump the HRX pair to the full AMD sync (llama.cpp b04c4e95 + hrx-system 563afce1) - #329
bong-water-water-bong wants to merge 8 commits into
Conversation
Moves both pinned submodules onto AMD's tested pair, merged with our commits.
third_party/llama.cpp 522dab47 -> b04c4e95
= AMD b802a507 merged with our 159 commits, then llama.cpp#94 on top, so it
carries the engine#315 NaN fix as well as AMD's kernel-corpus refactor.
third_party/hrx-system 98d05d94 -> 563afce1
= AMD 40a1a36c merged with our libhrx device knobs (HRX_AQL_BLOCK_SIZE,
HRX_COMMAND_BUFFER_MODE). The three Loom target-info conflicts resolved to
AMD's side: our commit there was a backport of a fix AMD has since landed.
DO NOT MERGE UNTIL VALIDATED ON STRIX HALO. GitHub-hosted CI builds this WITHOUT
HRX, so CI cannot catch an HRX breakage:
cmake -B build -G Ninja -DONEBIT_HRX=ON && cmake --build build --target onebit
tests/serve_e2e.sh build/1bit <Qwen3-0.6B Q4_K_M .gguf> hrx
tests/serve_e2e.sh build/1bit <Qwen3-0.6B Q4_K_M .gguf> cpu
test-backend-ops -b HRX0
Pre-validation check:
|
The bump moved third_party/llama.cpp to b04c4e95 but left registry/architectures.json recording 522dab4, so ctest registry_pins - which the build job runs, not gated on ONEBIT_HRX - fails on the pin mismatch. The registry's inputs are byte-identical between the two pins (convert_hf_to_gguf.py, conversion/, gguf-py/gguf/constants.py, src/llama-arch.cpp, src/llama-model.cpp), so only sources["llama.cpp (hrx)"] moves; the architecture set and counts are unchanged. Co-authored-by: agent <agent@local>
|
Added the registry regen in
I made it a one-line edit rather than running The HRX validation gate in the description is untouched: still a draft, still needs the Strix Halo build plus |
AMD's refactor removed the multipass reduce provider, so the merged corpus only
covers key_value_token_capacity up to 2048. Our inherited constant was still
32768, which would offer the decode-split dispatch for capacities with no
provider, making the selector reject every candidate ("all_rejected") and
failing the whole decode. Capped at 2048.
Static check while the HRX gate waits: nothing is left dangling by the corpus refactorThe riskiest part of this merge is the corpus refactor (three monoliths replaced by motifs), because dispatchers reference kernels by string through
202 references, all present. That cap is the other half of the same refactor - the capacity range rather than the names: the merged corpus keeps |
The bump does not build: the manifest union kept 47 pre-refactor pathsI ran the validation gate from the description on Strix Halo (
Measured against the tree the PR pins (
First missing and the missing recipe refs include So the semantic union kept our pre-refactor entries alongside AMD's new ones. What is needed: drop or repath our stale One other thing worth knowing, not a pin problem: a first configure aborted in |
Follow-up: the manifest union is not consumable by its own generator (it is not just stale paths)I repaired the 47 stale
The 19 lost primaries are ours, not AMD's: AMD's refactor restructured exactly those areas ( Conclusion: this needs reconciliation, not a repath. Either take AMD's corpus wholesale and deliberately re-port the kernels we still need (the Reproduce (Strix Halo, in a throwaway worktree, no fork push): |
The corpus repair is pushed and the HRX build is greenBranch What changed
Build evidence (Strix Halo, throwaway worktree,
|
GPU half run: the build is green, and
|
| op | failures | shape |
|---|---|---|
MUL_MAT |
40 | type_a in iq1_s, iq1_m, iq3_xxs, iq2_xxs, mxfp4, q2_*, pq2_0, ptq1_0; ERR = inf |
MUL_MAT_ID |
30 | f32/f16, m=512,n=129,k=256; ERR ~ 1.0 (i.e. wrong, not noise) |
GET_ROWS |
7 | same IQ/MXFP4/Prism types; ERR = inf |
ADD |
5 |
ERR = inf across exactly the formats our corpus adds and AMD's does not is the pattern to look at: at this pin, AMD's motifs/dequant.loom carries 45 IQ references where ours carries 81, and our ours/ops/kquant_decode_f32.loom is the file that enumerates IQ1_S/IQ1_M/IQ2_XXS/IQ2_XS/IQ3_XXS. So the sync's dispatch is very likely selecting a kernel that has no lane for those formats.
I do not want to claim that from the pattern alone, so I have started the control: the same test-backend-ops -b HRX0 build at the pre-sync pin (522dab478), to separate "the sync regressed these" from "HRX already failed these". I will report which it is before anyone acts on this.
Everything else - the corpus reconciliation, the kept kernels under ours/, the manifest rebuild - is unchanged from the previous comment.
Control run: the 84 failures are a regression from the sync, not pre-existingI built the same
Same box, same toolchain, same So this is not something my corpus reconciliation introduced - the reconciliation is what makes the synced tree build and run at all - and it is not a pre-existing HRX weakness. It is the sync: at this pin, the ops handling the formats our corpus adds and AMD's does not ( The shape of it matters for the merge policy: the "auto-merged remainder preserved" rule kept our callers and matchers while the merge resolved the files that carry the format lanes to AMD's side. Building a corpus from AMD's tree plus our kernels keeps our files intact, but it does not bring back a dispatch path that AMD's refactor rewrote. This PR should not be un-drafted or merged until these 84 are back to 0 - Branch is |
|
Split into two causes: one is our hrx-system loom delta, the other is the hybrid dispatchI ran the two controls. First: the pins are not the problem
which is precisely the base our Cause 1 - our loom delta makes every kernel fail to loadOur That
So our loom delta changes the compiled export ABI (or the parameter count) relative to what the corpus records, and every kernel the model needs is then refused. It needs re-expressing on top of AMD's plan, or dropping where AMD's newer code already covers the hazard. Cause 2 - the 84 op failures are not the loomWith AMD's stock loom the run is identical: The worktree currently sits at AMD's stock loom (the passing state for |
Cause 1 narrowed: it is the merged
|
Cause 1 fixed and verified:
|
Cause 2 characterised: the failures are shape-specific, so it is specialisation selection, not missing formatsI bisected with
At
One flag ruled out: Taken with the pre-sync tree passing all of these (1075 OK / 0 FAIL), the mechanism is clear: our dispatch selects a specialisation for the token count, and the set of specialisations it can choose from is now AMD's corpus instead of ours, so the selected kernel is not the one the shape needs. That is the port - our dispatchers re-based onto AMD's kernel definitions - and it is bounded: it is Nothing else changed; both fix branches stand ( |
|
Correction to the flag sentence in the previous comment: on the |
Cause 2 is mutual inconsistency: neither dispatcher works against the other side's kernelsI traced the The merge took AMD's dispatcher wholesale for this file. but the header next to it, Restoring our dispatcher does not fix it either. With Those are AMD's definitions ( So it is not "our kernels are stale" or "AMD's kernels are buggy" - it is that our caller layout and AMD's kernel definitions disagree in both directions, and each side only works with its own. That is the port: per op, pick one side's dispatcher and its kernel definitions, rather than the half-merge that is there now. I have reverted my experiment and left the tree as it was. Running now: is any of this AMD's own?The one control that settles the boundary is a pure AMD tree - |
The pure-AMD control was invalid - please disregard that line of enquiryReverting That is a revert inside our tree, not AMD's shipped pair, so it says nothing about whether AMD's own Everything in the previous comment stands and does not depend on this: with AMD's dispatcher the |
The merge discarded our side of 59 files - that is the whole of cause 2I classified every file under
So of the 94 files both trees had and that differed, the merge took AMD's side in 59 (63%), auto-merged 21 and kept ours in 14. The PR description's "11 content-conflicted files ... auto-merged remainder preserved (so our non-conflicting 159 commits survive)" does not hold inside The 59 by area: And they map exactly onto the failures:
That is why the callers and callees disagree: we kept our dispatcher in some files while the kernel it targets and the dispatch registry that describes it were replaced by AMD's. Experiment running: |
Worse than the 59: the merge also deleted 83 files we had and AMD does notThe "restore the 59 from because one of the 59 is our
So the merge's effect on That also explains why our restored dispatchers then refused AMD's routing kernels: the whole MoE corpus they were written against was deleted, so the names resolve to AMD's replacements with AMD's layouts. Where that leaves it. "Keep our side" means restoring 142 files, not 59 - and the Tree reverted to the repaired state ( |
Bigger than the 84 ops: real models do not run at the synced pinI ran Pre-sync ( Synced + repaired ( Both failures are Loom root linking failures on the corpus's own symbol constraints - Two controls:
And it is not just the two models: So the count of things this bump needs is larger than the 84 op regressions: the port has to satisfy the corpus's constraints, not merely resolve kernel names. A repaired-and-building tree that cannot start the target models is still not landable, and I would keep this PR a draft. (Round-10 note for context: the gate pass itself measured pin |
The two model failures are corpus constraints, and relaxing them is necessary but not sufficientI traced round-19's failures to exact lines. AMD's corpus tightened two symbols against ours: GLM-4.7-Flash needs I relaxed both in 7 files and rebuilt:
Net: the constraints are a real defect class of this sync (they exclude shapes real models use), but they |
Correction: relaxing the constraints is a false fix, not a partial oneMy previous comment reported "gpt-oss prompt now runs: 1906.16 t/s" after relaxing The prompt path passing means the constraint check no longer rejects 2880 - it does not mean a kernel supports 2880. The decode that follows is refused: So AMD's kernels are compiled for the shapes their constraints declare, and letting 2880 past the guard just moves the failure to kernel materialisation. The What that means for the port: I have reverted the constraint edits; the validation worktree is back to |
Engine#329 as it stands does not build: the AMD sync auto-merged our kernels into AMD's refactored corpus, which left duplicate SSA names in shared motifs, 47 manifest paths with no file, 19 exports with no source, and dispatch-mul-mat.cpp with our body and AMD's include block. Point third_party/llama.cpp at 1bit/amd-sync-corpus-repair (420b4e6f) and third_party/hrx-system at 1bit/amd-sync-wait-plan (3d0cca89a0), and move the registry source pin with them. Both are one commit ahead of what this PR pinned, so the pin check still only moves forward. With these, cmake --build build --target onebit succeeds (0 errors) and serve_e2e.sh passes on HRX for Qwen3-0.6B. test-backend-ops -b HRX0 still reports 989 OK / 84 FAIL and gpt-oss/GLM do not run, so this makes the gate measurable, not green.
I pinned the repaired trees into this branch - it builds now, and here is what the gate will showThis PR's head ( So I added one commit on top,
What the gate will now show, measured on this box (all with
So the validation is now measurable rather than dead at the build step, and it will fail on the last two rows - those are the port (our dispatchers and kernels against AMD's kernel definitions and constraints), not something this commit can fix. Two notes for whoever runs it: |
Correction: "compiled ABI does not match manifest" is not about the ABI - it is
|
The decode blocker is 15 kernels with an empty launch program - and 9 of them are AMD'sFollowing the Counted the distinct kernels that fail this way in one GLM run: 15, split 6 ours / 9 AMD's: So this is not the That is precisely the class our lane-mask drain backport ( Current map, all measured on this box:
Nothing changed in the tree; it is still |
…g launch programs The previous pin (420b4e6f) still had runtime/loom-jit-disk-cache.cpp storing entries without the host-side launch program, so every cache hit was refused as "compiled ABI does not match manifest". 39588ef9 skips caching those results and bumps the cache version. With it, GLM-4.7-Flash-Q4_K_M runs on HRX end to end: pp512 896.84 t/s, tg32 28.05 t/s. serve_e2e on Qwen3-0.6B still passes. test-backend-ops -b HRX0 is unchanged at 989 OK / 84 FAIL.
GLM-4.7-Flash now runs on HRX at these pins - pp512 896.84 t/s, tg32 28.05 t/sI found the cause of the "compiled ABI does not match manifest" wall, fixed it, and moved this PR's pin to the fix ( The bug
The fix (
|
The bump regresses 32
|
The
|
| check | before | after |
|---|---|---|
test-backend-ops -b HRX0 -o MUL_MAT_ID |
78 / 108 (32 x ERR ~ 1.0) |
108 / 108 |
GLM-4.7-Flash -p 512 -n 32, default -ub 512 |
test_gen: failed to decode generation batch |
pp512 902.21 t/s, tg32 27.40 t/s |
That second row is #315: the all-NaN logits used to fire for every ubatch of >= 256 tokens, which is why the
-ub 128 workaround existed. At the default ubatch GLM now generates, so the workaround is no longer
needed either.
One intermediate result worth keeping: pointing only the tiled constant at our kernel gave 90/108 and fixed
n=17/32/129, but left n=5 at ERR ~ 85 - garbage rather than an unwritten output - because the skinny
constant was still on a different interface. Both must name the same kernel, exactly as our pre-merge
dispatcher did.
I will commit this onto the repair branch and move the pin, so the branch carries the fix rather than just
pointing at it.
… kernel again 39588ef9 dropped our MoE matmul in favour of AMD's tiled/skinny pair, which mishandles n = 5, 17, 32 and 129. 56c3c8a3 points both matcher constants back at our ggml_mul_mat_id_f32_f32_wmma. test-backend-ops -b HRX0 -o MUL_MAT_ID 78/108 -> 108/108 GLM-4.7-Flash, default -ub 512 decode failure -> pp512 902.21 t/s, tg32 27.40 t/s The GLM row is engine#315: the all-NaN logits are gone and the -ub 128 workaround is no longer needed.
|
Pinned: |
Gate after the fix: 989/1073 -> 1019/1071, failures 84 -> 54Full
Exactly the 30
Which suggests the same lever applies again, and it is worth checking before anyone plans a kernel port: several of these names ( |
ggml_binary_f32 is a name both trees use and the merged corpus held AMD's file under it, so the ADD path ran AMD's implementation. 89d4f17c repoints that export at our pre-merge source. test-backend-ops -b HRX0 -o ADD 5 failures -> 22/22 passed GLM-4.7-Flash pp512/tg32 885.17 / 27.30 t/s, unchanged Not fixed this way, and recorded as such: ggml_get_rows_f32 (the same repoint makes GET_ROWS worse, 7 -> 24) and MUL_MAT (40 -> 42 alone).
Correction: the
|
…neutral The full gate is 1019 passed / 54 failed both before and after 89d4f17c, and my '5 -> 22/22' comparison was invalid: -o ADD selects 22 cases of a different class while the 5 [ADD]-class failures are the MUL_MAT_VEC_FUSION ones, which still fail. Keeping the pin on the two changes that have a measured effect (the JIT-cache fix and the mul_mat_id routing fix).
Correction: the
|
| prompt tokens | NaN events |
|---|---|
512 (llama-bench -p 512, single ubatch) |
0 |
| 558 | 2 (every generated token) |
| 666 | 2 |
| 774 | 2 |
| 882 | 2 |
| 839 (needle prompt) | 18 over three requests |
So the fix raised the batch limit - pre-fix a 269-token prompt already failed, post-fix 512 works - but every prompt from 558 up still produces all-NaN logits. #315 is not fixed by this pin, and the workaround still has a purpose.
Note the boundary coincides with the -ub 512 split (558 = 512 + 46): either the continuation batch with a non-empty KV cache is the trigger, or a >512 single batch is. I have not separated those two, and I am not going to claim which.
What the fix genuinely did, and what I am still claiming: test-backend-ops -b HRX0 -o MUL_MAT_ID 78/108 -> 108/108, the full gate 84 -> 54 failures, and a 512-token prompt that failed pre-fix now generates (pp512 902, tg32 27.4). That is real; "the NaN is gone" was not.
An attempted -ub 128 re-run at 839 tokens never reached the server (my readiness gate waited 1200s and the server had already exited), so whether the workaround still works post-fix is unmeasured rather than assumed either way.
Second correction, and a much better lead: the NaN is content-dependent, not length-dependentTwo measurements this round overturn what I said in the last two comments. 1. The same prompt, before and after the So the fix did not address the NaN at all. My "GLM now generates at the default ubatch" (round 44) and "the fix raised the limit from 269 to 512" (round 48) were both wrong: the 512-token case I measured with 2. The trigger is the prompt's content, not its length or the ubatch size.
Same model, same pins, same length: no NaN at any So the lead is routing concentration, not length. That fits everything: content-dependence, the apparent length-dependence (longer prose piles more assignments onto the same hot expert), why random prompts never fire it, and why the What still stands from the fix, measured: |
Verified state of this bump, and what it still costsFor the record, since this PR has been open a while and its title predates the last few commits. Landed on this branch and verified: the corpus/manifest repair ( What the bump still costs — measured in fresh build directories with the engine's own
The core sync is sound; AMD's Ready but unpushable: branch |
Corrected pin branch pushed — this PR's staged pin imports AMD's
|
|
Corrected pin is now up as a draft: #336 — |
|
Closing as superseded by #336. Do not land this PR. Your measurement is the decisive one, and it inverts this PR's premise: resolving the 15 conflicting I verified #336's composition independently, by tree hash rather than by eye: So Two things from this branch that are worth keeping, in case they are not already in #336's line:
Full write-up and the six harness bugs I hit (all mine, none of them the pins) are in |
1 participant
Moves both pinned submodules onto AMD's tested pair, merged with our commits. Opened as a draft on purpose — see the validation gate.
third_party/llama.cpp522dab47b04c4e95= AMDb802a507merged with our 159 commits, plusllama.cpp#94so it also carries the engine#315 fixthird_party/hrx-system98d05d94563afce1= AMD40a1a36cmerged with our libhrx knobshrx-system conflicts were 3 Loom AMDGPU target-info files, resolved to AMD: our commit there was
f1b558f19"[Loom] GFX11 wave64: drain ALU dependencies … (backport)" — a backport of a fix AMD has since landed. Our98d05d94alibhrx knobs commit does not conflict and is preserved.llama.cpp carried 75 conflict hunks: 11 files took AMD's side per-conflict with the auto-merge remainder preserved,
loom-libs/manifest.jsonbecame a semantic union (277 files / 215 exports, keeping 61 + 55 of ours, plus AMD's newsources/kernels/link_modules/plan_cases), and 3 superseded monoliths gave way to AMD'sops/flash_attention/*+motifs/flash_attention/*refactor (symbols still exist at the new paths).Validation gate — do not un-draft until this passes
Neither merged tip has been built or run. GitHub-hosted CI builds this without HRX, so CI cannot catch an HRX breakage. On Strix Halo:
Then this is the pin the release gate must be re-run against — it measures the build we would ship, which the currently-running pass does not.