openvino icon indicating copy to clipboard operation
openvino copied to clipboard

Implement IsInf JIT Emitter for ARM64 SIMD in OpenVINO

Open inbasperu opened this issue 9 months ago • 4 comments

Hello maintainers,

I've implemented the IsInf JIT emitter for the ARM64 SIMD platform, as outlined in the OpenVINO CPU plugin JIT emitters documentation.

This PR addresses #24419.

Please let me know if any further adjustments are needed. Thank you!

inbasperu avatar May 11 '24 05:05 inbasperu

Thanks, good start, there are minor comments. Don't forget about the tests, please.

Thank you @eshoguli for the comments. I will make these changes. I had some questions about the tests. In the description of the issue here, it is mentioned that we should add tests for the supported operands in the respective element-wise or activation files. However, I am not sure how to proceed with the ComparisonTypes. I did look at a similar example, the equal JIT emitter, in this pull request, but couldn't find how to add tests. Could you please help me with this?

inbasperu avatar May 13 '24 16:05 inbasperu

Thanks, good start, there are minor comments. Don't forget about the tests, please.

Thank you @eshoguli for the comments. I will make these changes. I had some questions about the tests. In the description of the issue here, it is mentioned that we should add tests for the supported operands in the respective element-wise or activation files. However, I am not sure how to proceed with the ComparisonTypes. I did look at a similar example, the equal JIT emitter, in this pull request, but couldn't find how to add tests. Could you please help me with this?

Use this commit https://github.com/eshoguli/openvino/commit/1e27b762d225ec02d920e02692ef4255db29239d for Maximum & Minimum operations as example how to easilly add new operation to existing eltwise test. Let me know if you still have any questions.

eshoguli avatar May 15 '24 10:05 eshoguli

Thanks, good start, there are minor comments. Don't forget about the tests, please.

Thank you @eshoguli for the comments. I will make these changes. I had some questions about the tests. In the description of the issue here, it is mentioned that we should add tests for the supported operands in the respective element-wise or activation files. However, I am not sure how to proceed with the ComparisonTypes. I did look at a similar example, the equal JIT emitter, in this pull request, but couldn't find how to add tests. Could you please help me with this?

Use this commit eshoguli@1e27b76 for Maximum & Minimum operations as example how to easilly add new operation to existing eltwise test. Let me know if you still have any questions.

Hey @eshoguli, I have incorporated the code review feedback and added the IsInf operation to the existing element-wise tests. However, when I run the command ./bin/arm64/Release/ov_cpu_func_tests --gtest_filter="*smoke*Eltwise*", 76 tests failed. I'm not sure how to proceed further. Could you please guide me?

image

inbasperu avatar May 19 '24 08:05 inbasperu

Hey @eshoguli, I have incorporated the code review feedback and added the IsInf operation to the existing element-wise tests. However, when I run the command ./bin/arm64/Release/ov_cpu_func_tests --gtest_filter="*smoke*Eltwise*", 76 tests failed. I'm not sure how to proceed further. Could you please guide me?

The fix: https://github.com/inbasperu/openvino/commit/94e04bb6eeb818e9a7fb1fa15628b3e876ea79ea. I added the commit to your branch. Please, review the commit and rebase your branch to the latest master.

eshoguli avatar May 20 '24 18:05 eshoguli

build_jenkins

eshoguli avatar May 24 '24 09:05 eshoguli

build_jenkins

dmitry-gorokhov avatar Jun 10 '24 05:06 dmitry-gorokhov

@inbasperu PR is merged into master branch, Thanks for the contribution!

dmitry-gorokhov avatar Jun 10 '24 12:06 dmitry-gorokhov