Repository navigation
ggml-hrx: the f32_f32 WMMA cores accumulate in f32 (engine #284) - #81
Conversation
The f32_f32 WMMA cores carried their whole K accumulation in an f16 accumulator vector, which lowers to v_wmma_f16_16x16x16_f16. On gfx11 that instruction's accumulator keeps about 8 mantissa bits, and a long sum of small products cannot even be represented: every 16-wide step of 0.0234375 products adds 0.5 instead of 0.375 once the running sum passes 32. Reproduced from engine ggml-org#284 with MXFP4 weights of 0x77 (0.0234375) and 1.0 inputs, output_size 96, 1 token, 1 route: K exact f16 accumulator 2048 48.0 53.3125 2304 54.0 61.3125 2816 66.0 77.3125 (+17%) 2880 67.5 79.3125 A bf16 accumulation of the same 16-wide steps reproduces those numbers exactly, so the f16 accumulator is the root cause and the fix is to use the f32 accumulator of the same 16x16x16 WMMA. gfx11 wave64 f16->f32 WMMA uses a 4xf32 accumulator payload (rdna3_wmmar3_f32_16x16x16_f16_w64, v_wmma_f32_16x16x16_f16), so each core's accumulator becomes vector<4xf32> and the finished tile is staged in f32. The publish path already consumed f32, so the f16->f32 widen is gone. Covers the cores engine ggml-org#284 names plus the ones that share the motif: motifs/mul_mat_id_f32_f32_wmma_core.loom common.mul_mat_id.f32_f32_wmma motifs/mul_mat_f32_f32_wmma_core.loom common.mul_mat.f32_f32_wmma ops/mul_mat_swiglu_f32_f32_wmma.loom common.mul_mat_swiglu.f32_f32_wmma ops/mul_mat_id_swiglu_f32_f32_wmma.loom Verified by linking and compiling every affected export for gfx11-generic and disassembling the artifact: each v_wmma_f16_16x16x16_f16 became v_wmma_f32_16x16x16_f16 (x8 in the matmul cores, x16 in the swiglu cores), and every export that compiled before this change still compiles.
|
Adding the check that matters most: the same link+compile+disassemble for gfx1151, the part in engine ggml-org#284.
This is still instruction selection, not a numeric run — a Strix Halo box is |
|
One more build-level check, plus where existing coverage lives. The corpus build step itself. The Existing numeric coverage on hardware. Left alone deliberately. |
|
Static cost of the change, since I could not benchmark on hardware. Same link,
The f32 accumulator is smaller: −56 instructions (−4.8%) and −288 bytes, at This says nothing about the runtime rate of |
…lator The f32-accumulator change dropped encoding.layout.strided [1, 16] from the result fragment views. The fragment is stored as [channel, token]; the publish path reads the staged tile as [token][channel], so without the transposed layout every published vector took its values from the wrong element. A constant test with every token equal hides this; a case with fewer live tokens than the 16-wide tile shows it. This restores the strided layout on the f32 views in all four files (mul_mat, mul_mat_id cores; mul_mat_swiglu, mul_mat_id_swiglu ops). Known-answer cases on gfx1151 (iree-test-loom --sanitizer=access, MXFP4 weights 0x77 = 0.0234375, inputs 1.0, expected K * 0.0234375; mul_mat and swiglu with 6 tokens, mul_mat_id and mul_mat_id_swiglu with 1 token; K = 1152, 2048, 2816, 2880, 4096, 5120; 2 repeats each): - base f5b7f4a: 1/6 K exact per kernel (K=2816 gives 77.3125, not 66) - PR head 85f5983: 0/6 (element 1 / element 6 read as 0) - this commit: 6/6 exact for mul_mat_id, mul_mat, mul_mat_swiglu and mul_mat_id_swiglu Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Verified on strixhalo (gfx1151, balanced power mode 85 W). As posted, 85f5983 gives wrong results. I pushed a fix, b5669e0, onto this branch. The bug in 85f5983. The change removed Known-answer cases. Run with
The access sanitizer passes for all four kernels. Models (base vs f5b7f4a + 85f5983 + b5669e0):
Prompt speed. llama-bench, 3 interleaved rounds, medians:
All samples: gpt-oss pp base 1011.6/1006.5/1006.2, PR 971.1/965.8/967.5. 27B pp base 344.0/336.8/342.1, PR 353.2/354.5/349.2. |
|
Follow-up on the UD-IQ3_S check: that prompt does run the kernels this PR changes. I reran the same chat request ("capital of France", greedy, 1 request) with
So the matching IQ3_S result in the comment above is a real check of the changed kernels on a model, not a path that skips them. |
|
Conflicts with #85, which fixed the same issue (engine ggml-org#284) in mul_mat_id_f32_f32_wmma_core.loom by folding an f16 accumulator per 256-wide K tile into f32. This PR takes the true f32-accumulator approach in all four WMMA files. Resolving in favour of this PR, then re-verifying on the current tip (known-answer, KLD vs CPU, pp/tg A/B) before merging. |
Conflict in motifs/mul_mat_id_f32_f32_wmma_core.loom (#85 vs this PR): kept this PR's f32 accumulation (vector<4xf32> MMA results), which replaces #85's f16 MMA folded into f32 carries per 256-wide K tile. The other three files merged without conflict. Measured on strixhalo gfx1151 (performance mode), base 86eee89 vs this merge: - The pinned JIT (hrx-system 51b1739) compiles the f32 accumulator: v_wmma_f32_16x16x16_f16 in all 93 mul_mat and 72 mul_mat_id specializations that test-backend-ops compiled (base: v_wmma_f16_16x16x16_f16). - Known-answer cases (MXFP4 0x77, inputs 1.0, exact K x 0.0234375, K = 1152..5120, 6/17-token and 1-token/1-route tiles, iree-test-loom --sanitizer=access, 2 repeats): this merge 84/84; base 28/84 (mul_mat_id passes with #85, the other three kernels fail above K = 1152). - test-backend-ops -b HRX0 MUL_MAT 322/322, MUL_MAT_ID 108/108, MUL_MAT_VEC_FUSION 45/45, 2 runs each. - gpt-oss-20b KLD vs CPU (c512 b512, 8 chunks): 0.029296 -> 0.026932. - llama-bench pp512 (medians of 3): gpt-oss-20b 1045.8 -> 995.7 (-4.8%), Qwen3.8-27B UD-Q4_K_XL 355.3 -> 367.0 (+3.3%), Qwen3-Coder-30B 2060.8 -> 2058.8; tg128 unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged 1bit/hrx-vulkan-patched (86eee89, with #85, #82, #83) into this branch as d5a9932. The only conflict was On the pinned hrx-system 51b1739, the JIT compiles the Checks on strixhalo, performance mode, base 86eee89 vs merge:
|
A fix for 1bit-MONSTER/engine#284.
Root cause
ggml_mul_mat_id_f32_f32_wmmaand the other f32_f32 WMMA cores carried thewhole K accumulation in an f16 accumulator vector (
vector<8xf16>), whichlowers to
v_wmma_f16_16x16x16_f16. On gfx11 that accumulator keeps about8 mantissa bits: it is not f16 rounding. Simulating a bf16 accumulation of the
same 16-wide steps reproduces the issue's numbers exactly, while a true f16
accumulation of 0.375 steps is exact.
From the issue's setup (MXFP4 weights of 0x77 = 0.0234375, 1.0 inputs,
output_size 96, 1 token, 1 route):
Change
Use the f32 accumulator of the same 16x16x16 WMMA. gfx11 wave64 f16->f32 uses
a 4xf32 accumulator payload (
rdna3_wmmar3_f32_16x16x16_f16_w64,v_wmma_f32_16x16x16_f16), sovector<8xf16>becomesvector<4xf32>and thefinished tile is staged in a plain row-major f32 view. The publish path already
consumed f32, so the
vector.extfwiden is gone (the swiglu cores droppedtheir gate/up narrow->widen too).
Covers the core the issue reports plus the two it names as possibly the same
root cause, and the routed-expert swiglu that shares the motif:
motifs/mul_mat_id_f32_f32_wmma_core.loom—common.mul_mat_id.f32_f32_wmma(the reported kernel)motifs/mul_mat_f32_f32_wmma_core.loom—common.mul_mat.f32_f32_wmmaops/mul_mat_swiglu_f32_f32_wmma.loom—common.mul_mat_swiglu.f32_f32_wmmaops/mul_mat_id_swiglu_f32_f32_wmma.loom— routed-expert swigluVerification done here
Link + compile each affected export for
gfx11-generic, then disassemble:ggml_mul_mat_id_f32_f32_wmmav_wmma_f16_16x16x16_f16x8v_wmma_f32_16x16x16_f16x8ggml_mul_mat_f32_f32_wmmaggml_mul_mat_swiglu_f32_f32_wmmaggml_mul_mat_id_swiglu_f32_f32_wmmaggml_mul_mat_id_postops_f32_f32_wmmaggml_mul_mat_bias_f32_f32_wmmaggml_mul_mat_add_f32_f32_wmmaggml_mul_mat_bias_add_f32_f32_wmmallm_attention_v_matmul_set_rows_f32_f32_wmmaAll 21 exports that depend on the four changed files were compile-checked; every
one that compiled before still compiles (the handful that fail do so identically
on the base commit, from template-specialization constraints on the small
synthetic configs used to compile them standalone).
Not verified here
No gfx1151 box was available, so this PR has no numeric run on Strix Halo.
Before merge someone should re-run the issue's
calib.shK sweep to confirm thevalues become exact, and
llama-benchto measure the cost of the f32accumulator (RDNA3 may issue f16-accumulate WMMA at a higher rate). If it costs
too much, the alternative is periodic f16->f32 flushes, but the f32 accumulator
is the straightforward correct fix.