Repository navigation
cpu: accept BF16 in src1 of mul_mat - #28937
Merged
ServeurpersoCom merged 2 commits intoSep 30, 2026
Merged
Conversation
github-actions
Bot
added
testing
Contributor
Author
|
The BF16 kernel case of the conv_1d_dw test exercises the f32 x bf16 mat vec path on Metal, which only exists in #28741; the gpu-metal job goes green once that one is merged and this branch is rebased. |
ggerganov
approved these changes
Sep 15, 2026
Member
|
Thanks for working on this follow-up and adding a test. :) |
Contributor
|
Is this used for something, so would it be worth adding support to Vulkan? If so I can do that. |
Contributor
Author
Yes, I hit it on an unreleased LLM with an audio input tower: its depthwise conv kernels are BF16, so ggml_conv_1d_dw lands on f32 x bf16. Anything audio ML is something I end up using sooner or later, so I'd love for GGML to have no holes in this op matrix, it's more future proof. A Vulkan path would be great if you're up for it! I also have Vulkan on hand (windows/linux), so if you prefer I can write it myself, keeping it strictly symmetric with the existing BF16 paths, and validate it on the real model with cosine similarity against the CPU reference, leaving you just the review. Whichever you prefer! |
Contributor
|
I'll look into it. |
Contributor
Author
|
That's actually better, and I would just run the Vulkan correctness on the model |
ServeurpersoCom
force-pushed
the
cpu-mul-mat-bf16-src1
branch
from
September 23, 2026 12:46
d05aadd to
fa5e7cd
Compare
Contributor
Author
|
Rebase only |
ServeurpersoCom
force-pushed
the
cpu-mul-mat-bf16-src1
branch
2 times, most recently
from
September 27, 2026 17:33
03e40df to
e053886
Compare
Contributor
Author
|
Rebase after the merge of #29365, which fixes the Vulkan misalignment assert that broke the previous CI run. Tested locally on an RTX PRO 6000:
|
ggml_conv_1d_dw builds its im2col in F32 when the kernel is BF16, then calls ggml_mul_mat(im2col, kernel), which puts F32 in src0 and BF16 in src1. The CPU backend refused that combination, so it was reported as unsupported on every backend and never compared against anything. Widen BF16 into the F32 work buffer, next to the existing packing of F32 into vec_dot_type. This is the arithmetic the Metal mat vec kernel already uses, both operands promoted to float and accumulated in float, so the two agree exactly rather than approximately. Cover it with a conv_1d_dw test over F32, F16 and BF16 kernels, plus three mul_mat cases with BF16 in src1.
supports_op only checked the src1 type for non contiguous tensors, so a contiguous BF16 src1 was accepted and the pipeline lookup asserted. The only BF16 src1 path is the BF16 x BF16 multiply, every other src0 type now reports the op as unsupported and the scheduler keeps it on the CPU. The BF16 kernel case of the conv_1d_dw test needs the f32 x bf16 mat vec variants of the Metal backend, which land separately.
ServeurpersoCom
force-pushed
the
cpu-mul-mat-bf16-src1
branch
from
September 30, 2026 06:55
e053886 to
743bd31
Compare
0cc4m
approved these changes
Sep 30, 2026
Contributor
Author
|
The red jobs are unrelated: Metal fusion is the missing glm5-next baseline fixed by #29712, WebGPU is red on master, and the Vulkan Intel and OpenVINO failures are tolerance misses on SET_ROWS q2_0 and ADD_ADD f16, with no MUL_MAT failure anywhere. Merging |
pierreguillot
pushed a commit
to Ircam-Partiels/llama.cpp
that referenced
this pull request
Oct 1, 2026
* cpu: accept BF16 in src1 of mul_mat ggml_conv_1d_dw builds its im2col in F32 when the kernel is BF16, then calls ggml_mul_mat(im2col, kernel), which puts F32 in src0 and BF16 in src1. The CPU backend refused that combination, so it was reported as unsupported on every backend and never compared against anything. Widen BF16 into the F32 work buffer, next to the existing packing of F32 into vec_dot_type. This is the arithmetic the Metal mat vec kernel already uses, both operands promoted to float and accumulated in float, so the two agree exactly rather than approximately. Cover it with a conv_1d_dw test over F32, F16 and BF16 kernels, plus three mul_mat cases with BF16 in src1. * vulkan: reject BF16 in src1 of mul_mat unless src0 is BF16 supports_op only checked the src1 type for non contiguous tensors, so a contiguous BF16 src1 was accepted and the pipeline lookup asserted. The only BF16 src1 path is the BF16 x BF16 multiply, every other src0 type now reports the op as unsupported and the scheduler keeps it on the CPU. The BF16 kernel case of the conv_1d_dw test needs the f32 x bf16 mat vec variants of the Metal backend, which land separately.
1 task
frostyautumnleaf
pushed a commit
to frostyautumnleaf/llama.cpp
that referenced
this pull request
Oct 5, 2026
* cpu: accept BF16 in src1 of mul_mat ggml_conv_1d_dw builds its im2col in F32 when the kernel is BF16, then calls ggml_mul_mat(im2col, kernel), which puts F32 in src0 and BF16 in src1. The CPU backend refused that combination, so it was reported as unsupported on every backend and never compared against anything. Widen BF16 into the F32 work buffer, next to the existing packing of F32 into vec_dot_type. This is the arithmetic the Metal mat vec kernel already uses, both operands promoted to float and accumulated in float, so the two agree exactly rather than approximately. Cover it with a conv_1d_dw test over F32, F16 and BF16 kernels, plus three mul_mat cases with BF16 in src1. * vulkan: reject BF16 in src1 of mul_mat unless src0 is BF16 supports_op only checked the src1 type for non contiguous tensors, so a contiguous BF16 src1 was accepted and the pipeline lookup asserted. The only BF16 src1 path is the BF16 x BF16 multiply, every other src0 type now reports the op as unsupported and the scheduler keeps it on the CPU. The BF16 kernel case of the conv_1d_dw test needs the f32 x bf16 mat vec variants of the Metal backend, which land separately.
edwardyoon
pushed a commit
to edwardyoon/focus-llama
that referenced
this pull request
Oct 7, 2026
* cpu: accept BF16 in src1 of mul_mat ggml_conv_1d_dw builds its im2col in F32 when the kernel is BF16, then calls ggml_mul_mat(im2col, kernel), which puts F32 in src0 and BF16 in src1. The CPU backend refused that combination, so it was reported as unsupported on every backend and never compared against anything. Widen BF16 into the F32 work buffer, next to the existing packing of F32 into vec_dot_type. This is the arithmetic the Metal mat vec kernel already uses, both operands promoted to float and accumulated in float, so the two agree exactly rather than approximately. Cover it with a conv_1d_dw test over F32, F16 and BF16 kernels, plus three mul_mat cases with BF16 in src1. * vulkan: reject BF16 in src1 of mul_mat unless src0 is BF16 supports_op only checked the src1 type for non contiguous tensors, so a contiguous BF16 src1 was accepted and the pipeline lookup asserted. The only BF16 src1 path is the BF16 x BF16 multiply, every other src0 type now reports the op as unsupported and the scheduler keeps it on the CPU. The BF16 kernel case of the conv_1d_dw test needs the f32 x bf16 mat vec variants of the Metal backend, which land separately. (cherry picked from commit 90c908d)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
ggml_mul_mat already handles BF16 x F32 and BF16 x BF16, but not the mirror case F32 x BF16, which is exactly what ggml_conv_1d_dw produces with a BF16 kernel. This fills that cell on the CPU, with the tests that were missing.
Additional information
CPU reference for #28741: test-backend-ops only compares a case that both the tested backend and the CPU declare supported, so this cell was reported as not supported everywhere and the Metal kernels had nothing to be checked against. CUDA already runs it and #28741 adds it to Metal, but the reference backend does not.
BF16 in ggml_mul_mat on the CPU:
F32 x BF16 per backend:
Requirements