-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Aarch64 jit depthwise convolution kernels #2014
Aarch64 jit depthwise convolution kernels #2014
Conversation
@vpirogov , Can you please share your feedback for the PR & Support us for merger |
@oneapi-src/onednn-cpu-aarch64, @snadampal, @kawakami-k, could you please help reviewing and validating this one? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great work, thank you!
My only important comment is that I think it is better than the current ACL depthwise implementation and it should go ahead of it in the dispatch list.
I have also done some testing with this on a C7g with benchdnn and with PyTorch, and I didn't hit any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code generally looks good, but can you remove the merge commit from the history by rebasing onto main please? https://github.com/oneapi-src/oneDNN/blob/main/CONTRIBUTING.md#code-contribution-guidelines
7740689
to
ad33f1c
Compare
@vpirogov kindly support to merge the changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe a missing closing brace should make that code break the build on ARM platforms.
8dd253d
to
9a4fcc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove the merge commit. Instead rebase your branch on top of the main branch.
e70a447
to
9a4fcc5
Compare
9a4fcc5
to
6f6a7ae
Compare
Hi @mgouicem, removed merge commits and rebased to latest main branch. |
Description
This commit expands ARM SVE support for forward and backward JIT SVE Depth-wise convolution in FP32 for, introducing compatibility with various vector lengths. The changes made are for implementing different ARM ISA.
Major code changes:
Checklist
General
make test
andmake test_benchdnn_*
) pass locally for each commit? YesTest output is same with and without this commit.