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

Fix signalling and waiting on semaphore from two queues #2271

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

Conversation

MichaelRizkalla-arm
Copy link
Contributor

semaphores_ooo_ops_cross_queue uses two OOO command queues to run the test. In one queue, a semaphore is signalled and the semahpore is waited on in the other queue.

The CL specification requires the application to synchronize the queues if objects are shared.

Comment on lines 274 to 275
err = clFinish(producer_queue);
test_error(err, "clFinish failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a clFlush be sufficient here, or does this need to be a clFinish?

I believe both are correct, but the test is a little stronger if it's a clFlush and not a clFinish. With a clFinish, the semaphore is a NOP for an implementation that maximally batches commands.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, flush is a bit more appropriate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the commit to use clFlush

`semaphores_ooo_ops_cross_queue` uses two OOO command queues to run
the test. In one queue, a semaphore is signalled and the semahpore is
waited on in the other queue.

The CL specification requires the application to synchronize the queues
if objects are shared.

Signed-off-by: Michael Rizkalla <[email protected]>
@MichaelRizkalla-arm MichaelRizkalla-arm force-pushed the fix_semaphore_ooo_queues_test branch from f7a2b6a to 57edc75 Compare February 20, 2025 11:17
@bcalidas
Copy link

Looks good for Qualcomm

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

Successfully merging this pull request may close these issues.

4 participants