sycl: use DNN in the first part of ggml_sycl_mul_mat_batched_sycl #12972
Conversation
github-actions
bot
added
ggml
ggml/src/ggml-sycl/ggml-sycl.cpp
Outdated
| // TODO: use strided dnnl::memory::desc ctor in row_gemm to relax below assertions | ||
| GGML_ASSERT(nb11/nb10 == ne10); | ||
| GGML_ASSERT(nb01/nb00 == ne00); |
There was a problem hiding this comment.
Are you planning to remove these restrictions before making this PR ready?
Keep in mind oneDNN is enabled by default so we'll need to make sure that the paths using oneDNN for gemm_batch are as feature complete and performant as oneMKL. If this becomes an issue we may want to introduce another compiler option so that oneDNN batch gemm is disabled by default.
There was a problem hiding this comment.
This indeed looks like not handling some cases which were handled before. However it seems that all cases which need strided API fall into the second "if" branch. I see no test in backend-ops which hits these assertions (it passes). If I understand "ggml_is_contiguous_2" code correctly, this is due to "ggml_is_contiguous_2" condition.
Therefore, unless I will find a case which hits these assertions, I plan to remove TODO and leave them in the final code PR as a self-validating good documentation of this "if" details.
If you know the case which can hit this assertion please let me know - I will add to backend-ops test and handle it.
Update: I've just found that case :) This TODO will be implemented
|
This is the first change towards making SYCL graphs working by avoiding using calls which issue sycl::wait inside recording phase. Also using oneDNN should be preferred over using MKL in AI, as in this PR.