Skip to content

[GPU] to use original dims for fc_onednn_impl #30087

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

Merged
merged 8 commits into from
Apr 18, 2025

Conversation

e-ddykim
Copy link
Contributor

Details:

  • This PR updates fc_onednn_impl to use original dims instead of squashing input layout to 2D.
  • It resolves layout mismatch problems between fc and post-ops.

Tickets:

  • 164104, 164106

<< orig_impl_param.typed_desc<fully_connected>()->id
<< " " << input_shape
<< " + " << fuse_op_input_shape << std::endl;
can_apply_fake_alignment = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

NOTE: I think we can add a reshape for this case in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I'll try it in the future as you reviewed. Thank you!

@yeonbok yeonbok added this to the 2025.2 milestone Apr 15, 2025
@@ -66,14 +66,14 @@ format::type get_preferred_format(fully_connected_node const& node, const kernel
}

if (input_layout.data_type == data_types::f32 &&
(input_layout.format == format::bfyx || input_layout.format == format::bfzyx) &&
(input_layout.format == format::bfyx || input_layout.format == format::bfzyx || input_layout.format == format::bfwzyx) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: You can use one_of(input_layout.format, format::bfyx, format::bfzyx, format::bfwzyx).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated as you reviewed. Thank you.

group_size = ifm / ngroups;
if (!is_four_bit_weight) {
// 8-bit quantized weight
attr->set_scales(DNNL_ARG_WEIGHTS, PER_OC, dnnl::memory::dims{}, ds_data_type);
attr->set_scales(DNNL_ARG_WEIGHTS, (PER_OC << shift_size), dnnl::memory::dims{}, ds_data_type);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: what about introducing a local variable per_oc = PER_OC << shift_size? That will reduce duplicated expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated as you reviewed. Thank you.

@e-ddykim e-ddykim force-pushed the gpu_onednn_fc_fix branch 2 times, most recently from a1e95d1 to 4d8f7dd Compare April 16, 2025 08:44
@e-ddykim e-ddykim force-pushed the gpu_onednn_fc_fix branch from 4d8f7dd to 4b0097b Compare April 17, 2025 13:43
Copy link
Contributor

@isanghao isanghao left a comment

Choose a reason for hiding this comment

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

no perf issue from dgpu static-shape daily test

@isanghao isanghao added this pull request to the merge queue Apr 18, 2025
Merged via the queue into openvinotoolkit:master with commit 95122bd Apr 18, 2025
169 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: GPU OpenVINO GPU plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants