Skip to content

[UR][HIP] Refactor reference counting in UR HIP adapter using new ur::RefCount class #19387

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

Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from

Conversation

martygrant
Copy link
Contributor

@martygrant martygrant commented Jul 10, 2025

For #18644

Most UR adapters had their own reference counting of some sort. This adds a new RefCount class and refactors adapter code so all adapters can share the same code for reference counting. This PR handles HIP.

I have kept the pre-existing reference counting members inside stream_queue.hpp, temporarily renaming it as RefCountOld as the CUDA adapter also uses this and want to only change one adapter per PR. I will change this in the final PR for this piece of work.

@martygrant martygrant force-pushed the refCountRefactorHIP branch from cb97307 to bdf41ce Compare July 10, 2025 11:14
@martygrant martygrant force-pushed the refCountRefactorHIP branch from bdf41ce to 6de79b3 Compare July 10, 2025 12:50
@martygrant martygrant force-pushed the refCountRefactorHIP branch from 6de79b3 to d2740e6 Compare July 10, 2025 12:56
@martygrant martygrant force-pushed the refCountRefactorHIP branch from d2740e6 to 3cea679 Compare July 10, 2025 13:03
@martygrant martygrant force-pushed the refCountRefactorHIP branch from 3cea679 to 2beb1e2 Compare July 10, 2025 13:30
@martygrant martygrant force-pushed the refCountRefactorHIP branch from 2beb1e2 to 11bee99 Compare July 10, 2025 13:38
@martygrant martygrant force-pushed the refCountRefactorHIP branch from 11bee99 to d720bc6 Compare July 10, 2025 13:49
@martygrant martygrant marked this pull request as ready for review July 10, 2025 15:23
@martygrant martygrant requested review from a team as code owners July 10, 2025 15:23
@martygrant martygrant requested review from frasercrmck and reble July 10, 2025 15:23
@npmiller
Copy link
Contributor

The CUDA and HIP adapters are very similar and covered by the same reviewer team, it would be totally fine to migrate them together in one PR. If you have the CUDA changes already I'd suggest just adding them here, but otherwise it's fine.

Copy link
Contributor

@npmiller npmiller left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@reble reble left a comment

Choose a reason for hiding this comment

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

LGTM for command buffer related changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants