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

[ARM CPU] Fix eltwise op tests (Divide) #17029

Merged
merged 14 commits into from
Apr 19, 2023

Conversation

allnes
Copy link
Contributor

@allnes allnes commented Apr 18, 2023

No description provided.

@allnes allnes requested review from a team as code owners April 18, 2023 15:50
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Apr 18, 2023
@allnes allnes added the platform: arm OpenVINO on ARM / ARM64 label Apr 18, 2023
@ilya-lavrenov ilya-lavrenov added this to the 2023.0 milestone Apr 18, 2023
if (!divide) {
return false;
}
if (divide->get_element_type() != ov::element::i32) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to use divide->get_element_type().is_integral_number() instead. We are going to merge I64 native support, so it would be nice to make this transofmration suitable for upcoming change.

namespace ov {
namespace intel_cpu {

ConvertI32Div::ConvertI32Div() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe: DecomposeIntegerDivide?
File name should also be changed

@@ -262,6 +263,7 @@ void Transformations::PreLpt(const std::vector<ov::element::Type>& defaultPrecis
CPU_REGISTER_PASS_ARM(manager, ConvertConv1D);
CPU_REGISTER_PASS_ARM(manager, ConvertGroupConv1D);
CPU_REGISTER_PASS_ARM(manager, ConvertGroupConvolution);
CPU_REGISTER_PASS_ARM(manager, ConvertI32Div);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets put the explanation here: The plugin computes Divide in floating point precision. To preserve correct math for integer division we need to insert explicit Floor operation.

@allnes
Copy link
Contributor Author

allnes commented Apr 19, 2023

@dmitry-gorokhov done

@ilya-lavrenov ilya-lavrenov enabled auto-merge (squash) April 19, 2023 13:48
@ilya-lavrenov ilya-lavrenov disabled auto-merge April 19, 2023 14:52
@ilya-lavrenov ilya-lavrenov merged commit 3d33cb2 into openvinotoolkit:master Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin platform: arm OpenVINO on ARM / ARM64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants