Very thanks for your reviewing:)
The v4 will fix the code format issues.
-----Original Message-----
From: EdB [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 10:19 PM
To: [email protected]
Cc: Meng, Mengmeng
Subject: Re: [Piglit] [PATCH v3] cl: add clEnqueue-read_write BufferRect
releated tests
Hello
Thanks for the patch, some comments bellow.
On Friday 10 July 2015 20:17:52 Meng Mengmeng wrote:
> It's a simple function test for clEnqueueReadBufferRect and
> clEnqueueWriteBufferRect. ---
> tests/cl.py | 2 +
> tests/cl/api/CMakeLists.cl.txt | 1 +
> tests/cl/api/enqueue-read_write-buffer-rect.c | 330
> ++++++++++++++++++++++++++ 3 files changed, 333 insertions(+)
> create mode 100644 tests/cl/api/enqueue-read_write-buffer-rect.c
>
> diff --git a/tests/cl.py b/tests/cl.py index c55d3dd..7fce00a 100644
> --- a/tests/cl.py
> +++ b/tests/cl.py
> @@ -58,6 +58,8 @@ with profile.group_manager(PiglitCLTest, 'api') as g:
> g(['cl-api-enqueue-copy-buffer-rect'], 'clEnqueueCopyBufferRect')
> g(['cl-api-enqueue-read_write-buffer'],
> 'clEnqueueReadBuffer and clEnqueueWriteBuffer')
> + g(['cl-api-enqueue-read_write-buffer-rect'],
> + 'clEnqueueReadBufferRect and clEnqueueWriteBufferRect')
> g(['cl-api-get-mem-object-info'], 'clGetMemObjectInfo')
> g(['cl-api-get-image-info'], 'clGetImageInfo')
> g(['cl-api-retain_release-mem-object'],
> diff --git a/tests/cl/api/CMakeLists.cl.txt
> b/tests/cl/api/CMakeLists.cl.txt index 7e78491..e1a25fa 100644
> --- a/tests/cl/api/CMakeLists.cl.txt
> +++ b/tests/cl/api/CMakeLists.cl.txt
> @@ -21,6 +21,7 @@ piglit_cl_add_api_test (enqueue-copy-buffer
> enqueue-copy-buffer.c) piglit_cl_add_api_test (enqueue-map-buffer
> enqueue-map-buffer.c)
> piglit_cl_add_api_test (enqueue-copy-buffer-rect
> enqueue-copy-buffer-rect.c) piglit_cl_add_api_test
> (enqueue-read_write-buffer enqueue-read_write-buffer.c)
> +piglit_cl_add_api_test (enqueue-read_write-buffer-rect
> enqueue-read_write-buffer-rect.c) piglit_cl_add_api_test
> (retain_release-mem-object retain_release-mem-object.c)
> piglit_cl_add_api_test (get-mem-object-info get-mem-object-info.c)
> piglit_cl_add_api_test (get-image-info get-image-info.c) diff --git
> a/tests/cl/api/enqueue-read_write-buffer-rect.c
> b/tests/cl/api/enqueue-read_write-buffer-rect.c new file mode 100644
> index 0000000..c91e930
> --- /dev/null
> +++ b/tests/cl/api/enqueue-read_write-buffer-rect.c
> @@ -0,0 +1,330 @@
> +/*
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> "Software"), + * to deal in the Software without restriction,
> including without limitation + * the rights to use, copy, modify,
> merge, publish, distribute, sublicense, + * and/or sell copies of the
> Software, and to permit persons to whom the + * Software is furnished
> to do so, subject to the following conditions: + *
> + * The above copyright notice and this permission notice (including
> + the
> next + * paragraph) shall be included in all copies or substantial
> portions of the + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + EXPRESS
> OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR
> ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF
> CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION
> WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE.
> + *
> + * Authors: Meng Mengmeng <mengmeng.meng at intel.com>
> + *
> + */
> +
> +#include "piglit-framework-cl-api.h"
> +#include "piglit-util-cl.h"
> +
> +
> +PIGLIT_CL_API_TEST_CONFIG_BEGIN
> +
> + config.name = "clEnqueueReadBufferRect and clEnqueueWriteBufferRect";
> + config.version_min = 11;
> +
> + config.run_per_platform = true;
> + config.create_context = true;
> +
> +PIGLIT_CL_API_TEST_CONFIG_END
> +
> +static bool
> +test_read_rect(cl_command_queue command_queue,
> + cl_mem buffer,
> + cl_bool blocking_read,
> + const size_t * buffer_origin,
> + const size_t * host_origin,
> + const size_t * region,
> + size_t buffer_row_pitch,
> + size_t buffer_slice_pitch,
> + size_t host_row_pitch,
> + size_t host_slice_pitch,
> + void *ptr,
> + cl_uint num_events_in_wait_list,
> + const cl_event *event_wait_list,
> + cl_event *event,
> + cl_int expected_error,
> + enum piglit_result* result,
> + const char* test_str) {
> + cl_int errNo;
> +
> + errNo = clEnqueueReadBufferRect(command_queue,
> + buffer,
> + blocking_read,
> + buffer_origin,
> + host_origin,
> + region,
> + buffer_row_pitch,
> + buffer_slice_pitch,
> + host_row_pitch,
> + host_slice_pitch,
> + ptr,
> + num_events_in_wait_list,
> + event_wait_list,
> + event);
use one tab and then use space to align args.
> +
> + if(!piglit_cl_check_error(errNo, expected_error)) {
> + fprintf(stderr, "clEnqueueReadBufferRect: Failed (error code:
> %s):
> %s.\n",
> + piglit_cl_get_error_name(errNo), test_str);
2 tabs and then use space to align args.
and so on after that
> + piglit_merge_result(result, PIGLIT_FAIL);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool
> +test_wirte_rect(cl_command_queue command_queue,
s/test_wirte_rect/test_write_rect
> + cl_mem buffer,
> + cl_bool blocking_write,
> + const size_t * buffer_origin,
> + const size_t * host_origin,
> + const size_t * region,
> + size_t buffer_row_pitch,
> + size_t buffer_slice_pitch,
> + size_t host_row_pitch,
> + size_t host_slice_pitch,
> + void *ptr,
> + cl_uint num_events_in_wait_list,
> + const cl_event *event_wait_list,
> + cl_event *event,
> + cl_int expected_error,
> + enum piglit_result* result,
> + const char* test_str) {
> + cl_int errNo;
> +
> + errNo = clEnqueueWriteBufferRect(command_queue,
> + buffer,
> + blocking_write,
> + buffer_origin,
> + host_origin,
> + region,
> + buffer_row_pitch,
> + buffer_slice_pitch,
> + host_row_pitch,
> + host_slice_pitch,
> + ptr,
> + num_events_in_wait_list,
> + event_wait_list,
> + event);
> +
> + if(!piglit_cl_check_error(errNo, expected_error)) {
> + fprintf(stderr, "clEnqueueWiteBufferRect: Failed (error code:
> %s):
> %s.\n", + piglit_cl_get_error_name(errNo), test_str);
> + piglit_merge_result(result, PIGLIT_FAIL);
> + return false;
> + }
> +
> + return true;
> +}
> +enum piglit_result
> +piglit_cl_test(const int argc,
> + const char **argv,
> + const struct piglit_cl_api_test_config* config,
> + const struct piglit_cl_api_test_env* env)
> +{
> +
> + enum piglit_result result = PIGLIT_PASS;
> + piglit_cl_context context;
> +
> + cl_event valid_event;
> + cl_event invalid_event = NULL;
> +
> + cl_int i, j;
> + cl_int k = 0;
> + float full_matrix[81], result_matrix[9];
> + const size_t buffer_origin[3] = {3*sizeof(float), 3, 0};
spaces between * : 3 * sizeof(float)
> + const size_t host_origin[3] = {3*sizeof(float), 3, 0};
> + const size_t host_new_origin[3] = {0*sizeof(float), 0, 0};
> + const size_t region[3] = {3*sizeof(float), 3, 1};
> + cl_mem matrix_buffer;
> +
> + for(i=0; i<81; i++) {
> + full_matrix[i] = i*1.0f;
> + }
> + for(i=0; i<9; i++) {
> + result_matrix[i] = 0.0f;
> + }
> +
> + float reference_matrix[9]={30.0f, 31.0f, 32.0f, 39.0f, 40.0f, 41.0f,
> 48.0f, 49.0f, 50.0f}; +
78 characters limit
However it is ok to go after this limits for long strings
> + matrix_buffer = piglit_cl_create_buffer(env->context,
> CL_MEM_READ_WRITE ,
> sizeof(full_matrix)); +
> + /*
> + * CL_INVALID_COMMAND_QUEUE if command_queue is not a valid
> command-queue.
> + */
> + test_wirte_rect(NULL, matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, NULL,
> + CL_INVALID_COMMAND_QUEUE, &result,
> + "Trigger CL_INVALID_COMMAND_QUEUE when command_queue is
> not a
valid
> command-queue"); + test_read_rect(NULL, matrix_buffer, CL_TRUE,
> + buffer_origin, host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, NULL, NULL,
> + CL_INVALID_COMMAND_QUEUE, &result,
> + "Trigger CL_INVALID_COMMAND_QUEUE when command_queue is
> not a
valid
> command-queue"); +
> + /*
> + * CL_INVALID_CONTEXT if the context associated with command_queue
> + * and buffer are not the same or if the context associated with
> command_queue + * and events in event_wait_list are not the same.
> + */
> + context = piglit_cl_create_context(env->platform_id,
> + env->context->device_ids,
> + 1);
> + if(context != NULL){
> + test_wirte_rect(context->command_queues[0], matrix_buffer,
> CL_TRUE,
> + buffer_origin, host_origin, region,
> 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, NULL,
> + CL_INVALID_CONTEXT, &result,
> + "Trigger CL_INVALID_CONTEXT when context
> associated with
command_queue
> and buffer are not the same");
> + test_read_rect(context->command_queues[0], matrix_buffer,
> CL_TRUE,
> + buffer_origin, host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, NULL,
> NULL,
> + CL_INVALID_CONTEXT, &result,
> + "Trigger CL_INVALID_CONTEXT when context
> associated with
command_queue
> and buffer are not the same"); + }else {
> + fprintf(stderr, "Could not test triggering
> CL_INVALID_CONTEXT.\n");
> + piglit_merge_result(&result, PIGLIT_FAIL);
may be we should just PIGLIT_WARN here?
> + }
> +
> + /*
> + * CL_INVALID_MEM_OBJECT if buffer is not a valid buffer object.
> + */
> + test_wirte_rect(env->context->command_queues[0], NULL, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, NULL,
> + CL_INVALID_MEM_OBJECT, &result,
> + "Trigger CL_INVALID_MEM_OBJECT when buffer is not a
> valid buffer
> object"); + test_read_rect(env->context->command_queues[0], NULL, CL_TRUE,
> + buffer_origin, host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, NULL, NULL,
> + CL_INVALID_MEM_OBJECT, &result,
> + "Trigger CL_INVALID_MEM_OBJECT when buffer is not a
> valid buffer
> object"); +
> + /*
> + * CL_INVALID_VALUE if the region being read specified by (buffer_origin,
> region) is out of bounds or if ptr is a NULL value. + */
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin + 1, host_origin, region,
> 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, NULL,
> + CL_INVALID_VALUE, &result,
> + "Trigger CL_INVALID_VALUE if the region being write
> specified by
> (buffer_origin, region) is out of bounds");
> + test_read_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin + 1, host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, NULL, NULL,
> + CL_INVALID_VALUE, &result,
> + "Trigger CL_INVALID_VALUE if the region being read
> specified by
> (buffer_origin, region) is out of bounds");
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region + 1,
> 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, NULL,
> + CL_INVALID_VALUE, &result,
> + "Trigger CL_INVALID_VALUE if the region being write
> specified by
> (buffer_origin, region) is out of bounds");
> + test_read_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin , host_new_origin, region + 1,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, NULL, NULL,
> + CL_INVALID_VALUE, &result,
> + "Trigger CL_INVALID_VALUE if the region being read
> specified by
> (buffer_origin, region) is out of bounds");
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region , 9*sizeof(float), 0,
> + 9*sizeof(float), 0, NULL, 0, NULL, NULL,
> + CL_INVALID_VALUE, &result,
> + "Trigger CL_INVALID_VALUE if ptr is a NULL value");
> + test_read_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin , host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, NULL, 0, NULL, NULL,
> + CL_INVALID_VALUE, &result,
> + "Trigger CL_INVALID_VALUE if ptr is a NULL value");
> +
> + /*
> + * CL_INVALID_EVENT_WAIT_LIST if event_wait_list is NULL and
> num_events_in_wait_list greater than 0, + * or event_wait_list is not NULL
> and num_events_in_wait_list is 0, or if event objects in event_wait_list
> are not valid events. + */
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, &valid_event,
> + CL_SUCCESS, &result,
> + "Create an event");
> +
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 1, NULL, NULL,
> + CL_INVALID_EVENT_WAIT_LIST, &result,
> + "Trigger CL_INVALID_EVENT_WAIT_LIST when
> event_wait_list is NULL
and
> num_events_in_wait_list greater than 0");
> + test_read_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin , host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 1, NULL, NULL,
> + CL_INVALID_EVENT_WAIT_LIST, &result,
> + "Trigger CL_INVALID_EVENT_WAIT_LIST when
> event_wait_list is NULL
and
> num_events_in_wait_list greater than 0");
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, &valid_event, NULL,
> + CL_INVALID_EVENT_WAIT_LIST, &result,
> + "Trigger CL_INVALID_EVENT_WAIT_LIST when vent_wait_list
> is not
NULL and
> num_events_in_wait_list is 0");
> + test_read_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin , host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, &valid_event,
> NULL,
> + CL_INVALID_EVENT_WAIT_LIST, &result,
> + "Trigger CL_INVALID_EVENT_WAIT_LIST when vent_wait_list
> is not
NULL and
> num_events_in_wait_list is 0");
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 1, &invalid_event,
> NULL,
> + CL_INVALID_EVENT_WAIT_LIST, &result,
> + "Trigger CL_INVALID_EVENT_WAIT_LIST when event objects
> in
> event_wait_list are not valid events");
> + test_read_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin , host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, &invalid_event,
> NULL,
> + CL_INVALID_EVENT_WAIT_LIST, &result,
> + "Trigger CL_INVALID_EVENT_WAIT_LIST when event objects
> in
> event_wait_list are not valid event"); +
> + clReleaseEvent(valid_event);
> +
> + /*
> + * CL_MISALIGNED_SUB_BUFFER_OFFSET if buffer is a sub-buffer object and
> offset specified + * when the sub-buffer object is created is not aligned
> to CL_DEVICE_MEM_BASE_ADDR_ALIGN + * value for device associated with
> queue.
> + */
> +
> + /*
> + * CL_EXEC_STATUS_ERROR_FOR_EVENTS_IN_WAIT_LIST if the read and write
> operations are blocking + * and the execution status of any of the events
> in event_wait_list is a negative integer value. + */
> +
> + /*
> + * Write to a buffer object from host memory(9*9 matrix), then read
> rectangular the region 3*3 + * from that buffer object to host memory,
> finally compare the read result with reference matrix. + */
there's trailing spaces here.
usually the correct behavior is tested at the begin followed by test that
check error cases.
> + test_wirte_rect(env->context->command_queues[0], matrix_buffer, CL_TRUE,
> + buffer_origin, host_origin, region, 9*sizeof(float), 0,
> + 9*sizeof(float), 0, full_matrix, 0, NULL, NULL,
> + CL_SUCCESS, &result,
> + "clEnqueueWriteBufferRec: Failed when write to a buffer
> object 9*9
from
> host memor"); + test_read_rect(env->context->command_queues[0],
> matrix_buffer, CL_TRUE, + buffer_origin,
> host_new_origin, region,
> 9*sizeof(float), 0,
> + 3*sizeof(float), 0, result_matrix, 0, NULL, NULL,
> + CL_SUCCESS, &result,
> + "clEnqueueReadBufferRect: Failed when read from a
> rectangular
region 3*3
> to host memory"); +
> + for (i = 0; i < 3; ++i){
> + for (j = 0; j < 3; ++j){
> + if (result_matrix[j+i*3] != reference_matrix[k]){
> + fprintf(stderr,
> + "clEnqueueWriteBufferRect and
> clEnqueueReadBufferRect:
Failed
> (expected %f, but got %f)\n",
> +
> reference_matrix[k],result_matrix[j+i*3]);
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }else
> + k++;
> + }
> + }
> + return result;
> +}
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit
