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.

Sign up for GitHub

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

Jump to bottom

[CPU][ARM64] Add JIT emitter for Eltwise Less operation #27144

Closed
wants to merge 2 commits into from
Closed

[CPU][ARM64] Add JIT emitter for Eltwise Less operation #27144

wants to merge 2 commits into from
+84 −0

Conversation

nashez
Copy link
Contributor

@nashez nashez commented Oct 19, 2024

Details:

  • Added a jit_less_emitter derived class in aarch64/jit_eltwise_emitters
  • Created entry Algorithm::EltwiseLess in the get_supported_precisions in nodes/kernels/aarch64
  • Add the EltwiseLess entry in the aarch64 executors supported algorithms

Tickets:

@nashez nashez requested review from a team as code owners October 19, 2024 04:12
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Oct 19, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Oct 19, 2024
@nashez
Copy link
Contributor Author

nashez commented Oct 19, 2024

@dmitry-gorokhov Please review the changes.

@nashez nashez force-pushed the nashez/jit_less_emitter branch from 8b380e0 to e801da7 Compare October 19, 2024 04:25
@dmitry-gorokhov dmitry-gorokhov added this to the 2024.5 milestone Oct 21, 2024
@dmitry-gorokhov dmitry-gorokhov self-assigned this Oct 21, 2024
@dmitry-gorokhov
Copy link
Contributor

build_jenkins

@nashez nashez force-pushed the nashez/jit_less_emitter branch from e801da7 to 506e82b Compare October 21, 2024 19:01
@dmitry-gorokhov
Copy link
Contributor

build_jenkins

@nashez
Copy link
Contributor Author

nashez commented Oct 23, 2024

@dmitry-gorokhov Does this need something from my end?
I see CI failing

@nashez nashez force-pushed the nashez/jit_less_emitter branch from 506e82b to 4ac0454 Compare October 23, 2024 06:20
@dmitry-gorokhov
Copy link
Contributor

@dmitry-gorokhov Does this need something from my end? I see CI failing

@nashez No. this is common issue. We are waiting #27171 to be merged

@dmitry-gorokhov
Copy link
Contributor

@nashez Seems like Greater PR works well - I added it in the merge queue. However this one has issues on TF tests: https://github.com/openvinotoolkit/openvino/actions/runs/11474224180/job/31987687598?pr=27144.
Could you please take a look?

@nashez nashez force-pushed the nashez/jit_less_emitter branch from 4ac0454 to 8590e10 Compare October 27, 2024 07:48
@dmitry-gorokhov
Copy link
Contributor

build_jenkins

@dmitry-gorokhov
Copy link
Contributor

@nashez The CI is still failing with the same issue: https://github.com/openvinotoolkit/openvino/actions/runs/11538732059/job/32139671900?pr=27144

@ilya-lavrenov ilya-lavrenov added the platform: arm OpenVINO on ARM / ARM64 label Oct 31, 2024
@nashez
Copy link
Contributor Author

nashez commented Oct 31, 2024

Ok @dmitry-gorokhov I might find some time this weekend to check out the tests. Will update you then.
But in general, the implementation looks correct right? Because the smoke tests have been passing.
This is probably some other issue being uncovered upon enabling this emitter.

@dmitry-gorokhov
Copy link
Contributor

dmitry-gorokhov commented Oct 31, 2024 •
edited
Loading

Ok @dmitry-gorokhov I might find some time this weekend to check out the tests. Will update you then. But in general, the implementation looks correct right? Because the smoke tests have been passing. This is probably some other issue being uncovered upon enabling this emitter.

Thanks @nashez!
I suspect there might be some corner cases which might not be covered by Smoke tests. So would be greate to check failing scope.

@mlukasze
Copy link
Contributor

mlukasze commented Nov 6, 2024

build_jenkins

@mlukasze mlukasze enabled auto-merge November 6, 2024 07:09
@nashez
[CPU][ARM64] Add JIT emitter for Eltwise Less operation
073d0ed 
* Added a jit_less_emitter derived class in
  aarch64/jit_eltwise_emitters
* Created entry Algorithm::EltwiseLess in the
  get_supported_precisions in nodes/kernels/aarch64
* Add the EltwiseLess entry in the aarch64
  executors supported algorithms

Closes: openvinotoolkit#24415

Signed-off-by: Nashez Zubair <[email protected]>
auto-merge was automatically disabled November 21, 2024 09:54

Head branch was pushed to by a user without write access

@nashez nashez force-pushed the nashez/jit_less_emitter branch from b73067e to 073d0ed Compare November 21, 2024 09:54
@dmitry-gorokhov
Copy link
Contributor

build_jenkins

@nashez
Copy link
Contributor Author

nashez commented Nov 24, 2024 •
edited
Loading

Unfortunately I am unable to reproduce the failure because the Tensor flow tests are not building for me.
I will try to arrange a different machine and try to reproduce the tests.
@dmitry-gorokhov I might need some help in trying to reproduce tests

@nashez nashez mentioned this pull request Nov 24, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label Dec 10, 2024
@mlukasze
Copy link
Contributor

build_jenkins

@mlukasze
Copy link
Contributor

Unfortunately I am unable to reproduce the failure because the Tensor flow tests are not building for me. I will try to arrange a different machine and try to reproduce the tests. @dmitry-gorokhov I might need some help in trying to reproduce tests

let me update a branch and run Jenkins again.
sometimes such problem may be sporadic issue on our side, we will know soon.

@github-actions github-actions bot removed the Stale label Dec 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

This PR will be closed in a week because of 2 weeks of no activity.

@github-actions github-actions bot added the Stale label Dec 26, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 3, 2025

This PR was closed because it has been stalled for 2 week with no activity.

@github-actions github-actions bot closed this Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitry-gorokhov dmitry-gorokhov dmitry-gorokhov approved these changes

Assignees

@dmitry-gorokhov dmitry-gorokhov

Labels
category: CPU OpenVINO CPU plugin ExternalPR External contributor platform: arm OpenVINO on ARM / ARM64 Stale
Projects
None yet
Milestone
2024.5
Development

Successfully merging this pull request may close these issues.

[Good First Issue] [ARM]: Implement CPU plugin just-in-time emitter for Less operation
5 participants
@nashez @dmitry-gorokhov @mlukasze @ilya-lavrenov @sys-openvino-ci