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 external semaphore test when exportability is not supported #2045

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

saurabhnv
Copy link
Contributor

An implementation may not support exportable semaphore, subtests available in cl_khr_external_semaphore assumes support for exportable semaphore, resulting in failure on such implementation. Allow implementations to use importable semaphore in such cases.

* Allow get_device_semaphore_handle_types to return
  CL_SUCCESS for case where implementation returns '0'
  size for import/export handle type query to indicate
  that it doesn't support importing/exporting semaphore.
* If cl_khr_external_semaphore is reported, add condition
  to check any of import/export or maybe both are supported.
* Current test performs a union rather than intersection of
  import and export handle types list, perform an intersection
  to find common handle types.
An implementation may not support exportable semaphore, subtests
available as part of cl_khr_external_semaphore assumes support for
exportable semaphore, resulting in failure on such implementation.
Instead of skipping these test, allow importable semaphore creation for
the purpose of testing.
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.

Note, I didn't try running these changes.

Comment on lines +104 to +110
std::vector<cl_external_semaphore_handle_type_khr>
handle_types_query_result(num_handle_types);
err =
clGetDeviceInfo(deviceID, param,
handle_types_query_result.size()
* sizeof(cl_external_semaphore_handle_type_khr),
handle_types_query_result.data(), nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: could we read directly into the handle_types vector here, rather than reading into a temporary vector and then copying?

@bashbaug
Copy link
Contributor

bashbaug commented Sep 3, 2024

Merging as discussed in the September 3rd memory subgroup.

@bashbaug bashbaug merged commit 7131f87 into KhronosGroup:main Sep 3, 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.

4 participants