Skip to content

Conversation

@MichaelRizkalla-arm
Copy link
Contributor

This change extends the test coverage for KhronosGroup/OpenCL-Docs#1280

The change tests:

  1. Writing to immutable buffers.
  2. Writing to buffer/image from immutable buffers.
  3. Reading from immutable buffers.

This change adds the following tests:

  1. test_negative_imagearraycopy
  2. test_negative_imagearraycopy3d
  3. test_immutable_bufferreadwriterect
  4. test_immutable_arrayreadwrite
  5. test_write_from_immutable_buffer_to_buffer
  6. test_immutable_buffer_map_*

and extends the following tests:

  1. test_arrayimagecopy3d
  2. test_arrayimagecopy
  3. test_imagearraycopy3d
  4. test_imagearraycopy
  5. test_buffer_copy
  6. test_buffer_partial_copy

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.

I don't have access to a system to test these changes, but here are a few things I noticed reading through the code.

Comment on lines 30 to 32
extern const cl_mem_flags immutable_flag_set[];
extern const char* immutable_flag_set_names[];
#define NUM_IMMUTABLE_FLAGS 3
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this confusing.

  • Why is NUM_FLAGS equal to 5 when there are 8 entries in the array?
  • Why are there separate arrays for immutable flags that are used sometimes, but not every time?

Could we have a single array instead and filter based on the flags we care about testing?

Alternatively, could we have two distinct arrays, and iterate over both if we want to test both mutable and immutable flags?

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 followed the first option; a single array with all available flags to test. Updated buffer test suite existing tests to skip to new flags whenever needed.

@MichaelRizkalla-arm MichaelRizkalla-arm force-pushed the cl_ext_immutable_memory_objects_tests_part_2_buffers branch from f37293a to 6d388d7 Compare October 30, 2025 23:26
The change tests:
1. Writing to immutable buffers.
2. Writing to buffer/image from immutable buffers.
3. Reading from immutable buffers.

This change adds the following tests:
1. `test_negative_imagearraycopy`
2. `test_negative_imagearraycopy3d`
3. `test_immutable_bufferreadwriterect`
4. `test_immutable_arrayreadwrite`
5. `test_write_from_immutable_buffer_to_buffer`
6. `test_immutable_buffer_map_*`

and extends the following tests:
1. `test_arrayimagecopy3d`
2. `test_arrayimagecopy`
3. `test_imagearraycopy3d`
4. `test_imagearraycopy`
5. `test_buffer_copy`
6. `test_buffer_partial_copy`

Signed-off-by: Michael Rizkalla <michael.rizkalla@arm.com>
@MichaelRizkalla-arm MichaelRizkalla-arm force-pushed the cl_ext_immutable_memory_objects_tests_part_2_buffers branch from 6d388d7 to 5377c00 Compare October 30, 2025 23:28
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.

3 participants