metal : fix idle threads in mul_mv_iq3_xxs for ne00 < 1024 - #28086
Conversation
| #define N_SG_IQ2_S 2 | ||
|
|
||
| #define N_R0_IQ3_XXS 4 | ||
| #define N_R0_IQ3_XXS 8 |
There was a problem hiding this comment.
For nb32 >= 32, the lane mapping is unchanged, but changing N_R0 from 4 to 8 doubles the rows per simdgroup and regresses performance on my device.
There was a problem hiding this comment.
I will revert this, and rerun the bench, thanks!
Can you please share test params/results that you got and hardware?
There was a problem hiding this comment.
I ran the following command on M4 Max:
test-backend-ops perf -b MTL0 -o MUL_MAT --test-file tests.txt
The tests.txt contains:
29 0 4096 1 1 1 0 2 18 512 4096 1 1 0 0 0 0 0 512 1 1 1 0 0 0 0 -
29 0 4096 1 1 1 0 2 18 768 4096 1 1 0 0 0 0 0 768 1 1 1 0 0 0 0 -
29 0 4096 1 1 1 0 2 18 1024 4096 1 1 0 0 0 0 0 1024 1 1 1 0 0 0 0 -
and results are:
| Parameters | Baseline | N_R0=8 | Latest PR |
|---|---|---|---|
| m=4096,n=1,k=512 (nb32=16) | 7.49 µs | 6.58 µs | 6.68 µs |
| m=4096,n=1,k=768 (nb32=24) | 9.09 µs | 9.67 µs | 9.20 µs |
| m=4096,n=1,k=1024 (nb32=32) | 8.76 µs | 9.46 µs | 8.68 µs |
The performance regression is gone now.
There was a problem hiding this comment.
Thanks for testing this!
Your M4 Max results match the intended behavior exactly: k=512 uses the split path and improves, while k=768 and k=1024 stay on the original path and remain at baseline.
I also reproduced the regression you found and changed the implementation so the normal kernel stays at N_R0=4; the 8-row version is now only used for the narrow split case.
On my M5, the regression is gone and the ne00=512 case keeps the performance gain. Thanks for catching it.
… for ne00/32 < 32 The plain kernel is unchanged from master (4 rows per simdgroup, one thread per chunk). The row-split mapping now lives in a separate kernel_mul_mv_iq3_xxs_f32_split instantiation with N_R0_IQ3_XXS_SPLIT = 8, and the host selects it only when ne00/32 < 32 and divides 32, so wide matrices keep the master kernel bit for bit.
8261e50 to
e4f4f1c
Compare
|
Is this valid only for |
|
@ggerganov Thank you for taking a look! Yes, the idea generalizes:
The K-quants have the same underlying issue, but use different lane mappings, so they would need a separate implementation. I suggest to extend this PR to the other iq kernels and handle the K-quants in a separate PR. Happy to do the follow up either way, WDYT? |
|
|
||
| template<int nr0, typename args_t> | ||
| // SPLIT: for nb32 < 32 (nb32 divides 32), 32/nb32 threads share each chunk and each takes a slice of the rows | ||
| template<int nr0, bool SPLIT, typename args_t> |
There was a problem hiding this comment.
Can you try to make the SPLIT a function constant instead of template argument? If the performance is the same, the FC would be preferable.
There was a problem hiding this comment.
Done, performance is unchanged on my M5.
… of a separate kernel
ggerganov
left a comment
There was a problem hiding this comment.
Nice! Looking forward to the generalization of this idea
Overview
This improves
iq3_xxsmul-mv thread utilization whennb32 < 32andnb32divides 32.Each thread currently takes one 32-element chunk based on its thread ID. At
ne00 = 512, there are only 16 chunks (nb32 = 16), so threads 16 through 31 have nothing to process. The patch assigns two threads to each chunk, with each thread covering half of thenr0output rows. Whennb32 >= 32, ornb32does not divide 32, the existing mapping is unchanged.The PR also changes
N_R0_IQ3_XXSfrom 4 to 8, so each simdgroup handles up to eight rows. This was tuned separately while testing; see the benchmark results below. I can revert it if this feels too specific.Tested on my 14inch MacBook Pro M5 24 GB , using Tiel-Coder-35B-A3B IQ3_XXS.
On a fixed replay of the same coding-agent workload, the
b10488development tree improved from 24.66 to 22.59 s/rep (−8.4% wall), with decode increasing from 65.6 to 73.9 tok/s and perplexity unchanged at 3.3969.Additional information
The same
(row, chunk)work is still computed once. Threads outside a row's assigned range contribute zero to the finalsimd_sum, and the full simdgroup reduction produces the intended row result.test-backend-opscomparisons against the CPU backend pass forMUL_MAT_IDandMUL_MAT, includingk = 256, the deepest split, andk = 768, the fallback path.Performance was measured on
upstream/master = daef7b6:N_R0 = 8) (µs)The stock
test-backend-ops perfkeeps the selected expert IDs constant across repetitions, so I used a local benchmark modification that rotates expert IDs between short timed batches to avoid repeatedly benchmarking the same cached weights. I can also submit the rotating-ID benchmark change separately if that would be useful.The
n = 1case is excluded because it was noisy on my setup.The measured case is
ne00 = 512(nb32 = 16). Otheriq3_xxsmatrices withnb32 < 32, wherenb32divides 32, take the same row-split path.The same
ix = tiisgthread assignment appears in six other kernels:iq2_xxs,iq2_xs,iq3_s,iq2_s,iq1_s, andiq1_m. They leave threads idle in the same way whennb32 < 32. I don't have a model that hits those kernels at smallne00, so they're left unchanged here and flagged for follow-up.Test methodology
llama-serverconfiguration for each run; results are the mean of 3 warm repetitions.n ≈ 2–4for MTP, up ton ≈ 24for ngram), withiq3_xxsffn_down_expsrunning across 37 blocks on each verify pass.b10488development tree; the kernel table was measured separately againstupstream/master = daef7b6.llama-perplexity -m Tiel-Coder-35B-A3B-MTP-UD-IQ3_XXS.gguf -f ppl_ref.txt -c 2048 -b 2048 --chunks 4 -ngl 999 -fa on -ctk q8_0 -ctv q8_0; the result remained unchanged at 3.3969.Requirements