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.

Sign up for GitHub

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

Jump to bottom

[CPU] enable brdgmm_dw_conv kernels with non f32 bias #28076

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[CPU] enable brdgmm_dw_conv kernels with non f32 bias #28076

wants to merge 2 commits into from

Conversation

liubo-intel
Copy link
Contributor

Details:

  • brdgmm_dw_conv kernels support only bia_type the same as src_type or dst_type

Tickets:

@liubo-intel liubo-intel requested review from a team as code owners December 16, 2024 08:10
@github-actions github-actions bot added the category: CPU OpenVINO CPU plugin label Dec 16, 2024
@liubo-intel liubo-intel changed the title enable brdgmm_dw_conv kernels with non f32 bias [CPU] enable brdgmm_dw_conv kernels with non f32 bias Dec 16, 2024
@liubo-intel liubo-intel force-pushed the liubo/brgconv_dw_kernels_with_non_f32_bias branch from d5b88c4 to 28e92a0 Compare December 16, 2024 08:20
@yuxu42
Copy link
Contributor

yuxu42 commented Dec 26, 2024

Hi @luweizhou2016 could you review the PR? Thanks!

luweizhou2016
luweizhou2016 approved these changes Dec 27, 2024
View reviewed changes
Copy link
Contributor

@luweizhou2016 luweizhou2016 left a comment

Choose a reason for hiding this comment

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

As we discussed, update the JIRA ticket with the perf gain reason as track.

  1. SPR: jit_uni_dw_conv on avx512_core_bf16 -> brdg conv on avx512_core_bf16
  2. GNR : brgconv on AMX_fp16 -> brdg conv on avx512_fp16

src/plugins/intel_cpu/src/nodes/conv.cpp Outdated
@@ -980,6 +980,10 @@ void Convolution::createDescriptor(const std::vector<MemoryDescPtr>& inputDesc,
memory::data_type bdt = outDnnlDesc.get_data_type();
#else
memory::data_type bdt = memory::data_type::f32;
// brdgmm_dw_conv supports only bia_type the same as src_type or dst_type
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add more comments here:
/* brdgmm_dw_conv has more perf gain on bf16/fp16 inference.
brdgmm_dw_conv supports only bia_type the same as src_type or dst_type.
dw convolution support in onednn 3.5.
BF16:
kernel type | brgdconv | jit_uni_dw_convolution_fwd_t
support impl type | native bf16 ISA without AMX | avx512_core_bf16 or avx512_core
bias dt | oneof(src,dest) | oneof(src, dest, f32)

FP16:
kernel type | brgdconv | brgemm_convolution_fwd_t
impl type | native FP16 ISA without AMX | native FP16 ISA
bias type | oneof(src,dest) | oneof(src, dest, f32)
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@liubo-intel liubo-intel force-pushed the liubo/brgconv_dw_kernels_with_non_f32_bias branch from 28e92a0 to 12133d1 Compare December 27, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luweizhou2016 luweizhou2016 luweizhou2016 approved these changes

Assignees
No one assigned
Labels
category: CPU OpenVINO CPU plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@liubo-intel @yuxu42 @luweizhou2016