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

[CPU] Removed contexts from Load/Store emitters #12446

Conversation

a-sidorova
Copy link
Contributor

@a-sidorova a-sidorova commented Aug 6, 2022

Details:

  • Removed contexts from Load/Store emitters. At the moment Store pollutes src_vmm register in cases when src and dst precisions are different. So in the next stage in jit pipeline our src_vmm can be broken. To avoid additional aux_vmm in each store emitter this PR removes contexts to create exact emitter using ctor. Thus, count of aux registers is known it the moment emitter creation and this count is different for each case (at the moment in master Store and Load emitters have hardcoded count of aux regs
  • Updated Interpolate, MVN, NMS, ROI Align, ROI Pooling, TopK, ColorConvert

Tickets:

  • 89024

@dmitry-gorokhov dmitry-gorokhov added the category: CPU OpenVINO CPU plugin label Aug 7, 2022
@dmitry-gorokhov dmitry-gorokhov added this to the 2022.2 milestone Aug 7, 2022
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch from 0f270c4 to f30705a Compare August 8, 2022 12:32
@a-sidorova a-sidorova marked this pull request as ready for review August 8, 2022 12:39
@a-sidorova a-sidorova requested review from a team as code owners August 8, 2022 12:39
@dmitry-gorokhov dmitry-gorokhov self-assigned this Aug 8, 2022
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch 3 times, most recently from 3232a90 to 467483c Compare August 10, 2022 10:15
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch 2 times, most recently from 958ab3f to 2f8b55a Compare August 11, 2022 05:14
@dmitry-gorokhov
Copy link
Contributor

@a-sidorova It is always a good practice to put motivation of the changes directly in the description of the PR.

@dmitry-gorokhov
Copy link
Contributor

dmitry-gorokhov commented Aug 12, 2022

@xuchen-intel @chenhu-wang FYI. If you have any comments feel free to add.

@dmitry-gorokhov dmitry-gorokhov modified the milestones: 2022.2, 2022.3 Aug 12, 2022
src/plugins/intel_cpu/src/nodes/non_max_suppression.cpp Outdated Show resolved Hide resolved
src/plugins/intel_cpu/src/nodes/interpolate.cpp Outdated Show resolved Hide resolved
src/plugins/intel_cpu/src/nodes/roi_align.cpp Outdated Show resolved Hide resolved
src/plugins/intel_cpu/src/nodes/roi_align.cpp Outdated Show resolved Hide resolved
src/tests/unit/cpu/jit_kernel_test.cpp Outdated Show resolved Hide resolved
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch from 2f8b55a to 34164b6 Compare August 12, 2022 15:20
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch from 34164b6 to 4aea160 Compare August 12, 2022 15:44
Copy link
Contributor

@dmitry-gorokhov dmitry-gorokhov left a comment

Choose a reason for hiding this comment

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

LGTM. Can be merged after fixing remaining comment and passing internal validation.

@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch from 4aea160 to 8fa8d92 Compare August 15, 2022 05:24
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch from 02deb5c to 37add92 Compare August 15, 2022 14:58
@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch 3 times, most recently from 939ad6e to 1f22e69 Compare August 15, 2022 18:21
@a-sidorova
Copy link
Contributor Author

a-sidorova commented Aug 16, 2022

@chenhu-wang thanks a lot!

@dmitry-gorokhov DLB has been passed. The reports are attached to the ticket

@a-sidorova a-sidorova force-pushed the feature/emitters/remove_contexts branch from 258b081 to 15e091f Compare August 17, 2022 13:05
@dmitry-gorokhov dmitry-gorokhov merged commit 6d6f528 into openvinotoolkit:master Aug 17, 2022
a-sidorova added a commit to a-sidorova/openvino that referenced this pull request Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: CPU OpenVINO CPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants