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] Add ACL executor for Convert #17323

Merged
merged 25 commits into from
Jul 13, 2023

Conversation

allnes
Copy link
Contributor

@allnes allnes commented May 2, 2023

  • Separate executors
  • Add ACL executor for convert

@allnes allnes requested review from a team as code owners May 2, 2023 17:23
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label May 2, 2023
@allnes allnes added the platform: arm OpenVINO on ARM / ARM64 label May 2, 2023
@allnes allnes added this to the 2023.1 milestone May 2, 2023
@akladiev
Copy link
Collaborator

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

@akladiev akladiev added the Stale label May 23, 2023
@allnes allnes removed the Stale label May 23, 2023
Copy link
Contributor

@EgorDuplensky EgorDuplensky left a comment

Choose a reason for hiding this comment

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

Could you please execute dlb regression (with timeout = 1) just to verify functionality.

auto selectedPD = getSelectedPrimitiveDescriptor();
std::vector<MemoryDescPtr> srcDescs = {parentMem.getDescPtr()};
std::vector<MemoryDescPtr> dstDescs = {childMem.getDescPtr()};
execPtr = selectedPD->getExecutorFactoryAs<ConvertExecutorFactory>()->makeExecutor(convertParams,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please create a prepareParams method for the convert node to align with the rest of the nodes.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

const std::vector<MemoryDescPtr>& srcDescs,
const std::vector<MemoryDescPtr>& dstDescs) const override {
if (convertParams.srcPrc != convertParams.dstPrc) {
if (convertParams.srcPrc != InferenceEngine::Precision::I8 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use one_of() utils method for such cases
Here and below

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

@@ -217,7 +217,7 @@ void Transformations::PreLpt(const std::vector<ov::element::Type>& defaultPrecis
{ov::element::u16, ov::element::i32},
{ov::element::u32, ov::element::i32},
{ov::element::f64, ov::element::f32},
{ov::element::f16, ov::element::f32},
//{ov::element::f16, ov::element::f32},
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert

Copy link
Contributor

Choose a reason for hiding this comment

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

done

@@ -122,7 +122,7 @@ inline InferenceEngine::Precision normalizeToSupportedPrecision(InferenceEngine:
break;
}
case InferenceEngine::Precision::FP16: {
precision = InferenceEngine::Precision::FP32;
//precision = InferenceEngine::Precision::FP32;
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert

Copy link
Contributor

Choose a reason for hiding this comment

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

done

@maxnick maxnick merged commit 6822eb6 into openvinotoolkit:master Jul 13, 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.

5 participants