On Tuesday 12 August 2014 17:25:37 Jan Vesely wrote:
> On Sun, 2014-08-10 at 16:46 +0200, EdB wrote:
> > From: EdB <[email protected]>
> > 
> > If there is only one device associed with the kernel and device arg is
> > NULL
> > you don't have to trigger CL_INVALID_DEVICE
> > ---
> > 
> >  tests/cl/api/get-kernel-work-group-info.c | 31
> >  +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12
> >  deletions(-)
> > 
> > diff --git a/tests/cl/api/get-kernel-work-group-info.c
> > b/tests/cl/api/get-kernel-work-group-info.c index 47d09da..bf0bb29 100644
> > --- a/tests/cl/api/get-kernel-work-group-info.c
> > +++ b/tests/cl/api/get-kernel-work-group-info.c
> > @@ -61,6 +61,7 @@ piglit_cl_test(const int argc,
> > 
> >     int i;
> >     cl_int errNo;
> >     cl_kernel kernel;
> > 
> > +   cl_uint* dev_count_ptr;
> > 
> >     size_t param_value_size;
> >     void* param_value;
> > 
> > @@ -168,24 +169,30 @@ piglit_cl_test(const int argc,
> > 
> >                     piglit_cl_get_error_name(errNo));
> >             
> >             piglit_merge_result(&result, PIGLIT_FAIL);
> >     
> >     }
> > 
> > -
> > +
> > 
> >     /*
> >     
> >      * CL_INVALID_DEVICE if device is not in the list of devices 
associated
> >      with
> >      * kernel or if device is NULL but there is more than one device
> >      associated
> >      * with kernel.
> >      */
> > 
> > -   errNo = clGetKernelWorkGroupInfo(kernel,
> > -                                    NULL,
> > -                                    CL_KERNEL_WORK_GROUP_SIZE,
> > -                                    0,
> > -                                    NULL,
> > -                                    &param_value_size);
> > -   if(!piglit_cl_check_error(errNo, CL_INVALID_DEVICE)) {
> > -           fprintf(stderr,
> > -                   "Failed (error code: %s): Trigger CL_INVALID_DEVICE if 
device
> > is NULL but there is more than one device associated with kernel.\n", -     
> >         
> >        piglit_cl_get_error_name(errNo));
> > -           piglit_merge_result(&result, PIGLIT_FAIL);
> > +   dev_count_ptr = piglit_cl_get_program_info(env->program,
> > CL_PROGRAM_NUM_DEVICES); +  if (*dev_count_ptr != 1) {
> > +           errNo = clGetKernelWorkGroupInfo(kernel,
> > +                                          NULL,
> > +                                          CL_KERNEL_WORK_GROUP_SIZE,
> > +                                          0,
> > +                                          NULL,
> > +                                          &param_value_size);
> > +           if(!piglit_cl_check_error(errNo, CL_INVALID_DEVICE)) {
> > +                   if (*dev_count_ptr != 1) {
> 
> What is the purpose of testing (*dev_count_ptr != 1) again? It's already
> in one big !=1 block.

It was the first attempt to fix the test, I forgot to remove it

> 
> Moreover, can the number of devices ever be != 1 since the test is
> marked run_per_device?
> IMO this change effectively disables the subtest

Indeed. I was thinking that even if the test is per device, the program is 
build only once (if not, why this test).

But you're right, piglit_cl_api_test_t rebuild the program per device
and this should always return CL_SUCCES

Thank for the review

> 
> jan
> 
> > +                           fprintf(stderr,
> > +                                   "Failed (error code: %s): Trigger 
CL_INVALID_DEVICE if device
> > is NULL but there is more than one device associated with kernel.\n",
> > +                                   piglit_cl_get_error_name(errNo));
> > +                           piglit_merge_result(&result, PIGLIT_FAIL);
> > +                   }
> > +           }
> > 
> >     }
> > 
> > +   free(dev_count_ptr);
> > 
> >     clReleaseKernel(kernel);
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to