Skip to content

cherry-pick oneDNN master binary post-Ops optimization#14051

Closed
usstq wants to merge 2 commits into
openvinotoolkit:masterfrom
usstq:tq/onednn_bopt
Closed

cherry-pick oneDNN master binary post-Ops optimization#14051
usstq wants to merge 2 commits into
openvinotoolkit:masterfrom
usstq:tq/onednn_bopt

Conversation

@usstq
Copy link
Copy Markdown
Contributor

@usstq usstq commented Nov 17, 2022

Details:

  • cherry-pick oneDNN master binary post-Ops optimizations

Tickets:

  • MFDNN-8569

@usstq usstq requested review from a team as code owners November 17, 2022 01:21
@usstq
Copy link
Copy Markdown
Contributor Author

usstq commented Nov 17, 2022

@dmitry-gorokhov About your concern on newly added register, I double checked all changes, most of them have set preserve_gpr = true for binary injector to add push & pop instructions to preserve & restore/isolate the register access within the injected code, so there are no risk of overwriting.

the only two kernels setting preserve_gpr = false are jit_avx512_core_amx_1x1_conv_kernel.cpp and jit_avx512_core_amx_conv_kernel.cpp:

for jit_avx512_core_amx_1x1_conv_kernel.cpp, the new register bin_injector_helper_reg_3(r11) was shared by reg_bias and reg_scratch, from the source code I can see these two registers are actually used as a scratch register, there is a mov instruction for initializing them right before using them, and the author also added an additional conditional_register_preserve_guard_t to preserve bin_injector_helper_reg_3 in interleave_store.

for jit_avx512_core_amx_conv_kernel.cpp, the new register bin_injector_helper_reg_3 was shared with few other registers also, but again the author added an additional conditional_register_preserve_guard_t to preserve bin_injector_helper_reg_3 in store_output() which is the common function for all call path leading to post ops injector.

I also have done local test on 70+ INT8 models on SPR over 10-samples dataset and saw no accuracy issues.

Thus I believe the author was aware of this risk and handled it intentionally and successfully.

@dmitry-gorokhov dmitry-gorokhov added the category: CPU OpenVINO CPU plugin label Nov 17, 2022
@dmitry-gorokhov dmitry-gorokhov added this to the 2022.3 milestone Nov 17, 2022
@dmitry-gorokhov dmitry-gorokhov self-assigned this Nov 17, 2022
@usstq usstq closed this Dec 1, 2022
@usstq
Copy link
Copy Markdown
Contributor Author

usstq commented Dec 1, 2022

closed since it has been included in #13775

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.

2 participants