[Piglit] [PATCH] cl: check value returned from clGetMemObjectInfo
Jan Vesely
jan.vesely at rutgers.edu
Wed Sep 9 22:35:43 PDT 2015
On Wed, 2015-09-09 at 21:23 +0200, Serge Martin wrote:
> On Tuesday 08 September 2015 21:07:06 Jan Vesely wrote:
> > On Mon, 2015-09-07 at 00:42 +0200, Serge Martin (EdB) wrote:
> > > From: Serge Martin <edb+piglit at sigluy.net>
> > >
> > > ---
> > >
> > > tests/cl/api/get-mem-object-info.c | 253
> > >
> > > +++++++++++++++++++++++++++++++------
> > >
> > > 1 file changed, 213 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/tests/cl/api/get-mem-object-info.c
> > > b/tests/cl/api/get
> > > -mem-object-info.c
> > > index c24b51d..b68262c 100644
> > > --- a/tests/cl/api/get-mem-object-info.c
> > > +++ b/tests/cl/api/get-mem-object-info.c
> > > @@ -46,6 +46,127 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
> > >
> > > PIGLIT_CL_API_TEST_CONFIG_END
> > >
> > > +#define BUFFER_SIZE 512
> > > +
> > > +static bool
> > > +test(int n,
> > > + cl_mem memobj,
> > > + cl_mem_info param_name,
> > > + cl_mem_object_type mem_type,
> > > + cl_mem_flags mem_flags,
> > > + size_t mem_size,
> > > + void *mem_ptr,
> > > + const struct piglit_cl_api_test_env* env,
> > > + cl_mem mem_parent,
> > > + size_t mem_offset,
> > > + enum piglit_result* result) {
> > > + cl_int errNo;
> > > + size_t param_value_size;
> > > + void* param_value;
> > > +
> > > + errNo = clGetMemObjectInfo(memobj,
> > > + param_name,
> > > + 0,
> > > + NULL,
> > > + ¶m_value_size);
> > > + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> > > + fprintf(stderr,
> > > + "Buffer %d, failed (error code: %s): Get
> > > size of %s.\n",
> > > + n,
> > > + piglit_cl_get_error_name(errNo),
> > > + piglit_cl_get_enum_name(param_name));
> > > + piglit_merge_result(result, PIGLIT_FAIL);
> > > + return false;
> > > + }
> > > +
> > > + param_value = malloc(param_value_size);
> > > + errNo = clGetMemObjectInfo(memobj,
> > > + param_name,
> > > + param_value_size,
> > > + param_value,
> > > + NULL);
> > > + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> > > + fprintf(stderr,
> > > + "Buffer %d, failed (error code: %s): Get
> > > value of %s.\n",
> > > + n,
> > > + piglit_cl_get_error_name(errNo),
> > > + piglit_cl_get_enum_name(param_name));
> > > + piglit_merge_result(result, PIGLIT_FAIL);
> > > + free(param_value);
> > > + return false;
> > > + }
> > > +
> > > +#define CHECK_SIZE(_type_) \
> > > + if (param_value_size != sizeof(_type_)) { \
> > > + fprintf(stderr, \
> > > + "Buffer %d, failed: the returned size
> > > doesn't matches. Expected %lu, got %lu\n", \
> >
> > "doesn't match"
> >
> > > + n, sizeof(_type_), param_value_size); \
> > > + piglit_merge_result(result, PIGLIT_FAIL); \
> > > + }
> > > +
> > > +#define CHECK_VALUE(_type_, _value_) \
> > > + if (*(_type_*)param_value != _value_) { \
> > > + fprintf(stderr, \
> > > + "Buffer %d, failed: the returned value
> > > doesn't matches.\n", \
> >
> > "doesn't match"
>
> sorry about that
I noticed one more thing. CHECK_ macros don't return false on piglit
failure. The return value is not checked, but it's good to be
consistent. Maybe a better way would be to return piglit_result and
merge it in the outer for loop. you save half a dozen lines of code and
one output parameter.
>
> >
> > > + n); \
> > > + piglit_merge_result(result, PIGLIT_FAIL); \
> > > + }
> > > +
> > > + switch (param_name) {
> > > + case CL_MEM_TYPE:
> > > + CHECK_SIZE(cl_mem_object_type)
> > > + CHECK_VALUE(cl_mem_object_type,
> > > mem_type)
> > > + break;
> > > + case CL_MEM_FLAGS:
> > > + CHECK_SIZE(cl_mem_flags)
> > > + CHECK_VALUE(cl_mem_flags, mem_flags)
> > > + break;
> > > + case CL_MEM_SIZE:
> > > + CHECK_SIZE(size_t)
> > > + CHECK_VALUE(size_t, mem_size)
> > > + break;
> > > + case CL_MEM_HOST_PTR:
> > > + CHECK_SIZE(void *)
> > > + CHECK_VALUE(void *, mem_ptr)
> > > + break;
> > > + case CL_MEM_MAP_COUNT:
> > > + CHECK_SIZE(cl_uint)
> > > + //stale
> > > + break;
> > > + case CL_MEM_REFERENCE_COUNT:
> > > + CHECK_SIZE(cl_uint)
> > > + //stale
> > > + break;
> > > + case CL_MEM_CONTEXT:
> > > + CHECK_SIZE(cl_context)
> > > + CHECK_VALUE(cl_context, env->context
> > > ->cl_ctx)
> > > + break;
> > > +#if defined(CL_VERSION_1_1)
> > > + case CL_MEM_ASSOCIATED_MEMOBJECT:
> > > + if (env->version >= 11) {
> > > + CHECK_SIZE(cl_mem)
> > > + CHECK_VALUE(cl_mem, mem_parent)
> > > + }
> > > + break;
> > > + case CL_MEM_OFFSET:
> > > + if (env->version >= 11) {
> > > + CHECK_SIZE(size_t)
> > > + CHECK_VALUE(size_t, mem_offset)
> > > + }
> > > + break;
> > > +#endif
> > > + default:
> > > + fprintf(stderr, "Warn: untested
> > > parameter
> > > %s\n",
> > > +
> > >
> > > piglit_cl_get_enum_name(param_name));
> > >
> > > + piglit_merge_result(result,
> > > PIGLIT_WARN);
> > > + }
> > > +
> > > +#undef CHECK_SIZE
> > > +#undef CHECK_VALUE
> > > +
> > > + free(param_value);
> > > + return true;
> > > +}
> > >
> > > enum piglit_result
> > > piglit_cl_test(const int argc,
> > >
> > > @@ -57,61 +178,108 @@ piglit_cl_test(const int argc,
> > >
> > > int i;
> > > cl_int errNo;
> > >
> > > - cl_mem memobj;
> > > + cl_mem memobj[3];
> > > + char host_mem[BUFFER_SIZE] = {0};
> > >
> > > size_t param_value_size;
> > > void* param_value;
> > >
> > > -
> > > +
> > >
> > > int num_mem_infos = PIGLIT_CL_ENUM_NUM(cl_mem_info, env
> > >
> > > ->version);
> > >
> > > const cl_mem_info* mem_infos =
> > >
> > > PIGLIT_CL_ENUM_ARRAY(cl_mem_info);
> > >
> > > - memobj = clCreateBuffer(env->context->cl_ctx,
> > > - CL_MEM_READ_WRITE,
> > > - 512,
> > > - NULL,
> > > - &errNo);
> > > + memobj[0] = clCreateBuffer(env->context->cl_ctx,
> > > +
> > >
> > > CL_MEM_READ_WRITE|CL_MEM_COPY_HOST_PTR,
> > >
> > > + BUFFER_SIZE,
> > > + host_mem,
> > > + &errNo);
> > >
> > > if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> > >
> > > fprintf(stderr,
> > >
> > > - "Failed (error code: %s): Create an
> > > event by
> > > enqueueing a buffer read.\n",
> > > + "Failed (error code: %s): Create buffer
> > > 0.\n",
> > >
> > > piglit_cl_get_error_name(errNo));
> > >
> > > return PIGLIT_FAIL;
> > >
> > > }
> > >
> > > - /*** Normal usage ***/
> > > - for(i = 0; i < num_mem_infos; i++) {
> > > - printf("%s ",
> > > piglit_cl_get_enum_name(mem_infos[i]));
> > > + memobj[1] = clCreateBuffer(env->context->cl_ctx,
> > > +
> > >
> > > CL_MEM_READ_WRITE|CL_MEM_USE_HOST_PTR,
> > >
> > > + BUFFER_SIZE,
> > > + host_mem,
> > > + &errNo);
> > > + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> > > + fprintf(stderr,
> > > + "Failed (error code: %s): Create buffer
> > > 1.\n",
> > > + piglit_cl_get_error_name(errNo));
> > > + return PIGLIT_FAIL;
> > > + }
> > >
> > > - errNo = clGetMemObjectInfo(memobj,
> > > - mem_infos[i],
> > > - 0,
> > > - NULL,
> > > - ¶m_value_size);
> > > - if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> > > - fprintf(stderr,
> > > - "Failed (error code: %s): Get
> > > size
> > > of %s.\n",
> > > - piglit_cl_get_error_name(errNo),
> > > -
> > >
> > > piglit_cl_get_enum_name(mem_infos[i]));
> > >
> > > - piglit_merge_result(&result,
> > > PIGLIT_FAIL);
> > > - continue;
> > > - }
> > > +#if defined(CL_VERSION_1_1)
> > > + if (env->version >= 11) {
> > > + cl_buffer_region region;
> > > + region.origin = BUFFER_SIZE/2;
> > > + region.size = BUFFER_SIZE/2;
> > >
> > > - param_value = malloc(param_value_size);
> > > - errNo = clGetMemObjectInfo(memobj,
> > > - mem_infos[i],
> > > - param_value_size,
> > > - param_value,
> > > - NULL);
> > > + memobj[2] = clCreateSubBuffer(memobj[1],
> > > + 0,
> > > +
> > >
> > > CL_BUFFER_CREATE_TYPE_REGION,
> > >
> > > + ®ion,
> > > + &errNo);
> > >
> > > if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> > >
> > > fprintf(stderr,
> > >
> > > - "Failed (error code: %s): Get
> > > value
> > > of %s.\n",
> > > - piglit_cl_get_error_name(errNo),
> > > -
> > >
> > > piglit_cl_get_enum_name(mem_infos[i]));
> > >
> > > - piglit_merge_result(&result,
> > > PIGLIT_FAIL);
> > > + "Failed (error code: %s): Create
> > > buffer 2.\n",
> > > +
> > > piglit_cl_get_error_name(errNo));
> > > + return PIGLIT_FAIL;
> > >
> > > }
> > >
> > > + }
> > > +#endif
> > >
> > > - //TODO: output returned values
> > > - printf("\n");
> > > - free(param_value);
> > > + /*** Normal usage ***/
> > > + for(i = 0; i < num_mem_infos; i++) {
> > > + void *copy_ptr = host_mem;
> > > +
> > > +#if defined(CL_VERSION_1_2)
> > > + if (env->version >= 12)
> > > + copy_ptr = NULL;
> > > +#endif
> > > + printf("%s\n",
> > > piglit_cl_get_enum_name(mem_infos[i]));
> > > +
> > > + test(0,
> > > + memobj[0],
> > > + mem_infos[i],
> > > + CL_MEM_OBJECT_BUFFER,
> > > + CL_MEM_READ_WRITE|CL_MEM_COPY_HOST_PTR,
> > > + BUFFER_SIZE,
> > > + copy_ptr,
> > > + env,
> > > + NULL,
> > > + 0,
> > > + &result);
> > > +
> > > + test(1,
> > > + memobj[1],
> > > + mem_infos[i],
> > > + CL_MEM_OBJECT_BUFFER,
> > > + CL_MEM_READ_WRITE|CL_MEM_USE_HOST_PTR,
> > > + BUFFER_SIZE,
> > > + host_mem,
> > > + env,
> > > + NULL,
> > > + 0,
> > > + &result);
> > > +
> > > +#if defined(CL_VERSION_1_1)
> > > + if (env->version >= 11) {
> > > + test(2,
> > > + memobj[2],
> > > + mem_infos[i],
> > > + CL_MEM_OBJECT_BUFFER,
> > > +
> > > CL_MEM_READ_WRITE|CL_MEM_USE_HOST_PTR,
> > > + BUFFER_SIZE/2,
> > > + host_mem + BUFFER_SIZE/2,
> > > + env,
> > > + memobj[1],
> > > + BUFFER_SIZE/2,
> > > + &result);
> > > + }
> > > +#endif
> > >
> > > }
> > >
> > > /*** Errors ***/
> > >
> > > @@ -122,7 +290,7 @@ piglit_cl_test(const int argc,
> > >
> > > * less than size of return type and param_value is not
> > > a
> > >
> > > NULL
> > >
> > > * value.
> > > */
> > >
> > > - errNo = clGetMemObjectInfo(memobj,
> > > + errNo = clGetMemObjectInfo(memobj[0],
> > >
> > > CL_DEVICE_NAME,
> > > 0,
> > > NULL,
> > >
> > > @@ -134,11 +302,13 @@ piglit_cl_test(const int argc,
> > >
> > > piglit_merge_result(&result, PIGLIT_FAIL);
> > >
> > > }
> > >
> > > - errNo = clGetMemObjectInfo(memobj,
> > > + param_value = malloc(sizeof(cl_mem_object_type));
> > > + errNo = clGetMemObjectInfo(memobj[0],
> > >
> > > CL_MEM_TYPE,
> > > 1,
> > > param_value,
> > > NULL);
> > >
> > > + free(param_value);
> > >
> > > if(!piglit_cl_check_error(errNo, CL_INVALID_VALUE)) {
> > >
> > > fprintf(stderr,
> > >
> > > "Failed (error code: %s): Trigger
> > >
> > > CL_INVALID_VALUE if size in bytes specified by param_value is
> > > less
> > > than size of return type and param_value is not a NULL value.\n",
> > > @@ -161,7 +331,10 @@ piglit_cl_test(const int argc,
> > >
> > > piglit_merge_result(&result, PIGLIT_FAIL);
> > >
> > > }
> > >
> > > - clReleaseMemObject(memobj);
> > > + clReleaseMemObject(memobj[0]);
> > > + clReleaseMemObject(memobj[1]);
> > > + if (env->version >= 11)
> > > + clReleaseMemObject(memobj[2]);
> >
> > I think this should be also ifdef-ed. if you build on 1.0 but run
> > on
> > 1.1 it will crash.
>
> Indeed I didn't think of this case.
> >
> > > return result;
> > >
> > > }
> >
> > with the above changes
> > Reviewed-by: Jan Vesely <jan.vesely at rutgers.edu>
> >
> > just FYI: passes on nVidia CUDA, fails on intel (CL_MEM_HOST_PTR
> > returns host_ptr even for COPY_HOST_PTR created buffer).
>
> I'm no surprised, CL 1.2 change the meanning of the query.
> Clover would have the same bug if we were exposing 1.2 now
>
> >
> > have you considered using subtests for individual queries?
>
> I didn't think about that, I didn't know that it exists
AFAICT only one cl piglit test other than program-tester (cl-buffer
-flags) uses subtests. It only occurred to me since you are printing
the enum name.
Jan
>
> Serge
>
> >
> > Jan
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150910/0f2f4349/attachment.sig>
More information about the Piglit
mailing list