sycl: contiguous fast path + 32-bit index math for unary elementwise ops - #25946
Conversation
arthw
left a comment
There was a problem hiding this comment.
It's good job!
With Qwen3.6-27B-UD-Q4_K_XL.gguf on B60:
| Test | fa | Base t/s | Primary t/s | Increase Rate (Primary vs Base) |
|---|---|---|---|---|
| pp512 | 0 | 186.00 | 189.34 | 1.80% |
| pp512 | 1 | 184.17 | 187.44 | 1.78% |
| tg128 | 0 | 16.91 | 16.99 | 0.47% |
| tg128 | 1 | 16.90 | 17.00 | 0.59% |
Thank you!
|
@Titaniumtown |
|
Ok, no problem! |
|
Have you considered using common.hpp:fast_div_modulo()? Could be another half-percent for you to grab if my test of your approach applied to gated_op_fused_swiglu() is good metric.
¹ mode1 pp512 sd inflated by one 848 t/s rep in block 2 (DVFS blip); its other 7 samples ≈ 876. |
|
@mndodd Cool find! I don't have the time in the next few days to do this, but I will definitely check this out when I can. |
|
@Titaniumtown Thank you! |
7e614b9 to
bc72017
Compare
|
@mndodd Thanks for the suggestion, I added this! |
|
Ready for review! |
…ops (ggml-org#25946) * sycl: contiguous fast path + 32-bit index math for unary elementwise ops * sycl: use fastdiv for elementwise index math
…ops (ggml-org#25946) * sycl: contiguous fast path + 32-bit index math for unary elementwise ops * sycl: use fastdiv for elementwise index math
…gml-org#25946) Stacks on ggml-org#25946 (ca7c42a): that added the contiguous fast-path (the +5.9% win on this deploy, silu is 100% contiguous) plus a 32-bit narrowing of the strided fallback. But findings/210+212 show Xe has NO hardware integer divide at ANY width -- the 32-bit `t % une` still emits an emulated divide (precompiled_u32divrem_sp). Replace it with fastdiv (init_fastdiv_values host-side + fast_div_modulo in-kernel: mul_hi + shift, no divide of any width), which the SYCL backend already ships in common.hpp and uses in col2im / gated_delta_net / fattn. Measured on the strided path: fastdiv +1.75% vs the 32-bit narrowing's +1.3% (4-way silu A/B, findings/210 UNARY ANCHOR). Inert on this deploy (strided unary = 0 calls), so this is CUDA-discipline parity + emulated-op hygiene for models that DO hit strided unary; the contiguous win is ggml-org#25946's fast-path. findings/210, findings/212. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
43 upstream commits, 15 of them in our paths. Four conflicts, resolved as follows. The dangerous change in this range did NOT conflict -- see (2). 1. ggml/src/ggml-sycl/element_wise.cpp -- TOOK UPSTREAM VERBATIM. ggml-org#25946 landed upstream as 11b068d. We had been carrying it as a cherry-pick of the then-unmerged PR (ca7c42a) plus two commits of our own stacked on top: 27d821e fastdiv for the strided unary index reconstruction 0595878 fastdiv for the fused-GLU index reconstruction Upstream's landed form contains BOTH optimisations by the same mechanism (init_fastdiv_values host-side + fast_div_modulo in-kernel, on the strided unary path and on all five gated_op_fused_* kernels). All three of ours are therefore superseded and are dropped; the file is now byte-identical to origin/master. Only semantic difference we give up: ours guarded k > u32 with an exact int64 fallback, upstream asserts ggml_nelements(dst) < 2^31 instead -- stricter by 2x, and unreachable for a GLU activation (~8 GB at f32). Our ne>0?ne:1 divisor guard is also dropped; init_fastdiv_values asserts d != 0 and a ggml tensor always has ne[i] >= 1, so it was defensive, not load-bearing. 2. ggml/src/ggml-sycl/fattn.cpp -- PRECEDENCE PRESERVED, both kernels kept. Upstream ggml-org#25025 adds a oneMKL GEMM flash-attention path and gives it BEST_FATTN_KERNEL_MKL = 300 -- the value we already use for BEST_FATTN_KERNEL_MMA. git flagged the enum collision. It did NOT flag the consequential half: upstream places the MKL gate ABOVE our MMA check, and that hunk auto-merged clean. MKL's gate is default-ON (GGML_SYCL_ENABLE_MKL_FA=1) and its envelope -- gqa_ratio >= 2, head_dim % 64 in [64,512], Q->ne[1] >= 32, K->ne[1] >= 1024, no sinks / ALiBi / softcap, with a quantized K/V SKIPPING the F16 stride test -- matches our deploy prefill exactly. Taken verbatim it would have silently replaced the measured MMA kernel with an unmeasured one, staged the whole q8_0 KV cache to F16 first, and (per upstream's own note) broken SYCL graph capture replay. Resolution: MKL renumbered to 400 so both kernels stay reachable, and its gate takes an added !ggml_sycl_fattn_mma_supported(dst) conjunct. MMA wins where MMA is supported; MKL keeps its FULL envelope for everything MMA declines, which is upstream's intent in every case that is not ours. This is a precedence choice, not a revert -- and it is A/B-able without a rebuild: GGML_SYCL_FATTN_MMA=0 -> MMA declines, MKL takes the path GGML_SYCL_ENABLE_MKL_FA=0 -> MKL off entirely Also merged both sides' env-gated instruments, hoisting the kernel selection to a single call: upstream re-derived it three times (watchdog, switch, fingerprint), so an instrument could disagree with what actually ran. All three now read one hoisted `k`. Fixed a latent lie in our own FATTN_DEBUG printer while there -- BEST_FATTN_KERNEL_ONEDNN was printing as "NONE"; ONEDNN and MKL now print. 3. ggml/src/ggml-sycl/cpy.cpp -- kept ours. Upstream's side of the hunk was empty; ggml-org#26005 touched adjacent lines. Our GGML_SYCL_CPY_CENSUS instrument is unchanged. 4. tests/test-backend-ops.cpp -- kept both sides, additive and disjoint (same resolution as the 07-28 sync). Ours = the finding-85 deployed-shape MUL_MAT sweep; upstream's = m==1 either side of MMVF_MAX_BATCH_SIZE. Gates run before this commit: - cmake-option-audit.sh e9fa078 origin/master -> 7/7 watched options unchanged, and --selftest fires (rc=3), so the check is proven able to go red. - post-configure asserts: GGML_SYCL / _F16 / _DNN / _GRAPH all ON. NOT done in this commit, and required before any number from this tree is comparable to a pre-sync one: rebuild + re-baseline. Upstream changed code under every arm; ratios within one arm survive, absolutes do not.
Overview
Every op routed through ggml_sycl_op_unary (silu, sigmoid, gelu*, exp, softplus, ETC) ran a generic strided kernel doing three int64 div/mod per element to reconstruct 4-D indices.
Intel's Xe cores has no native 64-bit integer divide op. So when compiling the kernels, ICPX adds its own implementation. The extra index math then dominates the kernel's compute time: SILU measured 3.2 ms/call on {2048,17408} f32 activations vs a ~0.63 ms bandwidth floor. This path is called 96x per qwen 3.6 ubatch, which results in 13% of profiled op time during prefill.
But, when src0 and dst are both contiguous, we can skip the index reconstruction as the reconstruction of indices is wasteful compute-wise when it can be easily calculated.
So what I ended up doing is adding a fast path that allows the skipping of those very very performance heavy division and 4-D index reconstruction operations.
Additional information
Benchmarks on an Arc B70 Pro
llama-bench -m Qwen3.6-27B-UD-Q4_K_XL.gguf -ngl 999 -fa 1-ctk q8_0 -ctv q8_0 -p 512,2048 -n 128 -r 4
master @ 571d0d5:
This PR:
TL;DR 4.3-3.9% uplift on pp bandwidth with no effect on decode.
Requirements