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] Implemented "jit_exp_emitter" #26974

Merged

Conversation

a-sidorova
Copy link
Contributor

@a-sidorova a-sidorova commented Oct 9, 2024

Details:

  • Previously, we used dnnl-injector for Exp op which require 2 aux_vec_regs. The snippets kernel have some pool of aux vec registers which can be used by emitters in their implementations. However, dnnl cannot work with user-provided aux registers and always spill them on stack while plugin emitters can do it. To avoid extra push-pop in Snippets kernel (it leads to performance degradations), we implemented own emitter for Exp with the same logic to have opportunity to pass free aux vec registers
  • Updated jit_erf_emitter: reused new jit_exp_emitter to compute exponent and now we work only with vmm_dst to avoid vmm_src data corruption (input registers must not be corrupted)

Tickets:

  • 155236

@a-sidorova a-sidorova requested review from a team as code owners October 9, 2024 13:07
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Oct 9, 2024
@a-sidorova
Copy link
Contributor Author

@IvanNovoselov could you take a look please? Thank you in advance

Copy link
Contributor

@IvanNovoselov IvanNovoselov left a comment

Choose a reason for hiding this comment

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

LGTM
Please note that we need to carefully validate both accuracy and performance for this PR.

Copy link
Contributor

@v-Golubev v-Golubev left a comment

Choose a reason for hiding this comment

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

LGTM

@a-sidorova
Copy link
Contributor Author

The accuracy and performance validations are successfully passed. For more information, please see the mentioned ticket in the description.

PR is ready to be merged 🚀

@dmitry-gorokhov dmitry-gorokhov added this pull request to the merge queue Oct 22, 2024
Merged via the queue into openvinotoolkit:master with commit e3ad821 Oct 22, 2024
153 checks passed
@dmitry-gorokhov dmitry-gorokhov deleted the feature/cpu/exp_emitter branch October 22, 2024 07:26
CuriousPanCake pushed a commit to CuriousPanCake/openvino that referenced this pull request Nov 6, 2024
### Details:
- *Previously, we used dnnl-injector for `Exp` op which require 2
`aux_vec_regs`. The snippets kernel have some pool of aux vec registers
which can be used by emitters in their implementations. However, dnnl
cannot work with user-provided aux registers and always spill them on
stack while plugin emitters can do it. To avoid extra push-pop in
Snippets kernel (it leads to performance degradations), we implemented
own emitter for `Exp` with the same logic to have opportunity to pass
free aux vec registers*
- *Updated `jit_erf_emitter`: reused new `jit_exp_emitter` to compute
exponent and now we work only with `vmm_dst` to avoid `vmm_src` data
corruption (input registers must not be corrupted)*

### Tickets:
 - *155236*
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.

4 participants