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

Implicit command queue flushes triggered by clReleaseCommandQueue #1248

Open
kpet opened this issue Sep 3, 2024 · 3 comments
Open

Implicit command queue flushes triggered by clReleaseCommandQueue #1248

kpet opened this issue Sep 3, 2024 · 3 comments
Labels
needs-cts-coverage The CTS needs to be extended needs-new-opencl-minor-version This change requires a new minor version of OpenCL

Comments

@kpet
Copy link
Contributor

kpet commented Sep 3, 2024

The specification currently states

clReleaseCommandQueue performs an implicit flush to issue any previously queued OpenCL
commands in command_queue.

A few implementers observed in the 2024/09/03 teleconference that this leads to unwanted flushes when RAII objects that hold command queues frequently call clRetainCommandQueue and clReleaseCommandQueue. The intent may have been to solve the issue described in #1238.

Do people rely on this behaviour? Is this something we want to relax? Would we want explicit opt-in/opt-out control for this behaviour?

@bashbaug
Copy link
Contributor

bashbaug commented Sep 4, 2024

Do people rely on this behaviour? Is this something we want to relax? Would we want explicit opt-in/opt-out control for this behaviour?

I think the currently specified behavior makes sense when releasing last reference to the command-queue, because this causes the command-queue to become inaccessible thereby eliminating the ability to explicitly flush the command-queue in the future.

Would it make sense to relax the behavior so there is no implicit flush if the reference count is still nonzero after releasing? This avoid the unwanted (implicit) flushes with RAII objects but retains the implicit flush behavior when the last reference is released.

Depending how we resolve #1238 we may want to consider strengthening the behavior when the last reference is released. It would be interesting to understand how other APIs (e.g. Vulkan, SYCL) handle this case.

@bashbaug bashbaug added the needs-new-opencl-minor-version This change requires a new minor version of OpenCL label Sep 24, 2024
@kpet kpet added the needs-cts-coverage The CTS needs to be extended label Sep 24, 2024
@kpet
Copy link
Contributor Author

kpet commented Sep 24, 2024

Discussed in 2024/09/24 teleconference. Requiring the flush on each call to clReleaseCommandQueue has unwanted side-effects. Consensus that the intent likely was to free applications from having to insert explicit flushes before they relinquish their last refcount on a queue in multi-queue scenarios. Consensus (+1 from Arm, Intel, Qualcomm) to change the behaviour in the next minor version of OpenCL to only require the flush when the app is giving up its last refcount on the queue. Implementations would still be free to insert additional implicit flushes if they wanted.

@bashbaug
Copy link
Contributor

Discussed in the November 5th teleconference. Pending what we find in the tests added by KhronosGroup/OpenCL-CTS#2134, we may revisit whether fixing this will require a new minor OpenCL version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cts-coverage The CTS needs to be extended needs-new-opencl-minor-version This change requires a new minor version of OpenCL
Projects
Status: Needs WG discussion
Development

No branches or pull requests

2 participants