-
Notifications
You must be signed in to change notification settings - Fork 216
Add tests for cl_ext_immutable_memory_objects #2286
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
Add tests for cl_ext_immutable_memory_objects #2286
Conversation
Looks like this is another one where we'll need header changes before we can merge. Would it be possible to get a draft headers PR up with these changes so this won't become a bottleneck once the tests have been reviewed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a device to test these changes, and I can't even build without the header changes, but the test changes look like a good start.
The command buffer negative testing looks pretty comprehensive. Are there plans to have negative tests for command-queue fills and copies also?
Are there any plans to extend positive testing to confirm that we can read from immutable buffers and images?
Thanks for the review. There is a plan to add more tests to cover command queue's fills and copies and reading from buffers and images. |
Here's a PR adding definitions to the headers: KhronosGroup/OpenCL-Headers#274 |
e0cb8e5
to
41652c3
Compare
Please re-add "focused review" once the merge conflicts are resolved. |
@MichaelRizkalla-arm Could you resolve the merge conflicts please? |
This change does the following: 1. Create `negative_enqueue_map_image` with tests for mapping images created with `CL_MEM_IMMUTABLE_EXT` 2. Update `enqueue_map_*` with more variants of `cl_mem_flags` Signed-off-by: Michael Rizkalla <[email protected]>
This change updates `cl_khr_command_buffer` test binary to include behaviour with `CL_MEM_IMMUTABLE_EXT` memory flag. Added the following tests: 1. negative_copy_to_immutable_buffer 2. negative_copy_image_to_immutable_buffer 3. negative_copy_to_immutable_buffer_rect 4. negative_copy_to_immutable_image 5. negative_copy_buffer_to_immutable_image 6. negative_fill_immutable_image 7. negative_fill_immutable_buffer Signed-off-by: Michael Rizkalla <[email protected]>
This change adds `negative_set_immutable_memory_to_writeable_kernel_arg` to verify setting memory arguments marked with `CL_MEM_IMMUTABLE_EXT`. Signed-off-by: Michael Rizkalla <[email protected]>
41652c3
to
b13aca0
Compare
I have updated the PR to resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I don't have a way to test these tests, but I did confirm that they all properly skip for a device that does not support the immutable memory objects extension.
Merging as discussed in the June 17th memory subgroup. |
This change provides partial test coverage for KhronosGroup/OpenCL-Docs#1280
Adding CTS tests for:
The bulk of the tests is to make sure that the CL driver does not allow writing to a memory object that is created with
CL_MEM_IMMUTABLE_EXT
flag when used with the above APIs.