Repository navigation
Conversation
github-actions
Bot
added
ggml
arthw
left a comment
arthw
left a comment
There was a problem hiding this comment.
It's good job!
I tested on Arc770, all related UT cases are passed.
Thank you!
arthw
added
the
merge ready
Titaniumtown
removed
the
merge ready
|
Seems github actions isn't building this? Lets make sure it passes CI before we merge. Idk how to trigger it though. |
I think the project may limit or restrict the CI builds now. I thought it was requiring at least 1 approval, but that may have been changed. A had a report of one CI build was taking 6 hours. (Ref: https://github.com/ggml-org/llama.cpp/actions/runs/38340871230/job/117015127203?pr=30271) Oh, here are some others:
A trivial patch/change should trigger it, I'm not sure if there is a cleaner way. You might be able to run the Action(s) from the Actions tab, I don't know. |
I'm not sure, but it seems to have lose the merge-ready tag, and doesn't have you listed as a reviewer. Maybe when you changed the PR to draft and back to ready to review. I'm not sure. Given it only touches I think 2 functions, both specifically about SWIGLU_OAI, in a single file, the risk of adverse effects is low. The worst is I make an infrequently used op less functional. |
|
@mctylr-gh Thank you! |
The modified (or fused) GLU activation function, SWIGLU_OAI op support already declares f32/f16 as supported. The kernel only instantiated f32 and asserted on f16. This implements f16 support for SWIGLU_OAI Resolves Issue ggml-org#30855 Assisted-by: pi (Greatjedi/Qwen3.8-27B-Intel-Arc-Tuned-GGUF:Q4_K via llama.cpp)
de2b460 to
7d30e4d
Compare
|
I've rebase this pull request. The CI tasks are queued. Please let me know if there is anything else I can do. Thank you. |
* sycl: work-group argmax for short TOP_K rows Used angelX to optimize SYCL/drama 3 coordination, results are as follows Rows with ncols <= max work-group size: one work-item per column, k rounds of work-group argmax. Ties take the lower column, NaN is never picked. Longer rows keep the existing paths. Arc Pro B70, oneAPI 2026.1, test-backend-ops perf (existing short-row cases), base 9b56ccf vs patch, median ratio of 6 rotating A/B/A rounds: | ncols | nrows | k | base us | patch us | speedup | |---|---|---|---|---|---| | 16 | 1024 | 1 | 19.97 | 2.88 | 6.95x | | 16 | 1024 | 2 | 23.83 | 2.90 | 8.25x | | 16 | 1024 | 8 | 66.12 | 4.66 | 14.21x | | 16 | 8192 | 1 | 110.42 | 5.96 | 18.51x | | 16 | 8192 | 2 | 130.21 | 8.82 | 14.77x | | 16 | 8192 | 8 | 420.62 | 27.27 | 15.42x | | 128 | 1024 | 1 | 20.49 | 2.94 | 6.97x | | 128 | 1024 | 2 | 25.89 | 3.45 | 7.52x | | 128 | 1024 | 8 | 101.67 | 8.95 | 11.36x | | 128 | 8192 | 1 | 112.19 | 13.29 | 8.44x | | 128 | 8192 | 2 | 141.18 | 20.60 | 6.85x | | 128 | 8192 | 8 | 643.21 | 67.60 | 9.51x | | 1024 | 1024 | 1 | 17.32 | 14.98 | 1.16x | | 1024 | 1024 | 2 | 28.77 | 24.02 | 1.20x | | 1024 | 1024 | 8 | 151.87 | 77.88 | 1.95x | | 1024 | 8192 | 1 | 156.49 | 138.78 | 1.13x | | 1024 | 8192 | 2 | 201.47 | 209.18 | 0.96x | | 1024 | 8192 | 8 | 1006.25 | 632.60 | 1.59x | The 1024 x 8192, k=2 case is 3.7% slower. Correctness: test-backend-ops -o TOP_K 533/533 and -o ARGSORT 98/98 on the patch, again on the rebase onto 3f03b17. Full test-backend-ops aborts on base and patch alike at the non-F32 SWIGLU_OAI assert (element_wise.cpp:1187, see #30857). With only those cases excluded: 20145/20173 on base and patch. One CPY f16->q2_0 [256,2,3,4] failure was seen once on the patch, and 3/3 re-runs pass on base and patch. All other patch-run failures also fail on base. Assisted-by: Claude Opus 5.5 * made changes as suggested Assisted-by: Claude Opus 5.5
Used angelX to optimize SYCL/drama 3 coordination, results are as follows Each output reads only inputs with i*s0 <= idx < i*s0 + K, in the same ascending order as before. Arc Pro B70, oneAPI 2026.1, test-backend-ops perf, base 9b56ccf vs patch, median of 3 same-boot A/B/A rounds, A/A 1.000-1.001: | input [ne0, ne1] | kernel [K, OC, IC] | s0 | base us | patch us | speedup | |---|---|---|---|---|---| | [16, 1536] | [16, 768, 1536] | 8 | 3910.57 | 1348.59 | 2.90x | | [128, 768] | [16, 384, 768] | 8 | 37735.00 | 1665.26 | 22.66x | | [1024, 384] | [8, 192, 384] | 4 | 270371.75 | 1471.74 | 183.71x | | [4096, 192] | [4, 96, 192] | 2 | 534614.63 | 752.74 | 710.23x | Shapes are the drama 3 codec decoder upsampling stack, added as perf cases. Chained 4-layer stack at 1/4/24 input frames: base 66.3/847.4/28638.5 ms, patch 2.3/6.7/30.7 ms. Kernel numbers, not end-to-end TTS latency. Correctness: test-backend-ops -o CONV_TRANSPOSE_1D 116/116 on base and patch, and 116/116 rebased on 3f03b17. Seeded output dump (7.46 MB) byte-identical to base. Full test-backend-ops aborts on base and patch alike at the non-F32 SWIGLU_OAI assert (element_wise.cpp:1187, see ggml-org#30857). With only those cases excluded: 20145/20173 on base and patch, and every patch-run failure also fails on base.
Overview
The modified (or fused) GLU activation function, SWIGLU_OAI op support already declares F32/F16 as supported. The kernel only instantiated F32 and asserted on F16. This implements F16 support for SWIGLU_OAI op.
Assisted-by: pi (Greatjedi/Qwen3.8-27B-Intel-Arc-Tuned-GGUF:Q4_K via llama.cpp)
Additional information
The design follows the existing
ggml_sycl_op_swiglu_clamppattern, and PR #23882 for the SwiGLU_OAI implementation on metal.Kernel template unchanged (already generic over T).
SWIGLU_OAI is a modified SwiGLU variant activation function, used in gpt-oss and MiniMax M3. Previously this the op aborted on the F32 assert even though the backend op-support check declared F16 supported. Reference issue #17643 (gpt-oss offload) and PR #17826 (original SYCL support, F32 only).
Testing:
test-backend-ops -b SYCL0 -o SWIGLU_OAIon an Intel Arc ProB70, 16/16 cases passPost change output of
test-backend-ops -b SYCL0 -o SWIGLU_OAI:test-backend-ops-SWIGLU_OAI-2026-10-19.txt
Resolves (my own issue) Issue: #30855
Requirements