From ab6618ba3720aa990c5a5352a0c8b8880f9a19ca Mon Sep 17 00:00:00 2001 From: Marcin Hajder Date: Tue, 8 Jul 2025 13:47:14 +0200 Subject: [PATCH 1/4] Added sub-case for basic test where the provided pitches are zero --- .../basic/test_bufferreadwriterect.cpp | 132 +++++++++++------- 1 file changed, 83 insertions(+), 49 deletions(-) diff --git a/test_conformance/basic/test_bufferreadwriterect.cpp b/test_conformance/basic/test_bufferreadwriterect.cpp index f334115a6f..196cb9b839 100644 --- a/test_conformance/basic/test_bufferreadwriterect.cpp +++ b/test_conformance/basic/test_bufferreadwriterect.cpp @@ -88,48 +88,70 @@ void print_buffer(BufferType* buf, size_t w, size_t h, size_t d) { } // Returns true if the two specified regions overlap. -bool check_overlap_rect(size_t src_offset[3], - size_t dst_offset[3], - size_t region[3], - size_t row_pitch, - size_t slice_pitch) +bool check_overlap_rect(size_t src_offset[3], size_t dst_offset[3], + size_t region[3], size_t src) { + // Copy between cl buffers. + size_t slice_pitch = + (width[src] * height[src] != 1) ? width[src] * height[src] : 0; + size_t row_pitch = width[src]; + const size_t src_min[] = { src_offset[0], src_offset[1], src_offset[2] }; - const size_t src_max[] = { src_offset[0] + region[0], src_offset[1] + region[1], src_offset[2] + region[2] }; + const size_t src_max[] = { src_offset[0] + region[0], + src_offset[1] + region[1], + src_offset[2] + region[2] }; const size_t dst_min[] = { dst_offset[0], dst_offset[1], dst_offset[2] }; const size_t dst_max[] = { dst_offset[0] + region[0], dst_offset[1] + region[1], - dst_offset[2] + region[2]}; -// Check for overlap - bool overlap = true; - unsigned i; - for (i = 0; i != 3; ++i) - { - overlap = overlap && (src_min[i] < dst_max[i]) && (src_max[i] > dst_min[i]); - } + dst_offset[2] + region[2] }; + // Check for overlap + bool overlap = true; + unsigned i; + for (i = 0; i != 3; ++i) + { + overlap = + overlap && (src_min[i] < dst_max[i]) && (src_max[i] > dst_min[i]); + } - size_t dst_start = dst_offset[2] * slice_pitch + dst_offset[1] * row_pitch + dst_offset[0]; - size_t dst_end = dst_start + (region[2] * slice_pitch + - region[1] * row_pitch + region[0]); - size_t src_start = src_offset[2] * slice_pitch + src_offset[1] * row_pitch + src_offset[0]; - size_t src_end = src_start + (region[2] * slice_pitch + - region[1] * row_pitch + region[0]); - if (!overlap) { - size_t delta_src_x = (src_offset[0] + region[0] > row_pitch) ? - src_offset[0] + region[0] - row_pitch : 0; size_t delta_dst_x = (dst_offset[0] + region[0] > row_pitch) ? - dst_offset[0] + region[0] - row_pitch : 0; - if ((delta_src_x > 0 && delta_src_x > dst_offset[0]) || - (delta_dst_x > 0 && delta_dst_x > src_offset[0])) { - if ((src_start <= dst_start && dst_start < src_end) || (dst_start <= src_start && src_start < dst_end)) overlap = true; + size_t dst_start = + dst_offset[2] * slice_pitch + dst_offset[1] * row_pitch + dst_offset[0]; + size_t dst_end = dst_start + + (region[2] * slice_pitch + region[1] * row_pitch + region[0]); + size_t src_start = + src_offset[2] * slice_pitch + src_offset[1] * row_pitch + src_offset[0]; + size_t src_end = src_start + + (region[2] * slice_pitch + region[1] * row_pitch + region[0]); + if (!overlap) + { + size_t delta_src_x = (src_offset[0] + region[0] > row_pitch) + ? src_offset[0] + region[0] - row_pitch + : 0; + size_t delta_dst_x = (dst_offset[0] + region[0] > row_pitch) + ? dst_offset[0] + region[0] - row_pitch + : 0; + if ((delta_src_x > 0 && delta_src_x > dst_offset[0]) + || (delta_dst_x > 0 && delta_dst_x > src_offset[0])) + { + if ((src_start <= dst_start && dst_start < src_end) + || (dst_start <= src_start && src_start < dst_end)) + overlap = true; } - if (region[2] > 1) { - size_t src_height = slice_pitch / row_pitch; size_t dst_height = slice_pitch / row_pitch; - size_t delta_src_y = (src_offset[1] + region[1] > src_height) ? src_offset[1] + region[1] - src_height : 0; - size_t delta_dst_y = (dst_offset[1] + region[1] > dst_height) ? dst_offset[1] + region[1] - dst_height : 0; - if ((delta_src_y > 0 && delta_src_y > dst_offset[1]) || - (delta_dst_y > 0 && delta_dst_y > src_offset[1])) { - if ((src_start <= dst_start && dst_start < src_end) || (dst_start <= src_start && src_start < dst_end)) + if (region[2] > 1) + { + size_t src_height = slice_pitch / row_pitch; + size_t dst_height = slice_pitch / row_pitch; + size_t delta_src_y = (src_offset[1] + region[1] > src_height) + ? src_offset[1] + region[1] - src_height + : 0; + size_t delta_dst_y = (dst_offset[1] + region[1] > dst_height) + ? dst_offset[1] + region[1] - dst_height + : 0; + if ((delta_src_y > 0 && delta_src_y > dst_offset[1]) + || (delta_dst_y > 0 && delta_dst_y > src_offset[1])) + { + if ((src_start <= dst_start && dst_start < src_end) + || (dst_start <= src_start && src_start < dst_end)) overlap = true; } } @@ -137,29 +159,38 @@ bool check_overlap_rect(size_t src_offset[3], return overlap; } - - // This function invokes the CopyBufferRect CL command and then mirrors the operation on the host side verify buffers. -int copy_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, size_t doffset[3], size_t dregion[3]) { - +int copy_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, + size_t doffset[3], size_t dregion[3], bool comp_pitch = true) +{ // Copy between cl buffers. - size_t src_slice_pitch = (width[src]*height[src] != 1) ? width[src]*height[src] : 0; - size_t dst_slice_pitch = (width[dst]*height[dst] != 1) ? width[dst]*height[dst] : 0; - size_t src_row_pitch = width[src]; + size_t src_slice_pitch = (width[src] * height[src] != 1 && comp_pitch) + ? width[src] * height[src] + : 0; + size_t dst_slice_pitch = (width[dst] * height[dst] != 1 && comp_pitch) + ? width[dst] * height[dst] + : 0; + size_t src_row_pitch = comp_pitch ? width[src] : 0; + size_t dst_row_pitch = comp_pitch ? width[dst] : 0; cl_int err; - if (check_overlap_rect(soffset,doffset,sregion,src_row_pitch, src_slice_pitch)) { - log_info( "Copy overlap reported, skipping copy buffer rect\n" ); + if (check_overlap_rect(soffset, doffset, sregion, src)) + { + log_info("Copy overlap reported, skipping copy buffer rect\n"); return CL_SUCCESS; - } else { + } + else + { if ((err = clEnqueueCopyBufferRect( gQueue, buffer[src], buffer[dst], soffset, doffset, sregion, /*dregion,*/ - width[src], src_slice_pitch, width[dst], dst_slice_pitch, 0, - NULL, NULL)) + src_row_pitch, src_slice_pitch, dst_row_pitch, dst_slice_pitch, + 0, NULL, NULL)) != CL_SUCCESS) { - CL_EXIT_ERROR(err, "clEnqueueCopyBufferRect failed between %u and %u",(unsigned)src,(unsigned)dst); + CL_EXIT_ERROR(err, + "clEnqueueCopyBufferRect failed between %u and %u", + (unsigned)src, (unsigned)dst); } } @@ -492,16 +523,19 @@ REGISTER_TEST(bufferreadwriterect) size_t operation = get_random_size_t(0,TotalOperations,mt); switch (operation) { - case 0: + case 0: { log_info("%zu Copy %zu offset (%zu,%zu,%zu) -> %zu offset " "(%zu,%zu,%zu) region (%zux%zux%zu = %zu)\n", iter, src, soffset[0], soffset[1], soffset[2], dst, doffset[0], doffset[1], doffset[2], sregion[0], sregion[1], sregion[2], sregion[0] * sregion[1] * sregion[2]); - if ((err = copy_region(src, soffset, sregion, dst, doffset, dregion))) + bool comp_pitch = (genrand_int32(mt) % 2) != 0 ? true : false; + if ((err = copy_region(src, soffset, sregion, dst, doffset, + dregion, comp_pitch))) return err; break; + } case 1: log_info("%zu Read %zu offset (%zu,%zu,%zu) -> %zu offset " "(%zu,%zu,%zu) region (%zux%zux%zu = %zu)\n", From 92e20de0edbbf0e2fd241c872476aa2fabee4971 Mon Sep 17 00:00:00 2001 From: Marcin Hajder Date: Tue, 2 Sep 2025 11:49:36 +0200 Subject: [PATCH 2/4] Corrections due to code review --- .../basic/test_bufferreadwriterect.cpp | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/test_conformance/basic/test_bufferreadwriterect.cpp b/test_conformance/basic/test_bufferreadwriterect.cpp index 8c5622af85..e2a5315406 100644 --- a/test_conformance/basic/test_bufferreadwriterect.cpp +++ b/test_conformance/basic/test_bufferreadwriterect.cpp @@ -163,16 +163,24 @@ bool check_overlap_rect(size_t src_offset[3], size_t dst_offset[3], int copy_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, size_t doffset[3], size_t dregion[3]) { - bool comp_pitch = (genrand_int32(mt) % 2) != 0 ? true : false; - // Copy between cl buffers. - size_t src_slice_pitch = (width[src] * height[src] != 1 && comp_pitch) + // Copy between cl buffers + // Compute or provide zero pitches randomly + size_t src_slice_pitch = + (width[src] * height[src] != 1 && ((genrand_int32(mt) % 2) != 0)) ? width[src] * height[src] : 0; - size_t dst_slice_pitch = (width[dst] * height[dst] != 1 && comp_pitch) + size_t dst_slice_pitch = + (width[dst] * height[dst] != 1 && ((genrand_int32(mt) % 2) != 0)) ? width[dst] * height[dst] : 0; - size_t src_row_pitch = comp_pitch ? width[src] : 0; - size_t dst_row_pitch = comp_pitch ? width[dst] : 0; + size_t src_row_pitch = ((genrand_int32(mt) % 2) != 0) ? width[src] : 0; + size_t dst_row_pitch = ((genrand_int32(mt) % 2) != 0) ? width[dst] : 0; + + if (src == dst) + { + src_row_pitch = dst_row_pitch; + src_slice_pitch = dst_slice_pitch; + } cl_int err; if (check_overlap_rect(soffset, doffset, sregion, src)) @@ -538,18 +546,17 @@ static int test_bufferreadwriterect_impl(cl_device_id device, size_t operation = get_random_size_t(0,TotalOperations,mt); switch (operation) { - case 0: { + case 0: log_info("%zu Copy %zu offset (%zu,%zu,%zu) -> %zu offset " "(%zu,%zu,%zu) region (%zux%zux%zu = %zu)\n", iter, src, soffset[0], soffset[1], soffset[2], dst, doffset[0], doffset[1], doffset[2], sregion[0], sregion[1], sregion[2], sregion[0] * sregion[1] * sregion[2]); - if ((err = copy_region(src, soffset, sregion, dst, doffset, - dregion))) + if ((err = test_functions.copy(src, soffset, sregion, dst, + doffset, dregion))) return err; break; - } case 1: log_info("%zu Read %zu offset (%zu,%zu,%zu) -> %zu offset " "(%zu,%zu,%zu) region (%zux%zux%zu = %zu)\n", From 862a23daacb032ddcf9357e35524547a9955c9c6 Mon Sep 17 00:00:00 2001 From: Marcin Hajder Date: Thu, 19 Mar 2026 11:55:20 +0100 Subject: [PATCH 3/4] corrections due to code review --- .../basic/test_bufferreadwriterect.cpp | 91 ++++++++----------- 1 file changed, 40 insertions(+), 51 deletions(-) diff --git a/test_conformance/basic/test_bufferreadwriterect.cpp b/test_conformance/basic/test_bufferreadwriterect.cpp index e2a5315406..5d5fbcbb04 100644 --- a/test_conformance/basic/test_bufferreadwriterect.cpp +++ b/test_conformance/basic/test_bufferreadwriterect.cpp @@ -25,6 +25,7 @@ #include "testBase.h" +#define TEST_READWRITERECT_PRINT_BUFFER 0 #define CL_EXIT_ERROR(cmd,format,...) \ { \ if ((cmd) != CL_SUCCESS) { \ @@ -35,6 +36,8 @@ log_error("\n"); \ } \ } +namespace { + typedef unsigned char BufferType; // Globals for test @@ -47,12 +50,11 @@ size_t height [TotalImages]; size_t depth [TotalImages]; // cl buffer and host buffer. -cl_mem buffer [TotalImages]; -BufferType* verify[TotalImages]; -BufferType* backing[TotalImages]; +clMemWrapper buffer[TotalImages]; +std::vector verify[TotalImages]; // Temporary buffer used for read and write operations. -BufferType* tmp_buffer; +std::vector tmp_buffer; size_t tmp_buffer_size; size_t num_tries = 50; // Number of randomly selected operations to perform. @@ -60,7 +62,7 @@ size_t alloc_scale = 2; // Scale term applied buffer allocation size. MTdata mt; // Initialize a buffer in host memory containing random values of the specified size. -static void initialize_image(BufferType* ptr, size_t w, size_t h, size_t d, MTdata mt) +void initialize_image(BufferType* ptr, size_t w, size_t h, size_t d, MTdata mt) { enum { ElementSize = sizeof(BufferType)/sizeof(unsigned char) }; @@ -72,6 +74,7 @@ static void initialize_image(BufferType* ptr, size_t w, size_t h, size_t d, MTda } } +#if TEST_READWRITERECT_PRINT_BUFFER // This function prints the contents of a buffer to standard error. void print_buffer(BufferType* buf, size_t w, size_t h, size_t d) { log_error("Size = %zux%zux%zu (%zu total)\n", w, h, d, w * h * d); @@ -86,6 +89,7 @@ void print_buffer(BufferType* buf, size_t w, size_t h, size_t d) { log_error("\n"); } } +#endif // Returns true if the two specified regions overlap. bool check_overlap_rect(size_t src_offset[3], size_t dst_offset[3], @@ -206,11 +210,12 @@ int copy_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, // Copy between host buffers. size_t total = sregion[0] * sregion[1] * sregion[2]; - size_t spitch = width[src]; - size_t sslice = width[src]*height[src]; - - size_t dpitch = width[dst]; - size_t dslice = width[dst]*height[dst]; + size_t spitch = (src_row_pitch == 0) ? sregion[0] : src_row_pitch; + size_t sslice = + (src_slice_pitch == 0) ? sregion[1] * spitch : src_slice_pitch; + size_t dpitch = (dst_row_pitch == 0) ? sregion[0] : dst_row_pitch; + size_t dslice = + (dst_slice_pitch == 0) ? sregion[1] * dpitch : dst_slice_pitch; for (size_t i = 0; i != total; ++i) { @@ -263,7 +268,7 @@ int verify_region(BufferType* device, size_t src, size_t soffset[3], size_t sreg "%zu) of region\n", i, sx, sy, sz); log_error("0x%02x != 0x%02x\n", device[d_idx], verify[src][s_idx]); -#if 0 +#if TEST_READWRITERECT_PRINT_BUFFER // Uncomment this section to print buffers. log_error("Device (copy): [%lu]\n",dst); print_buffer(device,width[dst],height[dst],depth[dst]); @@ -280,7 +285,6 @@ int verify_region(BufferType* device, size_t src, size_t soffset[3], size_t sreg return 0; } - // This function invokes ReadBufferRect to read a region from the // specified source buffer into a temporary destination buffer. The // contents of the temporary buffer are then compared to the source @@ -288,7 +292,7 @@ int verify_region(BufferType* device, size_t src, size_t soffset[3], size_t sreg int read_verify_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, size_t doffset[3], size_t dregion[3]) { // Clear the temporary destination host buffer. - memset(tmp_buffer, 0xff, tmp_buffer_size); + memset(tmp_buffer.data(), 0xff, tmp_buffer_size * sizeof(BufferType)); size_t src_slice_pitch = (width[src]*height[src] != 1) ? width[src]*height[src] : 0; size_t dst_slice_pitch = (width[dst]*height[dst] != 1) ? width[dst]*height[dst] : 0; @@ -297,11 +301,12 @@ int read_verify_region(size_t src, size_t soffset[3], size_t sregion[3], size_t CL_EXIT_ERROR(clEnqueueReadBufferRect( gQueue, buffer[src], CL_TRUE, soffset, doffset, sregion, width[src], src_slice_pitch, width[dst], dst_slice_pitch, - tmp_buffer, 0, NULL, NULL), + tmp_buffer.data(), 0, NULL, NULL), "clEnqueueCopyBufferRect failed between %u and %u", (unsigned)src, (unsigned)dst); - return verify_region(tmp_buffer,src,soffset,sregion,dst,doffset); + return verify_region(tmp_buffer.data(), src, soffset, sregion, dst, + doffset); } // This function performs the same verification check as @@ -335,19 +340,20 @@ int map_verify_region(size_t src) { // region of it to a region in the specified destination buffer. int write_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, size_t doffset[3], size_t dregion[3]) { - initialize_image(tmp_buffer, tmp_buffer_size, 1, 1, mt); + initialize_image(tmp_buffer.data(), tmp_buffer_size, 1, 1, mt); // memset(tmp_buffer, 0xf0, tmp_buffer_size); size_t src_slice_pitch = (width[src]*height[src] != 1) ? width[src]*height[src] : 0; size_t dst_slice_pitch = (width[dst]*height[dst] != 1) ? width[dst]*height[dst] : 0; // Copy the source region of the cl buffer, to the destination region of the temporary buffer. - CL_EXIT_ERROR(clEnqueueWriteBufferRect( - gQueue, buffer[dst], CL_TRUE, doffset, soffset, - /*sregion,*/ dregion, width[dst], dst_slice_pitch, - width[src], src_slice_pitch, tmp_buffer, 0, NULL, NULL), - "clEnqueueWriteBufferRect failed between %u and %u", - (unsigned)src, (unsigned)dst); + CL_EXIT_ERROR( + clEnqueueWriteBufferRect(gQueue, buffer[dst], CL_TRUE, doffset, soffset, + /*sregion,*/ dregion, width[dst], + dst_slice_pitch, width[src], src_slice_pitch, + tmp_buffer.data(), 0, NULL, NULL), + "clEnqueueWriteBufferRect failed between %u and %u", (unsigned)src, + (unsigned)dst); // Copy from the temporary buffer to the host buffer. size_t spitch = width[src]; @@ -377,11 +383,6 @@ int write_region(size_t src, size_t soffset[3], size_t sregion[3], size_t dst, s return 0; } -void CL_CALLBACK mem_obj_destructor_callback( cl_mem, void *data ) -{ - free( data ); -} - using test_fn = int (*)(size_t, size_t[3], size_t[3], size_t, size_t[3], size_t[3]); struct TestFunctions @@ -463,37 +464,28 @@ static int test_bufferreadwriterect_impl(cl_device_id device, // Allocate a temporary buffer for read and write operations. tmp_buffer_size = max_size; - tmp_buffer = (BufferType*)malloc(tmp_buffer_size); + tmp_buffer.resize(tmp_buffer_size, 0); // Initialize cl buffers log_info( "Initializing buffers\n" ); for (unsigned i=0; i != TotalImages; ++i) { - size_t size_bytes = width[i]*height[i]*depth[i]*sizeof(BufferType); + size_t num_elems = width[i] * height[i] * depth[i]; + size_t size_bytes = num_elems * sizeof(BufferType); // Allocate a host copy of the buffer for verification. - verify[i] = (BufferType*)malloc(size_bytes); - CL_EXIT_ERROR(verify[i] ? CL_SUCCESS : -1, "malloc of host buffer failed for buffer %u", i); - - // Allocate the buffer in host memory. - backing[i] = (BufferType*)malloc(size_bytes); - CL_EXIT_ERROR(backing[i] ? CL_SUCCESS : -1, "malloc of backing buffer failed for buffer %u", i); + verify[i].resize(num_elems); + CL_EXIT_ERROR(verify[i].data() ? CL_SUCCESS : -1, + "malloc of host buffer failed for buffer %u", i); // Generate a random buffer. log_info( "Initializing buffer %u\n", i ); - initialize_image(verify[i], width[i], height[i], depth[i], mt); - - // Copy the image into a buffer which will passed to CL. - memcpy(backing[i], verify[i], size_bytes); + initialize_image(verify[i].data(), width[i], height[i], depth[i], mt); // Create the CL buffer. - buffer[i] = - clCreateBuffer(context, buffer_flags, size_bytes, backing[i], &err); - CL_EXIT_ERROR(err,"clCreateBuffer failed for buffer %u", i); - - // Make sure buffer is cleaned up appropriately if we encounter an error in the rest of the calls. - err = clSetMemObjectDestructorCallback( buffer[i], mem_obj_destructor_callback, backing[i] ); - CL_EXIT_ERROR(err, "Unable to set mem object destructor callback" ); + buffer[i] = clCreateBuffer(context, buffer_flags, size_bytes, + verify[i].data(), &err); + CL_EXIT_ERROR(err, "clCreateBuffer failed for buffer %u", i); } // Main test loop, run num_tries times. @@ -607,11 +599,6 @@ static int test_bufferreadwriterect_impl(cl_device_id device, // Clean-up. free_mtdata(mt); - for (unsigned i=0;i Date: Thu, 19 Mar 2026 12:04:40 +0100 Subject: [PATCH 4/4] clang format correction --- test_conformance/basic/test_bufferreadwriterect.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test_conformance/basic/test_bufferreadwriterect.cpp b/test_conformance/basic/test_bufferreadwriterect.cpp index 90f896f8d4..82055adfca 100644 --- a/test_conformance/basic/test_bufferreadwriterect.cpp +++ b/test_conformance/basic/test_bufferreadwriterect.cpp @@ -455,12 +455,10 @@ struct TestFunctions test_fn write; }; -int test_bufferreadwriterect_impl(cl_device_id device, - cl_context context, - cl_command_queue queue, - int num_elements, - cl_map_flags buffer_flags, - const TestFunctions& test_functions) +int test_bufferreadwriterect_impl(cl_device_id device, cl_context context, + cl_command_queue queue, int num_elements, + cl_map_flags buffer_flags, + const TestFunctions& test_functions) { gQueue = queue; cl_int err;