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

[GPU] Fix issue for skipping gather #21887

Merged
merged 7 commits into from
Jan 4, 2024

Conversation

ahnyoung-paul
Copy link
Contributor

@ahnyoung-paul ahnyoung-paul commented Dec 27, 2023

Details:

  • checkout input layout is zero count before calling dep_memory()
  • add the case for input layout count is zero
  • in gather_inst, run build_deps before checking is_the_same_buffer(output_memory(), input_memory())

Tickets:

  • 128412
  • 128626

@ahnyoung-paul ahnyoung-paul added the category: GPU OpenVINO GPU plugin label Dec 27, 2023
@ahnyoung-paul ahnyoung-paul requested review from a team as code owners December 27, 2023 13:23
@ahnyoung-paul ahnyoung-paul force-pushed the fix_gather_issue branch 2 times, most recently from 465a430 to aee3c36 Compare December 29, 2023 05:34
@yeonbok
Copy link
Contributor

yeonbok commented Dec 29, 2023

Also as I mentioned in the offline chat, I think we can move do_rintime... stuffs to be done after empty primitive skipping. Because if the node is empty we do not need to do anything for that op already. Currently, the issue seems to happen for empty input primitive + its user's do_runtime_... check where it is trying to do something with its input mem. However we just can skip that user node if it is empty, before do the mem check and update things for those user nodes. So the priority of the optimization should be like this: runtime empty primitive skipping => further optimization in runtime (where we check the parent's memory, etc)

@ahnyoung-paul ahnyoung-paul force-pushed the fix_gather_issue branch 3 times, most recently from c61dec9 to 34429b6 Compare January 3, 2024 15:43
@yeonbok
Copy link
Contributor

yeonbok commented Jan 3, 2024

Note: To add corresponding test as a follow up work

@yeonbok yeonbok enabled auto-merge (squash) January 3, 2024 19:33
Copy link
Contributor

@kelvinchoi-intel kelvinchoi-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@yeonbok yeonbok merged commit f0cffc4 into openvinotoolkit:master Jan 4, 2024
78 checks passed
- checkout input layout is zero count before calling dep_memory()
- add the case for input layout count is zero
- in gather_inst, run build_deps before checking is_the_same_buffer(output_memory(), input_memory())
…ntime_skip_gather do_runtime_in_place_kv_cache
… current node is skpped for empty output tensor
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