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

[Bindless][Exp] Rename external semaphore destroy func to release #1855

Merged

Conversation

Seanst98
Copy link
Contributor

@Seanst98 Seanst98 commented Jul 11, 2024

Rename function urBindlessImagesDestroyExternalSemaphoreExp to urBindlessImagesReleaseExternalSemaphoreExp.

This name change is to clarify exactly what is happening to the external semaphore. That is, it is being released not destroyed.

Corresponding DPC++ PR: intel/llvm#14535

@Seanst98 Seanst98 requested review from a team as code owners July 11, 2024 14:15
@Seanst98 Seanst98 requested a review from npmiller July 11, 2024 14:15
@github-actions github-actions bot added loader Loader related feature/bug images UR images specification Changes or additions to the specification experimental Experimental feature additions/changes/specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues native-cpu Native CPU adapter specific issues labels Jul 11, 2024
@Seanst98 Seanst98 force-pushed the sean/rename-external-semaphore-release branch from e7e465f to 60eb35f Compare July 12, 2024 10:37
@Seanst98
Copy link
Contributor Author

Friendly ping @oneapi-src/unified-runtime-native-cpu-write, @oneapi-src/unified-runtime-opencl-write, @oneapi-src/unified-runtime-cuda-write, @npmiller. Thanks!

Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU lgtm, thank you

@Seanst98
Copy link
Contributor Author

Friendly ping @oneapi-src/unified-runtime-opencl-write, @oneapi-src/unified-runtime-cuda-write, and @npmiller for reviews. We'd like to get this in for the ABI-Break window. Thanks!

Rename function urBindlessImagesDestroyExternalSemaphoreExp to
urBindlessImagesReleaseExternalSemaphoreExp.

This name change is to clarify exactly what is happening to the
external semaphore. That is, it is being released not destroyed.
@Seanst98 Seanst98 force-pushed the sean/rename-external-semaphore-release branch from 60eb35f to 037d3b5 Compare July 17, 2024 08:39
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.

CUDA and HIP changes LGTM

@Seanst98 Seanst98 added the ready to merge Added to PR's which are ready to merge label Jul 17, 2024
@kbenzie kbenzie merged commit 29c29e9 into oneapi-src:main Jul 17, 2024
51 of 54 checks passed
kbenzie added a commit that referenced this pull request Jul 17, 2024
…emaphore-release"

This reverts commit 29c29e9, reversing
changes made to 6c2329e.
kbenzie added a commit that referenced this pull request Jul 17, 2024
sarnex pushed a commit to intel/llvm that referenced this pull request Jul 18, 2024
… release (#14535)

Rename function destroy_external_semaphore to
release_external_semaphore.

This name change is to clarify exactly what is happening to the external
semaphore. That is, it is being released not destroyed.

Corresponding UR PR:
oneapi-src/unified-runtime#1855
@Seanst98 Seanst98 deleted the sean/rename-external-semaphore-release branch July 19, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda CUDA adapter specific issues experimental Experimental feature additions/changes/specification hip HIP adapter specific issues images UR images level-zero L0 adapter specific issues loader Loader related feature/bug native-cpu Native CPU adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants