Skip to content

Conversation

sungeunk
Copy link
Contributor

Description of the issue(symptom, root-cause, how it was resolved)

  • onednn reorder can not handle the feature axis padding. 'is_supported_pad()' function does not check the feature axis padding.
  • fallback to ocl if the reorder has the feature axis padding.

The code and line that caused this issue (if it is not changed directly)

  • src/plugins/intel_gpu/src/graph/impls/onednn/reorder_onednn.cpp
  • src/plugins/intel_gpu/src/graph/impls/onednn/utils.cpp

Reproduction step and snapshot (if applicable. Do not attach for customer model)

  • reproducer is attached in the ticket
image

Problematic graph

image

Checklist

  • Is it a proper fix? (not a workaround)
  • Did you include test case for this fix, if necessary?
  • Did you review existing test that can be extended to cover this scenario? Which test did you review?

Tickets:

  • 172242

@sungeunk sungeunk added the category: GPU OpenVINO GPU plugin label Aug 19, 2025
@sungeunk sungeunk requested review from a team as code owners August 19, 2025 13:12
@sungeunk sungeunk requested a review from Copilot August 19, 2025 13:12
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the OneDNN reorder implementation where feature axis padding was not being properly checked, causing unsupported operations to be attempted instead of falling back to OpenCL.

  • Added feature axis padding validation to the is_supported_pad() function
  • Modified the return condition to include the new feature padding check alongside existing spatial and batch padding checks

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@geunhwan geunhwan left a comment

Choose a reason for hiding this comment

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

PR in the master branch has been approved.

Comment on lines +101 to +105
bool no_feature_padding = true;
no_feature_padding &= (pad._lower_size[1] == 0);
no_feature_padding &= (pad._upper_size[1] == 0);

return no_feature_padding;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool no_feature_padding = true;
no_feature_padding &= (pad._lower_size[1] == 0);
no_feature_padding &= (pad._upper_size[1] == 0);
return no_feature_padding;
return (pad._lower_size[1] == 0) && (pad._upper_size[1] == 0);

Can be refactored?

@sungeunk sungeunk force-pushed the 172242_cyberlink_onednn_reorder_pad_release_2025_3 branch from 3f8d8b7 to 0598987 Compare August 20, 2025 07:45
@geunhwan geunhwan added this to the 2025.3 milestone Aug 20, 2025
@moslex moslex added the priority: high High piority label Aug 20, 2025
@p-durandin
Copy link
Contributor

build_jenkins

@sungeunk sungeunk force-pushed the 172242_cyberlink_onednn_reorder_pad_release_2025_3 branch from 0598987 to 41eb55d Compare August 20, 2025 13:04
@sungeunk sungeunk force-pushed the 172242_cyberlink_onednn_reorder_pad_release_2025_3 branch from 41eb55d to 80e9129 Compare August 20, 2025 14:20
@yeonbok yeonbok added this pull request to the merge queue Aug 20, 2025
github-merge-queue bot pushed a commit that referenced this pull request Aug 20, 2025
### Description of the issue(symptom, root-cause, how it was resolved)
- onednn reorder can not handle the feature axis padding.
'is_supported_pad()' function does not check the feature axis padding.
 - fallback to ocl if the reorder has the feature axis padding.

#### The code and line that caused this issue (if it is not changed
directly)
 - src/plugins/intel_gpu/src/graph/impls/onednn/reorder_onednn.cpp
 - src/plugins/intel_gpu/src/graph/impls/onednn/utils.cpp

#### Reproduction step and snapshot (if applicable. Do not attach for
customer model)
 - reproducer is attached in the ticket
<img width="912" height="446" alt="image"
src="https://github.com/user-attachments/assets/db41a6f7-8738-4012-b5d7-6259cb922a0e"
/>

#### Problematic graph
<img width="1308" height="885" alt="image"
src="https://github.com/user-attachments/assets/b4ee632b-8375-4185-b7d1-b31bfe95b786"
/>


#### Checklist
 - [ ] Is it a proper fix? (not a workaround)
 - [ ] Did you include test case for this fix, if necessary?
- [ ] Did you review existing test that can be extended to cover this
scenario? Which test did you review?

### Tickets:
 - 172242
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 21, 2025
@sungeunk sungeunk added this pull request to the merge queue Aug 21, 2025
Merged via the queue into openvinotoolkit:releases/2025/3 with commit 3b461d5 Aug 21, 2025
193 of 195 checks passed
@sungeunk sungeunk deleted the 172242_cyberlink_onednn_reorder_pad_release_2025_3 branch August 21, 2025 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: GPU OpenVINO GPU plugin Code Freeze priority: high High piority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants