Repository navigation
sycl: fix prompt-processing regression after Q4_0/Q8_0 weight reorder - #30374
Conversation
github-actions
Bot
added
ggml
|
Hi @marcusds, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
This is the only open PR I have. |
6fb59d9 to
dd59f65
Compare
marcusds
changed the title
dd59f65 to
2517f14
Compare
|
Tested on Intel Arc Pro B70 (BMG, 32 GB), oneAPI 2026.1.1, Level Zero. Base = master-side SYCL for the files this PR touches (my branch carries unrelated SYCL patches; none of them touch the Q4_0/Q8_0 reorder dequant kernels). The Q8_0 half of this reproduces here, and larger than on B50:
So +75% on post-decode pp512 for this model, decode unchanged. The plain Spark-X2.5-4B Q8_0 did not show the d1 collapse on either binary here (7121 -> 7493 base), so whatever gates the slow path does not trigger for that one on B70; the uncensored model is the clean repro.
One nitpick below. One question on the correctness section: the description says the dequantized values are unchanged, verified on Q4_0. On Q8_0 I see a small but consistent perplexity shift with the PR (wikitext 4 chunks,
All shifts positive. Could |
2517f14 to
0693472
Compare
|
Thank you for taking the time to review this. You were right about the rounding. My review cleanup had switched both kernels to the shared I couldn't reproduce the shift on my side though, even using (toandev/Spark-X2.5-4B-Uncensored-GGUF, Q8_0). Arc Pro B50, wikitext-2,
My absolute values are about double yours, so we may not be running the same file or setup. Could you share which GGUF you used? The slowdown and fix do reproduce with your model here (
Full-run checks on the updated branch, Arc Pro B50, master vs PR:
Also, you mentioned a nitpick below, but maybe it wasn't posted? |
|
One Correction: the perplexity table in my earlier comment was mislabeled — those runs actually used the plain The model you asked about: Your fix verified on B70. I cherry-picked the updated head ( uncensored model,
stock model, same flags:
Deltas are now mixed-sign and within the run-to-run spread I measure on the base binary alone (the stock model moves ~0.5% between identical base runs at these flags), so I read this as the float-math restoration having removed the systematic positive shift — consistent with your half-precision-helper diagnosis, which also explains why I saw it only under The performance fix is unchanged on the new head, same repro as before ( On the nitpick: that line was a leftover from an earlier draft - the correctness question was the only substantive point, and it is now resolved. Nothing outstanding from my side; the PR looks good from here. |
0693472 to
12acc41
Compare
arthw
left a comment
arthw
left a comment
There was a problem hiding this comment.
It's good job!
There is perf increase of PP512 on Arc770 for Q4_0 and Q8_0, no impact of Q4_K.
| GGUF | Test | fa | Base t/s | Primary t/s | Increase Rate (Primary vs Base) |
|---|---|---|---|---|---|
| llama-2-7b.Q4_0.gguf | pp512 | 0 | 807.15 | 904.61 | 12.07% |
| llama-2-7b.Q4_0.gguf | pp512 | 1 | 955.34 | 956.66 | 0.14% |
| llama-2-7b.Q4_0.gguf | tg128 | 0 | 47.74 | 47.84 | 0.21% |
| llama-2-7b.Q4_0.gguf | tg128 | 1 | 54.37 | 54.30 | -0.13% |
| GGUF | Test | fa | Base t/s | Primary t/s | Increase Rate (Primary vs Base) |
|---|---|---|---|---|---|
| Meta-Llama-3-8B.Q8_0.gguf | pp512 | 0 | 797.55 | 865.22 | 8.48% |
| Meta-Llama-3-8B.Q8_0.gguf | pp512 | 1 | 877.20 | 876.86 | -0.04% |
| Meta-Llama-3-8B.Q8_0.gguf | tg128 | 0 | 36.79 | 36.78 | -0.03% |
| Meta-Llama-3-8B.Q8_0.gguf | tg128 | 1 | 39.29 | 39.31 | 0.05% |
Thank you!
Once the first token has been generated, prompt processing of a Q4_0 model drops by about 40% and stays there, so every multi-turn chat runs at the slow rate. Decode reorders the Q4_0 weights, and from then on every prompt batch dequantizes them to f16 through dequantize_row_q4_0_sycl_reorder before the GEMM. That kernel gave each thread a whole 32-value block, so neighbouring threads read and wrote 16/32 elements apart. Use one thread per qs byte instead, the same mapping as dequantize_block<dequantize_q4_0>, which in the reorder layout makes both the reads and the writes contiguous across threads. The math stays in float like the old kernel, so the output is bit-identical; the dequantize_q4_0_reorder helper computes in half with GGML_SYCL_F16, which would change f32 output. The kernel launches with ceil_div and downsample_sycl_global_range like convert_unary_nc_sycl, and loops over the global range so a shrunken work-group still covers every byte. The fp32 GEMM fallback now asserts that a reordered src0 is converted with its full row range, since the reorder kernels locate the scales from the element count. Qwen3.5-9B Q4_0, Arc Pro B50, llama-bench -fa 1 -ctk/-ctv q8_0 -ub 1024: pp512 before any decode 1029 -> 1028 t/s pp512 after a decode 607 -> 1005 t/s tg32 33 -> 33 t/s llama-server, 1490-token prompt after a decode: 767 -> 1150 t/s. wikitext-2 perplexity and greedy output on 7k/17k-token prompts are identical to master; test-backend-ops MUL_MAT passes.
Same regression as the reordered Q4_0 kernel: once decode has reordered a Q8_0 weight, every later prompt batch dequantizes it through a kernel that gave each thread a whole 32-value block, so neighbouring threads read and wrote 32 elements apart and prompt processing stayed slow for the rest of the session. Use two values per thread, the same mapping as dequantize_block<dequantize_q8_0>; in the reorder layout the qs of all blocks are contiguous, so both the reads and the writes are contiguous across threads. The math stays in float like the old kernel, so the output is bit-identical; the dequantize_q8_0_reorder helper computes in half with GGML_SYCL_F16, which would change f32 output. Launch with ceil_div and downsample_sycl_global_range like convert_unary_nc_sycl and loop over the global range so a shrunken work-group still covers every value. Qwen3.5-2B Q8_0, Arc Pro B50, llama-bench -fa 1 -ctk/-ctv q8_0 -ub 1024: pp512 before any decode 4084 -> 4092 t/s pp512 after a decode 3117 -> 4161 t/s tg32 81 -> 81 t/s llama-server, 1490-token prompt after a decode: 3506 -> 4335 t/s. wikitext-2 perplexity and greedy output on 7k/17k-token prompts are identical to master; test-backend-ops MUL_MAT passes.
It only fires for row-split tensors, which are never reordered.
3190c44 to
35e0908
Compare
arthw
added
the
merge ready
…own copy of our work Upstream added an IQ3_S / IQ3_XXS weight reorder (ggml-org#29107), a Q8_0 MoE reorder (ggml-org#29337) and wide q4_K / q5_K dequant stores (ggml-org#29696), all of which this branch already carries. The replay kept our side of each conflicting hunk but left upstream's copies next to ours; this removes them. - IQ3: our reorder, MMVQ, MoE GEMV and dequant stay, upstream's go. Upstream's IQ3_S layout puts d before the scales and ours after, so the two must not mix. - Q8_0 MoE: one reorder_qw_q8_0_moe (upstream's, same layout); our MoE reorder GEMV is kept. - dequant: our wide K-quant and q8_0 reorder dequant are kept (they also cover the reordered layouts); upstream's q4_0 reorder fix (ggml-org#30374) stays. It also restores upstream's side where the replay took ours: the f16 src1 pointer in ggml_sycl_op_mul_mat_sycl (ggml-org#29889), SET_ROWS on all threads (ggml-org#30454), and the lilicorr block size and top_k in the DFlash draft (ggml-org#30506). Assisted-by: Claude Opus 5.5
…own copy of our work Upstream added an IQ3_S / IQ3_XXS weight reorder (ggml-org#29107), a Q8_0 MoE reorder (ggml-org#29337) and wide q4_K / q5_K dequant stores (ggml-org#29696), all of which this branch already carries. The replay kept our side of each conflicting hunk but left upstream's copies next to ours; this removes them. - IQ3: our reorder, MMVQ, MoE GEMV and dequant stay, upstream's go. Upstream's IQ3_S layout puts d before the scales and ours after, so the two must not mix. - Q8_0 MoE: one reorder_qw_q8_0_moe (upstream's, same layout); our MoE reorder GEMV is kept. - dequant: our wide K-quant and q8_0 reorder dequant are kept (they also cover the reordered layouts); upstream's q4_0 reorder fix (ggml-org#30374) stays. It also restores upstream's side where the replay took ours: the f16 src1 pointer in ggml_sycl_op_mul_mat_sycl (ggml-org#29889), SET_ROWS on all threads (ggml-org#30454), and the lilicorr block size and top_k in the DFlash draft (ggml-org#30506). Assisted-by: Claude Opus 5.5
Overview
This PR fixes a prompt-processing slowdown on Q4_0 and Q8_0 models that starts after the first decode and persists for the rest of the session, so every multi-turn chat runs at the slow rate.
On SYCL, once a Q4_0 or Q8_0 weight has been reordered by decode, every later prompt batch dequantizes it to f16 through
dequantize_row_q4_0_sycl_reorder/dequantize_row_q8_0_sycl_reorderbefore the GEMM. Those kernels gave each work-item a whole 32-value block in 16-wide work-groups, so a sub-group's loads were 16 or 32 bytes apart and its stores 64 bytes apart. Prompt processing dropped by ~40% on Q4_0 and ~25% on Q8_0 after the first decode and stayed there (the fix speeds it back up by 66% and 33%).The kernels now use the same per-element mapping as
dequantize_block<dequantize_q4_0>/dequantize_block<dequantize_q8_0>for the plain layout (one work-item per packed byte for Q4_0, two values per work-item for Q8_0) in 256-wide work-groups, so reads and writes are contiguous across a sub-group. They launch withceil_div+downsample_sycl_global_rangelikeconvert_unary_nc_sycl, and loop over the global range so a shrunken work-group still covers every element.The math stays in float exactly as in the old kernels, so the dequantized values are bit-identical on both the f16 and fp32 paths. The
dequantize_q4_0_reorder/dequantize_q8_0_reorderhelpers are deliberately not used here, since they compute in half withGGML_SYCL_F16, which would change fp32 output.The fp32 GEMM fallback now asserts that a reordered
src0is converted with its full row range, since the reorder kernels locate the scales from the element count. The fp16 path already had this check.Additional information
I noticed a 40% slowdown on Arc B50, and was also able to measure the slowdown on UHD 730 iGPU.
Repro with:
llama-bench -m <model> -ngl 99 -fa 1 -ctk q8_0 -ctv q8_0 -ub 1024 -p 512 -n 32 -d 0,1Arc Pro B50 (Battlemage), Qwen3.5-9B Q4_0:
Arc Pro B50, Qwen3.5-2B Q8_0:
Correctness
Arc Pro B50, weights reordered by warmup, master vs this PR:
test-backend-ops -o MUL_MAT: 2272/2272 pass on SYCL0.Requirements