Skip to content
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

cpu: pooling: modify acl_pooling for stateless functions #2849

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zhili03
Copy link

@zhili03 zhili03 commented Mar 10, 2025

Description

Make pooling use stateless pooling op from ACL.

Checklist

General

  • Do all unit and benchdnn tests (make test and make test_benchdnn_*) pass locally for each commit?
  • Have you formatted the code using clang-format?

Performance improvements

  • Have you submitted performance data that demonstrates performance improvements?
./tests/benchdnn/benchdnn --pool --mode=P --dt=f16:f16 --tag=axb --alg=avg_np ic64ih56oh56kh3ph1

stateful:
OMP_NUM_THREADS=4
total perf: min(ms):0.301025 avg(ms):0.318544
OMP_NUM_THREADS=16
total perf: min(ms):0.121338 avg(ms):0.131665

stateless:
OMP_NUM_THREADS=4
total perf: min(ms):0.304443 avg(ms):0.320418
OMP_NUM_THREADS=16
total perf: min(ms):0.123047 avg(ms):0.133939

@zhili03 zhili03 requested a review from a team as a code owner March 10, 2025 17:39
@github-actions github-actions bot added the platform:cpu-aarch64 Codeowner: @oneapi-src/onednn-cpu-aarch64 label Mar 10, 2025
@Sqvid
Copy link
Contributor

Sqvid commented Mar 11, 2025

Thanks for the patch @zhili03. Could you please resolve the formatting failures?

Also nit: we usually prefix the aarch64 changes' commit messages with cpu: aarch64 rather than just cpu:. This makes it easier to grep for in the git logs as well.

@zhili03
Copy link
Author

zhili03 commented Mar 11, 2025

Hi @Sqvid , thanks for it, will fix the format issue and commit messages in my next commit

@Ryo-not-rio
Copy link
Contributor

Ryo-not-rio commented Mar 11, 2025

Hi Zhibo, could you add performance numbers and the corresponding benchdnn command? Thank you!

Also, we need to remove the change id from the PR message and commit message thanks!

@zhili03
Copy link
Author

zhili03 commented Mar 11, 2025

Hi Ryo, sure, will do in the next commit as well, thanks!

@zhili03 zhili03 requested a review from a team as a code owner March 13, 2025 07:59
@zhili03
Copy link
Author

zhili03 commented Mar 13, 2025

Hi @Sqvid @Ryo-not-rio some CI pipelines are failing because the stateless pooling function haven't been merge to ACL yet, does that mean this PR cannot be merged at least until next ACL's release? Thanks

@Sqvid Sqvid marked this pull request as draft March 13, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform:cpu-aarch64 Codeowner: @oneapi-src/onednn-cpu-aarch64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants