Skip to content

Conversation

@RossBrunton
Copy link
Contributor

@RossBrunton RossBrunton commented Apr 4, 2025

Rather than using atexit handles to drop the adapter (which causes ordering issues with SYCL atexit handlers), urAdapterRetain/Release now update a reference counter. When the last adapter handle is Released, the adapter is dropped.

Since it is now simple to do so, the function cache has been moved to be a member of the adapter itself rather than being a separate global.

@RossBrunton RossBrunton marked this pull request as ready for review April 4, 2025 14:05
@RossBrunton RossBrunton requested review from a team as code owners April 4, 2025 14:05
@RossBrunton RossBrunton requested a review from keyradical April 4, 2025 14:05
Copy link
Contributor

@kbenzie kbenzie left a comment

Choose a reason for hiding this comment

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

I like moving the function pointer cache state into the adatper handle but that's not described in this PR description at all

@RossBrunton RossBrunton changed the title [UR] Use reference counting on OpenCL adapters [UR] Add reference counting and ext. fn cache to OpenCL adapters Apr 4, 2025
Copy link
Contributor

@keyradical keyradical left a comment

Choose a reason for hiding this comment

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

Command buffer LGTM.

Rather than using atexit handles to drop the adapter (which causes
ordering issues with SYCL atexit handlers),
`urAdapterRetain/Release` now update a reference counter. When the
last adapter handle is Released, the adapter is dropped.

Since it is now simple to do so, the function cache has been moved
to be a member of the adapter itself rather than being a separate
global.
@RossBrunton
Copy link
Contributor Author

@intel/llvm-gatekeepers Please merge.

@martygrant martygrant merged commit b6b25ee into intel:sycl Apr 11, 2025
32 checks passed
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.

5 participants