Repository navigation
sycl: pick the src1 fp16 conversion from the runtime oneDNN switch - #288
Merged
Merged
Conversation
ggml_sycl_mul_mat_batched_sycl chose how to convert src1 to fp16 at compile time (#if GGML_SYCL_DNNL), but chose the GEMM at runtime (g_ggml_sycl_enable_dnn). In a oneDNN-enabled build run with GGML_SYCL_ENABLE_DNN=0, src1 was converted into the strided oneDNN layout, and s11/s12/s13 were then reset to contiguous strides for the MKL gemm_batch path. The fallback read a strided buffer as contiguous and gave wrong results whenever src1 was not contiguous. Gate the oneDNN conversion on the runtime switch and use the contiguous _nc conversion otherwise. test-backend-ops -o MUL_MAT -b SYCL0 on Arc B390 with GGML_SYCL_ENABLE_DNN=0: 1297/1329 -> 1329/1329. The 32 cases that failed were f16 x f32 with non-contiguous views (k_v != 0). The default oneDNN path is unchanged at 1329/1329.
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
1 participant
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.
What
Fixes wrong
MUL_MATresults on SYCL when oneDNN is compiled in but turned off at runtime withGGML_SYCL_ENABLE_DNN=0.Cause
ggml_sycl_mul_mat_batched_syclmakes two choices at different times:#if GGML_SYCL_DNNL).g_ggml_sycl_enable_dnn).In a oneDNN build run with
GGML_SYCL_ENABLE_DNN=0:s11/s12/s13are then reset to contiguous strides.gemm_batchfallback reads that strided buffer as if it were contiguous.The result is wrong whenever src1 isn't contiguous.
Fix
The oneDNN conversion now runs only when the runtime switch is on. Otherwise the existing
_ncconversion runs. The change is 5 lines in one function. The default oneDNN path takes exactly the same code as before.Testing
test-backend-ops test -o MUL_MAT -b SYCL0on an Intel Arc B390 (Panther Lake). Setup: Windows 11, oneAPI 2025.3 with oneDNN 2025.1,GGML_SYCL_F16=OFF, baseadfffbe41.GGML_SYCL_ENABLE_DNN=0All 32 cases that failed were
type_a=f16,type_b=f32,n=1with non-contiguous views (k_v=2112/2113), with ERR around 1.1–2.0. They failed identically with and without #278.The same code is in ggml-org/llama.cpp master, so the fix should also go upstream.