On Friday, October 10, 2014 09:14:19 AM Luo, Xionghu wrote:
> Ping for review the v3.
> Thanks.
>
> Luo Xionghu
> Best Regards
Hello
I came up with a different version and was about to post it when I saw yours.
I'll post mine as a reply to get your opinion.
Here is my comments on yours. Please not that I'm not a commiter.
>
> -----Original Message-----
> From: Luo, Xionghu
> Sent: Sunday, September 28, 2014 6:26 AM
> To: piglit@lists.freedesktop.org
> Cc: beig...@lists.freedesktop.org; Luo, Xionghu
> Subject: [PATCH] fix CL_KERNEL_GLOBAL_WORK_SIZE bug.
>
> From: Luo <xionghu....@intel.com>
>
> the option CL_KERNEL_GLOBAL_WORK_SIZE for clGetKernelWorkGroupInfo should
> call built in kernel or custom device according to the spec, this patch
> calls the built in kernel to query the GLOBAL_WORK_SIZE.
>
> v2: use built in kernel to qury the GLOBAL_WORK_SIZE if exist, dummy kernel
> for other options, handle the case when no built in kernel is provided.
>
> v3: fix indent issue; loop CL_KERNEL_GLOBAL_WORK_SIZE out, test it with the
> platform supports opencl-1.2.
>
> Signed-off-by: Luo <xionghu....@intel.com>
> ---
> tests/cl/api/get-kernel-work-group-info.c | 127
> +++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+)
>
> diff --git a/tests/cl/api/get-kernel-work-group-info.c
> b/tests/cl/api/get-kernel-work-group-info.c index 47d09da..f3fd6e5 100644
> --- a/tests/cl/api/get-kernel-work-group-info.c
> +++ b/tests/cl/api/get-kernel-work-group-info.c
> @@ -61,6 +61,11 @@ piglit_cl_test(const int argc,
> int i;
> cl_int errNo;
> cl_kernel kernel;
> +#ifdef CL_VERSION_1_2
> + cl_program built_in_prog = NULL;
> + cl_kernel built_in_kernel = NULL;
> + size_t built_in_kernels_size;
> +#endif
I think it ok you don't #ifdef those
>
> size_t param_value_size;
> void* param_value;
> @@ -84,6 +89,17 @@ piglit_cl_test(const int argc,
> for(i = 0; i < num_kernel_work_group_infos; i++) {
> printf("%s ",
> piglit_cl_get_enum_name(kernel_work_group_infos[i]));
>
> +#ifdef CL_VERSION_1_2
> + if(kernel_work_group_infos[i] == CL_KERNEL_GLOBAL_WORK_SIZE){
> + if(env->version >= 12) {
You could keep this test on a custom device
> + continue;
> + }else{
> + fprintf(stderr, "Could not query
CL_KERNEL_GLOBAL_WORK_SIZE. Piglit was
> compiled against OpenCL version >= 1.2 and cannot run this test for
> versions < 1.2 because CL_KERNEL_GLOBAL_WORK_SIZE option is not
> present.\n"); + piglit_merge_result(&result,
> PIGLIT_FAIL);
> + }
You don't need the else,
PIGLIT_CL_ENUM_NUM(cl_kernel_work_group_info, env->version)
already take care of it
> + }
> +#endif
> +
> errNo = clGetKernelWorkGroupInfo(kernel,
> env->device_id,
> kernel_work_group_infos[i], @@
-187,6
> +203,117 @@ piglit_cl_test(const int argc, piglit_merge_result(&result,
> PIGLIT_FAIL);
> }
>
> +#ifdef CL_VERSION_1_2
> + if(env->version < 12){
> + fprintf(stderr, "Could not query
> CL_KERNEL_GLOBAL_WORK_SIZE.
Piglit
> was compiled against OpenCL version >= 1.2 and cannot run this test for
> versions < 1.2 because CL_KERNEL_GLOBAL_WORK_SIZE option is not
> present.\n"); + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
You shouldn't fail here. The same way
PIGLIT_CL_ENUM_NUM(cl_kernel_work_group_info, env->version)
ignore this flag, skip it and don't tell the user.
It doesn't expect it to be tested on version < 1.2 anyway
> +
> + //use builtin kernel to test CL_KERNEL_GLOBAL_WORK_SIZE.
> + errNo = clGetDeviceInfo(env->device_id, CL_DEVICE_BUILT_IN_KERNELS, 0,
> 0,
> &built_in_kernels_size); + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> + fprintf(stderr,
> + "Failed (error code: %s): Get Device Info.\n",
> + piglit_cl_get_error_name(errNo));
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
> +
> + if(built_in_kernels_size != 0)
> + {
> + char* built_in_kernel_names;
> + char* kernel_name;
> + size_t ret_sz;
> + built_in_kernel_names = (char* )malloc(built_in_kernels_size *
> +sizeof(char) );
> +
> + errNo = clGetDeviceInfo(env->device_id,
> CL_DEVICE_BUILT_IN_KERNELS,
> built_in_kernels_size, (void*)built_in_kernel_names, &ret_sz);
> + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> + fprintf(stderr,
> + "Failed (error code: %s): Get Device Info.\n",
> + piglit_cl_get_error_name(errNo));
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
> +
built_in_kernel_names = piglit_cl_get_device_info(env->device_id,
CL_DEVICE_BUILT_IN_KERNELS);
is shorter
> + built_in_prog = clCreateProgramWithBuiltInKernels(env->context-
>cl_ctx,
> 1, &env->device_id, built_in_kernel_names, &errNo);
> + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> + fprintf(stderr,
> + "Failed (error code: %s): Create BuiltIn Program.\n",
> + piglit_cl_get_error_name(errNo));
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
Would you be better to PIGLIT_WARN and skip test with built-in ?
> +
> + kernel_name = strtok(built_in_kernel_names, ";");
> +
> + built_in_kernel = clCreateKernel(built_in_prog, kernel_name,
&errNo);
> + if(!piglit_cl_check_error(errNo, CL_SUCCESS)) {
> + fprintf(stderr,
> + "Failed (error code: %s): Create kernel.\n",
> + piglit_cl_get_error_name(errNo));
> + piglit_merge_result(&result, PIGLIT_FAIL);
PIGLIT_WARN ?
- EdB
> + }
> + free(built_in_kernel_names);
> + /*
> + * CL_INVALID_VALUE if kernel is not a built in kernel.
> + */
> + errNo = clGetKernelWorkGroupInfo(kernel,
> + env->device_id,
> + CL_KERNEL_GLOBAL_WORK_SIZE,
> + 0,
> + NULL,
> + ¶m_value_size);
> + if(!piglit_cl_check_error(errNo, CL_INVALID_VALUE)) {
> + fprintf(stderr,
> + "Failed (error code: %s): Trigger
> CL_INVALID_VALUE if
kernel is
> not a builtin kernel for CL_KERNEL_GLOBAL_WORK_SIZE.\n", +
>
> piglit_cl_get_error_name(errNo));
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
> +
> + if(built_in_kernel != NULL) {
> + errNo = clGetKernelWorkGroupInfo(built_in_kernel,
> + env->device_id,
> +
> CL_KERNEL_GLOBAL_WORK_SIZE,
> + 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(kernel_work_group_infos[i]));
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
> +
> + param_value = malloc(param_value_size);
> + errNo = clGetKernelWorkGroupInfo(built_in_kernel,
> + env->device_id,
> +
> CL_KERNEL_GLOBAL_WORK_SIZE,
> + param_value_size,
> + param_value,
> + NULL);
> + 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(kernel_work_group_infos[i]));
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
> +
> + //TODO: output returned values
> + printf("\n");
> + free(param_value);
> + }else{
> + fprintf(stderr,
> + "built in kernel create failed.\n");
> + piglit_merge_result(&result, PIGLIT_FAIL);
> + }
> + clReleaseKernel(built_in_kernel);
> + clReleaseProgram(built_in_prog);
> + }else{
> + fprintf(stderr,
> + "no built in kernel provided.\n");
> + piglit_merge_result(&result, PIGLIT_WARN);
> + }
> +#endif
> +
> clReleaseKernel(kernel);
>
> return result;
> --
> 1.7.9.5
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
