Repository navigation
Conversation
|
moving to review state, adding @ORippler and @gaugarg-nv for review |
ORippler
left a comment
There was a problem hiding this comment.
- What's your proposal of extending this to other data-types? Ideally, we want to apply this fusion to all supported
ggml_type's in the long term, and I fear simply adding a new kernel will fragment things. - We should look to make this code potentially compatible with HIP parts as well. I guess current blockers on this front are the async copies.
| return use_mul_mat_vec_f; | ||
| } | ||
|
|
||
| static bool ggml_cuda_mul_mat_bad_padding_clear(const ggml_tensor * src0) { |
There was a problem hiding this comment.
| static bool ggml_cuda_mul_mat_bad_padding_clear(const ggml_tensor * src0) { | |
| static bool ggml_cuda_mul_mat_bad_padding_clear(const ggml_tensor * src) { |
The helper only access a single tensor, so suffix is unneeded
| if (i + 2 < cgraph->n_nodes && cgraph->nodes[i + 2]->op == GGML_OP_GLU) { | ||
| ggml_tensor * glu = cgraph->nodes[i + 2]; | ||
| ggml_tensor * gate = cgraph->nodes[i]; | ||
| ggml_tensor * up = cgraph->nodes[i + 1]; | ||
| if (glu->src[0] == gate && glu->src[1] == up && | ||
| ggml_cuda_mul_mat_q_gate_up_swiglu_matches(up, gate, glu, cuda_ctx->device)) { | ||
| params->add_alloc_dep(params->user_data, up->src[1], glu); | ||
| i += 2; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Please do add_alloc_dep for both orderings of gate/up being passed into GLU
There was a problem hiding this comment.
Done, graph_optimize now accepts gate/up in either order before adding the alloc dep
| mmq_gate_up_swiglu_copy_y<J, nwarps, warp_size>(y + ncols_y * (kb0 * qk / QK8_1_MMQ) * sz, ncols_y, tile_y0); | ||
|
|
||
| mmq_gate_up_swiglu_load_x<type, J, fallback>(x_up, tile_x, offset_x + kb0, tile_x_max_i, stride_row_x); | ||
| cp_async_wait_all(); |
There was a problem hiding this comment.
Is the register pressure/L1 utilization so high at this point to warrant an async copy? What's the perf with regular copies? Note LDGSTS requires 128-byte alignment to be SMEM-conflict-free iirc, at least if done via cuda::memcpy_async.
Can use NCU to analyze this
| static constexpr int GGML_CUDA_MMQ_GATE_UP_SWIGLU_J_MAX = 64; | ||
| static constexpr int GGML_CUDA_MMQ_GATE_UP_SWIGLU_I = 64; |
There was a problem hiding this comment.
How were these magic constants determined?
There was a problem hiding this comment.
They are no longer hard-coded: the fused config is derived from the stock per-arch config (ggml_cuda_mmq_get_config_fusion halves threads and rows, e.g. 128 threads / I = 64 at two blocks per SM on
Ampere+). GGML_CUDA_MMQ_FUSION_J_MAX = 64 is the register limit: with gate and up accumulators per thread, J = 64 already uses 255 registers without spills (J = 8/16/32: 213/253/254); J = 128 would double
the accumulators. The half-size config beat the stock config with two accumulators (Spark pp16K +8.1% vs +5.8%), and forcing the current kernel to one block per SM makes it ~45% slower.
There was a problem hiding this comment.
Effectively we are now testing mul_mat_fusion, and no longer mul_mat_vec. Please rename or justify why current naming is still accurate
There was a problem hiding this comment.
The rename is done, and the latest commit also widened what the test covers.
| for (int kb0 = 0; kb0 < kb0_stop; kb0 += blocks_per_iter) { | ||
| mmq_gate_up_swiglu_copy_y<J, nwarps, warp_size>(y + ncols_y * (kb0 * qk / QK8_1_MMQ) * sz, ncols_y, tile_y0); | ||
|
|
||
| mmq_gate_up_swiglu_load_x<type, J, fallback>(x_up, tile_x, offset_x + kb0, tile_x_max_i, stride_row_x); | ||
| cp_async_wait_all(); | ||
| __syncthreads(); | ||
|
|
||
| ggml_cuda_mmq_vec_dot_q8_1_q8_1_mma<type, J, fallback>(tile_x, tile_y0, sum_up, 0); | ||
| ggml_cuda_mmq_vec_dot_q8_1_q8_1_mma<type, J, fallback>(tile_x, tile_y1, sum_up, MMQ_TILE_NE_K); | ||
| __syncthreads(); | ||
|
|
||
| mmq_gate_up_swiglu_load_x<type, J, fallback>(x_gate, tile_x, offset_x + kb0, tile_x_max_i, stride_row_x); | ||
| __syncthreads(); | ||
|
|
||
| ggml_cuda_mmq_vec_dot_q8_1_q8_1_mma<type, J, fallback>(tile_x, tile_y0, sum_gate, 0); | ||
| ggml_cuda_mmq_vec_dot_q8_1_q8_1_mma<type, J, fallback>(tile_x, tile_y1, sum_gate, MMQ_TILE_NE_K); | ||
| __syncthreads(); | ||
| } |
There was a problem hiding this comment.
Have you tried pipe-lining/warp-specialization (though this would make cross-IHV use worse)? How high is tensor-core utilization in this variant
There was a problem hiding this comment.
Tensor-pipe utilization (Nsight Compute, GB10): Q4_K is 37% fused versus 32% stock; Q8_0 and IQ4_XS are 28% versus 34%. Tensor cores do not appear to be the limiter. For Q4_K, the bottleneck is FP32 scaling around each MMA; for Q8_0 and IQ4_XS, it is weight loads and decoding.
Pipelining: The Y tile is copied with cp.async while the weights load. This gives about a 1% gain at pp16K. Running two blocks per SM hides most of the latency; one block per SM is 42–47% slower. Register-staged weight prefetch gave a 0.7% gain in an earlier version, but this version has no register budget left (255 registers).
Warp specialization: I tried splitting gate and up across two warp groups with a 128-token tile. It helps Q8_0 and IQ4_XS but hurts Q4_K and Q5_K, so it may be worth a per-type follow-up. I have not tried a producer/consumer design; that would also be NVIDIA-specific.
I’m refactoring how the decode phase handles this fusion. This will help avoid kernel fragmentation and extend the fusion support to all GGML types. |
…tream ggml-org#28702) Assisted-by: Claude Opus 5.5
…ill (upstream ggml-org#28702)" KLD vs master under -sm tensor: mean 0.0098, max 19.1 (master vs master: 0). Assisted-by: Claude Opus 5.5
…tream ggml-org#28702, re-apply) Verified on 2x RTX 5070/5070 Ti, -sm tensor, q8_0/q4_0 KV, wikitext-2 8x4096: - vs master: mean KLD 0.0098, same top p 97.46% - master -sm layer vs master -sm tensor: mean KLD 0.0079, same top p 97.39% - fused vs unfused per call: max abs err ~2.5e-5, no out-of-bounds writes pp4096 +2-3.5%, tg unchanged. Assisted-by: Claude Opus 5.5
Prefill-only fusion of the two dense Q4_K FFN projections and the following SwiGLU into one MMQ kernel. The activation is quantized to Q8_1 once and both gate and up tiles are computed against the same shared copy of it; up * silu(gate) is applied in the F32 write-back, so the separate gate matmul and the SwiGLU kernel disappear. The kernel uses the stock int8 MMA dot product and produces results identical to the unfused path. It runs with 128 threads and 64 rows per block (two blocks per SM) and a vectorized Q4_K tile loader with bank-conflict-free shared memory stores. Dispatched only for Q4_K weights, F32 activations, split SwiGLU and dense MUL_MAT where MMQ would have been selected, on devices with int8 MMA and cp.async. test-backend-ops gains dense Q4_K fusion cases and a whole-FFN case (MUL_MAT_Q_GATE_UP_SWIGLU_DOWN) that feeds the fused output into a quantized down projection.
Replace the Q4_K-only fused kernel with a has_fusion template flag on mul_mat_q, mirroring the mmvq decode fusion: both Y half tiles stay resident, a second accumulator holds the gate product, and the GLU is applied during write-back. Fused instances run with half the threads and rows at two blocks per SM (J <= 64), copy Y with cp.async where available (with a synchronous fallback otherwise), and are compiled for Q4_K, Q5_K, Q6_K, Q8_0, and IQ4_XS. SwiGLU, GeGLU, SwiGLU-OAI, and SwiGLU-clamp are handled by ggml_cuda_op_glu_single, which is now shared with the mmvq and mmvf fusions.
13ecfb0 to
27ac308
Compare
|
How much more effective is this vs fusing the gate-up weights? we can possibly extend some allocation primitive to ask for gate and up to contigiously allocated |
The helper only inspects a single tensor, so the src0 name is misleading.
The fused MMQ kernel reads the up and gate weights while it writes the GLU output. Under op offload the weights are copies in the CUDA compute buffer, which the allocator could reuse for the GLU output once the matmul nodes are done, corrupting the weights the kernel still reads (NaN logits with -ot ffn_gate=CPU at prompt batch sizes). Add alloc deps for both weights next to the existing one for the activation; for weights in weight buffers they have no effect.
The fused write-back applies SWIGLU_OAI with the default alpha and limit of ggml_cuda_op_swiglu_oai_single and ignores the values of the GLU node. Decline the fusion when they differ so those nodes take the unfused path, whose GLU kernel reads them. The in-tree users (gpt-oss, MiniMax-M3) use the defaults.
Restore the early-return layout of master instead of wrapping the stream-k part in an else: only its two mul_mat_q_process_tile calls change, and qk is declared at the top of the kernel again. The generated code is unchanged.
mmq.cuh now includes cp-async.cuh, which puts it into many translation units. Guard it against double inclusion like the other CUDA headers.
The MMQ-size fusion cases only ran the fused kernel with bounds-checked rows and small tiles, so the variant used for prompt processing (J = 48 and 64, row count a multiple of 128) was not tested. Add 48- and 128-token cases with 256 rows for each fused type.
Thanks, I prototyped it to answer this: the loader allocates Time per FFN layer for gate/up (+ GLU), ms:
With contiguous allocation alone, pp16K improves by only +0.6% for Q4_K, +1.2% for Q5_K, compared with +9.7%, +4.8% from this PR's fusion. |
@ORippler : Done with the refactoring, i am now following how this fusion is implemented for the decode. The async Y copy falls back to regular copies where CP_ASYNC_AVAILABLE is not defined (HIP, pre-Ampere). The fused variant uses the per-arch MMQ configs, so it is enabled for AMD |
So what's the real speed-up here? fusing the SWIGLU? |
The actual speedup is 0.6% for Q4_K and 1.2% for Q5_K. I haven’t fused SwiGLU yet; I’ll share the numbers after adding the fusion. |
|
We should also try interleaving them so that we can keep one accumulator. i.e. warp N calculates |
|
@am17an Agreed on keeping one accumulator. I looked at interleaved vs contiguous gate/up, and prototyped the one-accumulator fusion on contiguous weights. Interleaved vs contiguous For the fused kernel both should perform the same. A block reads the same bytes in the same pieces either way: for Q4_K, one 144-byte block per row per K step, with rows 2880 bytes apart. Let me know if you think otherwise Interleaving needs extra work outside the kernel:
Contiguous Prototype: contiguous + one accumulator
DGX Spark (GB10), Qwen3.8-27B. Gate/up (+ GLU) time per FFN layer, and end-to-end pp16K gain vs master:
Why each wins where it does The two kernels split the work differently:
Q4_K/Q5_K: Q4_K is limited by the FP32 scale/min math around each MMA, and Q5_K has the same scale structure. The second block per SM hides that latency: forcing this PR's kernel to one block per SM makes it 42-47% slower. Q8_0/IQ4_XS: these are limited by loading and decoding weights. Q8_0 has the largest weights (~1.06 bytes per weight) and IQ4_XS decodes through a lookup table. The 128-token tile loads and decodes each weight once per 128 tokens instead of once per 64: 16 times instead of 32 per 2048-token ubatch. Tensor-pipe utilization matches this. This PR vs master is 37% vs 32% for Q4_K, but 28% vs 34% for Q8_0 and IQ4_XS. So the one-accumulator design fits Q8_0/IQ4_XS better, and this PR's kernel fits Q4_K/Q5_K better. How would you like to proceed: keep this PR's kernel and add the one-accumulator path for Q8_0/IQ4_XS as a follow-up, or pick the variant per type in this PR? |
I think this can be avoided with the idea before that the interleaving is across 16 rows (i.e. the MMA tile width), that way 1 accumulator keeps the gate tile and 1 keeps the up tile. No shared memory interlude needed |
okey, so each warp covers 32 rows as two 16-row MMA tiles, so with gate rows in the first and up rows in the second, every thread holds g and the matching u at the same positions and can apply I'd do the interleave in the loader rather than in the weights:
Does this match what you had in mind? |
|
below are the perf numbers i am getting with the approach discussed in the #28702 (comment) DGX Spark (GB10), Qwen3.8-27B, pp16K end-to-end (-b/-ub 2048, fa on, f16 KV):
|
|
can you create a separate PR or branch with that approach? |
|
sure, I will share the changes soon |
Changes are in the branch: https://github.com/anujj/llama.cpp/tree/cuda-mmq-fused-gate-up-interleaved |
|
Please create a draft PR with that change after a self review |
matcher Upstream ggml-org#29633 added a warp_size parameter; the fused Q4_K gate/up SwiGLU matcher still used the old signature and failed to compile.
Overview
Extends the gate/up + GLU fusion, which today only covers small batches (MMVQ/MMVF), to prompt-processing batch
sizes on the MMQ path. For dense FFN layers where
ffn_gateandffn_uphave the same type and shape, the twomatmuls and the GLU now run as one MMQ kernel: the activations are quantized once, the GLU is applied in the
write-back, and the standalone GLU kernel and the second activation quantization disappear.
Enabled for Q4_K, Q5_K, Q6_K, Q8_0 and IQ4_XS on GPUs with the MMA data layout; other cases take the existing path, I have a plan to enable it other types also.
Implementation:
mul_mat_qgets a fused variant (has_fusion): every thread keeps separate up and gate accumulators, so theblock uses half the threads and rows of the stock tile (128 threads, 64 rows, 2 blocks/SM) and J is capped at 64.
Per K step both Y halves are loaded once (cp.async where available, plain copy otherwise), then the up and the gate
tiles are loaded and multiplied in turn. No stream-K: the GLU needs the full K reduction in one block. Column tiles
are the innermost grid dimension, so neighbouring blocks reuse the same weight tiles from L2.
has_fusionparameter; the fused kernel is onlycompiled for the MMA layout.
ggml_cuda_op_glu_single(unary.cuh) is now shared by the fused MMQ write-back, MMVQ and MMVF; the padding clearin
ggml_cuda_mul_mat_q/ MMVQ moved toggml_cuda_clear_padding.ggml_cuda_try_fuse;graph_optimizekeeps theactivation alive until the GLU node so the allocator cannot alias it with the fused output.
Additional information
Numbers for different Q4_K_M models
DGX Spark (GB10), CUDA 13.1, pp16384,
-b 2048 -ub 2048 -fa on, f16 KV, t/s, master and PR in one session:Numbers with supported GGML types in this PR
DGX Spark (GB10), CUDA 13.1, pp16384,
-b 2048 -ub 2048 -fa on, f16 KV, t/s:test-backend-ops: MUL_MAT_FUSION and the new MUL_MAT_FUSION_DOWN (gate/up + GLU + down projection) pass; thefull suite passes.
mul_mat_id), mixed gate/up types, have a plan to extend this fusion to the MOE also in follow up PR Only tested on NVIDIA; AMD MFMA/WMMA testing would be appreciated, I will also try I arrange the AMD also and test.Requirements