Skip to content

src: cpu: aarch64: Enable JIT fp16 -> fp16 reorder #3158

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

karmeh01
Copy link

Description

This PR adds support for fp16 JIT reorder for data type f16->f16.

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?
  • Have you submitted performance data that demonstrates performance improvements?

Performance improvements

Below are small test case logs that demonstrate performance numbers before and after.

./tests/benchdnn/benchdnn --reorder --sdt=f16 --ddt=f16 --stag=abc --dtag=aBc8b --mode=p 1x4096x4096

(new) jit:uni: total perf: min(ms):0.564941 avg(ms):1.01874
(old) simple:any: total perf: min(ms):25.6047 avg(ms):27.257

@karmeh01 karmeh01 requested a review from a team as a code owner April 24, 2025 15:01
@github-actions github-actions bot added the platform:cpu-aarch64 Codeowner: @oneapi-src/onednn-cpu-aarch64 label Apr 24, 2025
@karmeh01 karmeh01 changed the title src:cpu:aarch64: Enable JIT fp16 -> fp16 reorder src: cpu: aarch64: Enable JIT fp16 -> fp16 reorder Apr 25, 2025
@aditew01
Copy link
Contributor

Thanks for the PR. changes LGTM!
Are the relevant tests already present in benchdnn for this config: --sdt=f16 --ddt=f16 ?

@karmeh01
Copy link
Author

No they are not. I will add them and update the PR

@karmeh01 karmeh01 requested a review from a team as a code owner April 25, 2025 11:07
@github-actions github-actions bot added the component:tests Codeowner: @oneapi-src/onednn-arch label Apr 25, 2025
@karmeh01
Copy link
Author

karmeh01 commented Apr 25, 2025

82 tests added to check the --sdt=f16 --ddt=f16 case
Passes all tests
Command: ./tests/benchdnn/benchdnn --reorder --batch=tests/benchdnn/inputs/reorder/test_reorder_float16
Result: tests:566 passed:446 skipped:120 mistrusted:0 unimplemented:0 invalid_arguments:0 failed:0 listed:0

@@ -11,7 +11,7 @@
--dtag=gOIhw4i8o2i 4x16x16x3x3 2x16x6x3x2 2x2x10x2x3
--dtag=gOIhw4o8i2o 4x16x16x3x3 2x16x6x3x2 2x2x10x2x3

--sdt=f16 --ddt=f32,bf16,s8,u8,f8_e5m2,f8_e4m3
--sdt=f16 --ddt=f16,f32,bf16,s8,u8,f8_e5m2,f8_e4m3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is redundant, isn't it?

Copy link
Author

@karmeh01 karmeh01 Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think you're right.

@@ -1,6 +1,6 @@
# f32, s8, u8 <--> f16
--reset
--sdt=f32,bf16,s8,u8,f8_e5m2,f8_e4m3 --ddt=f16
--sdt=f16,f32,bf16,s8,u8,f8_e5m2,f8_e4m3 --ddt=f16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! this should be enough.

@karmeh01 karmeh01 requested a review from aditew01 April 25, 2025 13:11
@karmeh01 karmeh01 requested a review from aditew01 April 25, 2025 13:32
Copy link
Contributor

@Ryo-not-rio Ryo-not-rio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:tests Codeowner: @oneapi-src/onednn-arch platform:cpu-aarch64 Codeowner: @oneapi-src/onednn-cpu-aarch64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants