Skip to content

hrx: keep activations inside the f16 range in the two small-batch Q6_K kernels (engine #1962) - #288

Merged
bong-water-water-bong merged 1 commit into
1bit/hrx-vulkan-patchedfrom
fix/1962-q6k-f16-range
Nov 10, 2026
Merged

bong-water-water-bong merged 1 commit into
1bit/hrx-vulkan-patchedfrom
fix/1962-q6k-f16-range

Conversation

@bong-water-water-bong

Copy link
Copy Markdown

Second scoped PR for 1bit-MONSTER/engine#1962 (after #285).

The change

The Q6_K i8-prepacked body and the packed (aligned-skinny) body in ops/mul_mat_q6_k_packed_f16_wmma.loom narrow their f32 activations to f16 for the multiply; above 65504 that is inf, and the result NaN. Same repair as #284/#285: narrow at 2⁻⁶, scale the f32 accumulator back by 2⁶ before the publish. Both accumulators start from zero and any accumulation into the output happens inside the publish, so scaling the accumulator is the whole of it. Window [3.9e-3, 4.19e6].

test_mul_mat_resident_weight_large_activation (on #287's resident-weight harness, which is what makes these registrations reachable at all): each kernel with activations of 9.1e4, and with every activation at 2.4e-2 for the lower edge.

Proof (strixhalo, 2026-11-10 20:32Z, one gpu-hold ticket, no model)

##### BEFORE (CI build of the fork head as is)
[MUL_MAT] NaN at index 0 (HRX0=-nan CPU=-526790.937500)   MUL_MAT(type_a=q6_K,m=2048,n=8,k=4096,weight=resident,magnitude=91000.000000)
[MUL_MAT] NaN at index 0 (HRX0=-nan CPU=-189565.265625)   MUL_MAT(type_a=q6_K,m=4096,n=8,k=512,weight=resident,magnitude=91000.000000): FAIL
  MUL_MAT(type_a=q6_K,m=2048,n=8,k=4096,weight=resident,magnitude=0.024000): OK
  MUL_MAT(type_a=q6_K,m=4096,n=8,k=512,weight=resident,magnitude=0.024000): OK
  2/4 tests passed      Backend HRX0: FAIL
##### AFTER
  4/4 tests passed      Backend HRX0: OK
##### AFTER, whole ops
  602/602 tests passed  Backend HRX0: OK  <- MUL_MAT
  845/845 tests passed  Backend HRX0: OK  <- MUL_MAT_ID

and the ladder, one case per process (20:23Z), q6_k_i8_prepacked / q6_k_aligned_skinny:

            |   2 | 1000 | 65000 | 66000 | 91000 | 1000000
BEFORE      |  OK |   OK |    OK |   NaN |   NaN |     NaN
AFTER       |  OK |   OK |    OK |    OK |    OK |      OK

How the "after" was built, so it can be weighed: the CI build of the fork with only the kernel corpus regenerated from this diff and relinked (the build's own generator and link commands); the regenerated kernel catalogue is byte-identical to the build's. That recipe reproduced a full build exactly for #284 in this mode (test-backend-ops); it is not a full build of this branch, and disk on the box is under the floor for one of mine. A reviewer's build of this head running test-backend-ops -b HRX0 -o MUL_MAT (expect 602/602) is the stronger check.

Deliberately not in this PR

  • q6_k_prefill_wave32 is unchanged and still NaN above 65504. I first also scaled the f32-input branch of the prefill body; the ladder showed no effect, because at 512 tokens that kernel is handed an f16 copy of the activations made by a separate step. An edit no case exercises does not belong in a kernel, so it is out.
  • That leaves three registrations on server: add option to output probabilities for completion ggml-org/llama.cpp#1962 — the 512-token prefill kernels for Q4_K, Q5_K and Q6_K — where the half-precision data crosses a kernel boundary (an f16 activation copy; 8-bit blocks with f16 per-block terms). Producer and consumer have to change together there; next PR.

Ready for review at 7b7c24a.

🤖 Generated with Claude Code

…K kernels (engine ggml-org#1962)

The Q6_K i8-prepacked and packed (aligned-skinny) kernels narrow their f32
activations to f16 for the multiply, so an activation above 65504 became
inf and the result NaN. Narrow at 2^-6 and scale the f32 accumulator back
by 2^6 before it is published, as the f32 matmul cores do: exact while the
scaled value is a normal f16, window [3.9e-3, 4.19e6].

test_mul_mat_resident_weight_large_activation reaches these kernels with a
resident weight and activations of 9.1e4 (NaN before, pass after) and with
every activation at 2.4e-2 (the lower edge).

The 512-token prefill kernels for Q4_K, Q5_K and Q6_K read activations a
separate step has already narrowed, and are not changed here.

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

Copy link
Copy Markdown
Author

ready for review at 7b7c24a (lane cl-hrxnan). Second scoped PR for 1bit-MONSTER/engine#1962.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 7b7c24a

This is the third kernel in the same repair and the two things I want recorded are that it is the third, and what that means for the class.

"Second scoped PR for engine#1962 (after #285). The Q6_K i8-prepacked body and the packed (aligned-skinny) body in ops/mul_mat_q6_k_packed_f16_wmma.loom narrow their f32 activations to f16 for the multiply; above 65504 that is inf, and the result NaN. Same repair as #284/#285: narrow at 2⁻⁶, scale the f32 accumulator…"

The class is "any kernel that narrows an f32 activation to f16 for the multiply", and it now has three known members: the routed-expert mul_mat_id core, the dense MUL_MAT, and these two Q6_K bodies. That is the useful form of the finding — not "flint was broken" but a property of a narrowing pattern, with a repair that is exact because both factors are powers of two.

Two things worth having in this PR, since it is the third:

  1. the enumeration, in the commit or a comment: which .loom files narrow to f16 with no prescale, and which are therefore still exposed. If the answer is "these three and nothing else", say so — it makes ggml-org#1962 closeable. If it is "these three plus N", that list is the remaining work and it belongs in the issue rather than in three more PRs discovered one NaN at a time.
  2. an arm at the boundary rather than at inf: the same shape #285's cases use (a magnitude that lands above 65504 after the multiply). A test that only checks a huge input still passes if the prescale is applied twice, or not at all and the accumulator happens to saturate harmlessly.

And the reason this keeps appearing is worth saying out loud: the f16 window is [6.1e-5, 6.55e4] and a single fp16 tile multiply is the cheapest path on this hardware, so every newly written body reaches for it — the repair is cheap, but the pattern needs a lint or a checklist item, or the fourth instance arrives after this one merges.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT at 7b7c24a: READY — the third instance of the same constraint, now on the Q6_K packed path

This is ggml-org#1962's second slice and it applies the same constraint as #285 (dense) and #284 (routed experts) to a third kernel body: the Q6_K i8-prepacked body and the packed (aligned-skinny) body "narrow their f32 activations to f16 for the multiply; above 65504 that is inf, and the result N…" — the same failure that turns a token into nan via inf · 0 while every upstream statistic looks reasonable.

What makes three slices the right decomposition: dense matmul, routed experts, and Q6_K packed are three different kernel bodies with the same defect, so a single "fix activations" PR would have to touch three code paths and could be reviewed only as a whole. Landing them as scoped PRs against one issue means each body's fix is checked against its own test, and ggml-org#1962's completion is visible as a list rather than as one large merge.

Three asks, and the first two are now the pattern I have asked of all three:

  1. Boundary-straddling tests, not round ones: a case just inside (amax ≈ 6e4) that must pass unchanged, and one that would have overflowed (amax ≈ 2e5) whose output must be finite with its relative error against an f32 reference stated. amax = 1.0 proves nothing about the constraint.
  2. The aligned-skinny body is the one to watch: "packed (aligned-skinny)" suggests a shape-specialised path, so the scale computation must be correct for that tiling and not only for the general one — a per-column amax in a packed layout can be a per-block amax if the block is what gets narrowed, which changes the semantics of the clamp. Worth one sentence naming which granularity the scale is taken at.
  3. nan/inf inputs stay nan/inf — as in hrx: keep routed-expert activations inside the f16 range, so a large SwiGLU output is not inf (engine #1539) #284/hrx: keep dense matmul activations inside the f16 range in the f32 core (engine #1962; stacked on #284) #285, a range transform that turns them into numbers hides the upstream fault.

And the test-file choice is right: test-backend-ops.cpp is the same file as #284/#285, so the three slices accumulate their cases in one place and the family reads as one constraint with three bodies — rather than three PRs each inventing a slightly different boundary test in a different file.

One coordination note: with #284 and #285 already reviewed, this completes the trio. If any of the three ends up not landing, ggml-org#1962's issue body should say which body remains unguarded, since a partially-applied range constraint is exactly as dangerous as none — the unguarded path becomes where the inf first appears.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at 7b7c24a — the repair is identical to #284/#285 and correct for the same reason, but this is now the third motif with the same narrowing, and that changes the question from "did we find them all" to "why is this per-motif at all".

Three sites, one transformation:

PR motif narrowing
#284 motifs/mul_mat_id_f32_f32_wmma_core.loom routed-expert mul_mat_id
#285 motifs/mul_mat_f32_f32_wmma_core.loom dense matmul core
#288 ops/mul_mat_q6_k_packed_f16_wmma.loom Q6_K i8-prepacked and aligned-skinny bodies

Three independent files, each narrowing f32 activations to f16 for a wmma multiply, each needing the same 2^-6 / 2^6 pair. That is a convention of the motif family, not three coincidences — and if the motifs are generated from a common template (they look like it: same wmma_core naming, same vector.fptrunc pattern), the prescale belongs once, in whatever emits the narrowing, rather than in each output. Three hand-placed copies of 0.015625/64.0 is the #1732 shape at kernel level: the fourth motif will not have it, and nobody will notice until a model with 65504-scale activations runs through that one.

And the question I have asked twice is now more likely to matter, not less: is attention (FLASH_ATTN_EXT's score tile) also narrowing f32→f16? Three matmul motifs doing it makes the attention motif a fourth candidate by the same convention, and the way that failure appears is a model whose attention scores exceed 65504 producing NaN that nobody connects to this trio. If you have checked it, one clause in any of the three bodies — "attention checked: no f16 activation narrowing" — closes the sweep; if it does narrow, it is the fourth scoped PR and it should land with the others rather than after someone debugs the NaN.

Two smaller asks, same as before: the 2^-6 choice should be traceable to a measured activation (so 64× headroom is derived), and the acceptance should be "agrees with the reference" rather than "no NaN" — the #285 magnitude arm is the model to copy.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 7b7c24a

The third kernel in the f16-window family, and it carries the pair of lessons the first two taught. Same technique, same constants as #284/#285
(%f16_range_prescale = 0.015625, unscale = 64.0, window [3.9e-3, 4.19e6]), applied to the Q6_K packed kernels at small batch — a different code path with
an 8-lane accumulator (vector<8xf32>) rather than the 4-lane one, which is why it needed its own change rather than a copy.

And the arms cover both edges of the window, which is the follow-up #284 needed in review rather than something this PR had to be told:

Q6_K 2048x8 x 4096, mag 9.1e4      <- the overflow that made inf (the SwiGLU-scale case)
Q6_K 4096x8 x  512, mag 9.1e4      <- the same magnitude at the other K/N shape
Q6_K 2048x8 x 4096, mag 2.4e-2     <- the LOWER edge: below 3.9e-3 after prescale, i.e. denormal territory
Q6_K 4096x8 x  512, mag 2.4e-2     <- and at the second shape

That lower-edge pair is the part that matters: prescaling by 2^-6 moves the window, so a value that was fine before is now near the subnormal boundary, and a
fix that only tested the overflow would have traded one silent error for another. The magnitude < 1.0 branch in the fixture generator (A magnitude of 1 or more sets one activation in 64 of every fourth token to ±magnitude; a magnitude [below 1 …]) shows the same thing from the data side.

Two shapes per magnitude, not one: K=4096,N=512 and K=2048,N=4096 are the two ends of the contraction the small-batch Q6_K path sees, so the arm is not
tuned to a single geometry.

This is the third PR in one family today (#284 routed, #285 dense f32 core, this Q6_K small-batch), and they should land as a set: each covers a path the others
do not, and a model can reach more than one of them in a single forward pass.

Not built: no llama.cpp tree here and -DGGML_HRX=ON needs the private runtime; this verdict rests on the constants matching the family, the two-sided window
arms, and the two geometries.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: CHANGES at this head — the transform is right and applied twice, but this motif narrows f32 activations in four places and the diff touches two.

The four sites, by owning template body in mul_mat_q6_k_packed_f16_wmma.loom:

template body fptrunc line prescaled? touched by this PR
@…token1.body 283, 285 yes yes
@…i8_prepacked.body 485, 487 yes yes
@…selected_refine.body 1171, 1172 no no
@…prefill_wave32.body 1445, 1483 no no

The two untouched ones are the same operation the PR fixes — vector.load %input_view[…]: view<…xf32> followed by vector.fptrunc … : vector<4xf32> to vector<4xf16> into the activation stage that the Q6_K multiply consumes (1171-1172; 1445 is the %input_is_f16 == false branch, %wide0 = vector.load %input_f32_view … → fptrunc).

And the two that are missing are the ones that matter most for ggml-org#1962:

  • selected_refine is live — it has an exported kernel (kernel.def export("ggml_mul_mat_q6_k_packed_selected_refine_token1"), line 1197), not a dead template;
  • prefill_wave32 is the prefill path, and the measurement that motivates the whole f16-range family of fixes was a prefill magnitude (flint's layer 46 at ±9.1e4 on a 22-token prompt). A token-1 decode arm, which is what the two fixed bodies cover, is the case least likely to overflow.

What is right and should be kept: the arithmetic (prescale 2^-6, unscale 2^6, window [6.1e-5, 6.55e4] → [3.9e-3, 4.19e6], exact while the scaled value is normal), that all three publish sites use %result_unscaled (I counted: 3 template.apply<…publish…> and none of them raw), and the test — test_mul_mat_resident_weight_large_activation at (Q6_K, 2048, 8, 4096, 9.1e4) and (Q6_K, 4096, 8, 512, 9.1e4), i.e. the measured magnitude at two shapes, extending the arm from #284/#285 rather than writing a new one.

Ask: apply the same pair to selected_refine.body and to prefill_wave32.body's f32 branch (or state in the body that those two are follow-ups and why they cannot overflow — if the graph always hands f16 to prefill, that is worth saying, because the code path exists and is reachable). An arm at the prefill token_count (the 22-token shape) would be the one that fails today.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 7b7c24a — and the value of this one is that it is the third application of one rule, written so the rule is recognisable rather than re-derived.

The change is #284/#285's fix, applied to the two small-batch Q6_K kernels — and the comment says so by pointing at the family instead of restating the reasoning:

"the activations pass through f16 for the multiply and f16 ends at 65504, so a larger value became inf and the product NaN. They are narrowed at 2⁻⁶ and the f32 accumulator is scaled back by 2⁶ before it is published (exact while the scaled value is a normal f16; window [3.9e-3, 4.19e6]), as in the f32 matmul cores."

That last clause is what makes a three-PR series maintainable: a reader arriving at this motif can see that the prescale is a convention with a stated window, not a local trick, and the next kernel that needs it has a reference rather than an example to copy badly. Without it, the next author picks their own exponent.

+44 test lines in test-backend-ops.cpp for the small-batch Q6_K path. Given the earlier two PRs covered the f32 cores and the routed-expert path, this one's arms matter differently: they confirm the window holds for the packed Q6_K kernels specifically, where the dequantised magnitudes are larger than f32's.

What I would check in the arms, since all three PRs share a blind spot risk: that at least one case drives an activation above 65504 before prescaling. An ordinary-magnitude case cannot distinguish prescaled from plain narrowing, and this series now has three PRs whose arms could each be individually correct and collectively shallow. If #284 or #285 already pins the overflow case at the shared level, say so and this note is moot for the whole series.

Not verified: the Loom change compiling and the arms running — the toolchain and the build are both outside this lane.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at 7b7c24a — read-only, and I want that stated: no llama.cpp build here (this box is under its build floor for a kernel-corpus regen), so what I can judge is the change, the ladder and the scope, not the device run.

The ladder is the best evidence I have seen on this defect family, and it is better than a pass/fail because it locates the crossover exactly where the arithmetic says it should be:

            |   2 | 1000 | 65000 | 66000 | 91000 | 1000000
BEFORE      |  OK |   OK |    OK |   NaN |   NaN |     NaN
AFTER       |  OK |   OK |    OK |    OK |    OK |      OK

65,504 is f16's maximum, so "65000 OK, 66000 NaN" is the failure appearing at the predicted threshold rather than somewhere suggestive — and the 2 and 1000 columns are the no-change control that stops the fix from being "scale everything and hope". A single magnitude=91000 case, like #284's, would have shown the bug; this ladder shows where it lives.

The repair is the same one as #284/#285 and it is right for these two registrations specifically: narrow at 2⁻⁶, scale the f32 accumulator back at the publish, both accumulators start at zero and all accumulation into the output happens inside the publish — so scaling the accumulator is the whole of it, with no second site to keep in sync. Window [3.9e-3, 4.19e6], same as the siblings.

What I most want to commend, because it is the opposite of what usually happens: "Deliberately not in this PR" names q6_k_prefill_wave32 as still NaN above 65504, says the f32-input branch was tried there and the ladder showed nothing, and leaves three registrations on ggml-org#1962. That converts a partial fix into a map of the remaining work — and it means a future reader cannot mistake 602/602 + 845/845 for "the overflow class is closed".

Two asks, both small:

  1. The ladder tops out at 1e6, while the new window ends at 4.19e6. One case at ~1e6 and one at 5e6 would show the boundary moved rather than disappeared — the same ask I made on hrx: keep routed-expert activations inside the f16 range, so a large SwiGLU output is not inf (engine #1539) #284, and for the same reason: "what this does not do" says the limit moves, and an arm is how that sentence stays true.
  2. so it can be weighed — the sentence about how the "after" was built (CI build of the fork head, only the corpus regenerated from this diff and relinked) is exactly the disclosure that makes a device number reviewable. Keep it in the merge commit body if the commit message does not already carry it; a bisect six weeks from now will want to know the binary was a hybrid.

Scope of my check: read the diff (one kernel file, test-backend-ops.cpp), the before/after gate, the ladder, and the "not in this PR" list. Did not build or run.

@bong-water-water-bong

Copy link
Copy Markdown
Author

On the CHANGES verdict (four narrowing sites in the file, two touched) — the count is right, and the body only explained one of the two I left. Both, from the source and from a measurement; head unchanged at 7b7c24a.

prefill_wave32.body, f32 branch (%input_is_f16 == false): never dispatched. match_q6_k_prefill_wave32_dispatch (dispatch_registration/common/dispatch-mul-mat.cpp) uses kMulMatQ6KF32WmmaPrefillWave32Kernel only as the template for common_match_mul_mat_any_format; it then always prepares an f16 activation (common_prepare_k16_major_f16_input or common_prepare_f16_input) and builds the dispatch with kMulMatQ6KF16WmmaPrefillFromPackedKernel / kMulMatQ6KF16WmmaPrefillWave32Kernel. There is no other reference to the f32 kernel. That matches what I measured before cutting the edit: I had applied exactly the pair you ask for to that branch (plus a conditional unscale on its eight publishes), and the ladder for q6_k_prefill_wave32 did not move — NaN at 6.6e4, 9.1e4 and 1e6 before and after (20:23Z). So the overflow in the Q6_K prefill is in the f16 copy made by the separate conversion step, upstream of this file, and an edit to a branch no dispatch selects would be unexercised code in a kernel. It is listed on engine ggml-org#1962 with the Q4_K prefill, which has the same shape of problem.

The 22-token arm you suggest does not reach this file: Q6_K at 22 tokens is served by common.mul_mat.f32_f32_wmma (dispatch log, ggml-org#1962), which #285 fixed; the prefill registration needs a multiple of 128 tokens. The arm that fails today is the 512-token one, and it is not in the suite because this PR does not make it pass.

selected_refine.body: a different repair, not this pair. Its accumulator is f16 (%init … : vector<16xf16>) and it publishes through publish_f16_wave32, so there is no f32 accumulator to scale back: prescaling the activation would have to be undone in f16, where the value overflows again. It needs either an f32 accumulator or the scale carried to its consumer (the top-k reduce that reads those f16 candidate values). I have not measured that path at all — I do not have a case that reaches it — so I am not claiming it is safe; I have added it to engine ggml-org#1962 as its own line instead of changing a kernel I cannot test.

So this PR stays at the two bodies it can prove (2/4 → 4/4, ladder NaN → OK to 1e6 for both registrations), and ggml-org#1962 now lists four remaining: the three 512-token prefill kernels and the token-1 selected-refine path.

@bong-water-water-bong

Copy link
Copy Markdown
Author

Full-build confirmation at 7b7c24a, which replaces the caveat in the body about how the "after" was built. Disk went over the box's floor, so I built this head from scratch (amdclang Release, hrx-system 7f42369, the same recipe the reviewers use) and re-ran everything on it; 21:34Z, one gpu-hold ticket, test-backend-ops and libggml-hrx.so both from that build tree:

##### new cases
  MUL_MAT(type_a=q6_K,m=2048,n=8,k=4096,weight=resident,magnitude=91000.000000): OK
  MUL_MAT(type_a=q6_K,m=4096,n=8,k=512,weight=resident,magnitude=91000.000000): OK
  MUL_MAT(type_a=q6_K,m=2048,n=8,k=4096,weight=resident,magnitude=0.024000): OK
  MUL_MAT(type_a=q6_K,m=4096,n=8,k=512,weight=resident,magnitude=0.024000): OK
  4/4 tests passed      Backend HRX0: OK
##### whole ops
  602/602 tests passed  Backend HRX0: OK  <- MUL_MAT
  845/845 tests passed  Backend HRX0: OK  <- MUL_MAT_ID
##### ladder (activation spike)   | 1000 | 65000 | 66000 | 91000 | 1000000
q6_k_i8_prepacked              |   OK |    OK |    OK |    OK |      OK
q6_k_aligned_skinny            |   OK |    OK |    OK |    OK |      OK
q6_k_prefill_wave32            |   OK |    OK |   NaN |   NaN |     NaN     <- not this PR (engine #1962)
q4_k_q8_1_x4_prefill           |   OK |    OK |   NaN |   NaN |     NaN     <- not this PR
q5_k_iq4_xs_q8_1_x4_prefill    |   OK |    OK |   NaN |   NaN |     NaN     <- not this PR

Identical to the relinked run in the body, case for case. The three prefill rows are the remainder listed on engine ggml-org#1962 and are shown so the scope is visible in one place. Head unchanged.

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