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

Added new cl_khr_semaphore tests to verify clReleaseSemaphoreKHR/clRetainSemaphoreKHR negative results #1976

Merged

Conversation

shajder
Copy link
Contributor

@shajder shajder commented Jun 3, 2024

According to work plan from issue #1691

-clReleaseSemaphoreKHR
-clRetainSemaphoreKHR
// Release invalid semaphore
cl_int err = CL_SUCCESS;
err = clReleaseSemaphoreKHR(nullptr);
test_failure_error(err, CL_INVALID_SEMAPHORE_KHR,
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: this will print "expected (unknown)" because harness/errorHelpers.cpp does not know that CL_INVALID_SEMAPHORE_KHR exists.

paulfradgley
paulfradgley previously approved these changes Jun 25, 2024
Copy link
Contributor

@paulfradgley paulfradgley left a comment

Choose a reason for hiding this comment

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

Looks good to me.

bashbaug
bashbaug previously approved these changes Jun 28, 2024
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

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

LGTM, other than @paulfradgley 's comment, which is more of a nice-to-have since the error message will not be printed unless the test fails.

@kpet
Copy link
Contributor

kpet commented Jul 2, 2024

2024/07/02 teleconference: @shajder Could you rebase/resolve conflicts please?

@shajder shajder dismissed stale reviews from bashbaug and paulfradgley via f4d9321 July 4, 2024 08:04
@nikhiljnv
Copy link
Contributor

Merging as discussed in memory subgroup call of July 9th 2024.

@nikhiljnv nikhiljnv merged commit 7b0f4ee into KhronosGroup:main Jul 16, 2024
7 checks passed
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.

5 participants