[Beignet] [PATCH] Only provide device IDs if the type matches

[Simon Richter]

Hi,

On Fri, Feb 10, 2017 at 06:36:35AM +0000, Pan, Xiuli wrote:

> You point about the device type is right, but the git commit you found is
> not right the function is just copy form somewhere else. And the this
> patch is not follow the OpenCL spec. As the Beignet does not have a CPU
> device, we should just return CL_INVALID_DEVICE_TYPE.

If the device type does not match, CL_DEVICE_NOT_FOUND is correct. The
description of clGetDeviceIDs[1] says

    CL_DEVICE_NOT_FOUND if no OpenCL devices that matched device_type were
    found. 

The function should return CL_INVALID_DEVICE_TYPE if a bit is set that is
not defined in the version of the spec that is implemented, so

    if(device_type & ~(CL_DEVICE_TYPE_DEFAULT|CL_DEVICE_TYPE_CPU|CL_DEVICE_TYPE_GPU|CL_DEVICE_TYPE_ACCELERATOR|CL_DEVICE_TYPE_CUSTOM))
    {
        return CL_INVALID_DEVICE_TYPE;
    }

at the beginning of the function is needed. Otherwise this patch is
correct, but suboptimal as we don't need to search for a device if we're
going to throw away the result anyway.

   Simon

[1] https://www.khronos.org/registry/OpenCL/sdk/2.0/docs/man/xhtml/clGetDeviceIDs.html

> 
> I will send a patch to fix this problem.
> 
> Thanks
> Xiuli
> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Giuseppe Bilotta
> Sent: Friday, February 10, 2017 6:46 AM
> To: Beignet ML <beignet at lists.freedesktop.org>
> Cc: Giuseppe Bilotta <giuseppe.bilotta at gmail.com>
> Subject: [Beignet] [PATCH] Only provide device IDs if the type matches
> 
> If the user is asking for (say) CPU devices only, we should not return a GPU device instead. This fixes a regression introduced with the refactoring done in 494081299ac3f6970b906cb4d7d1eb1fc2fb23c8, by reinstating the check that used to be in cl_check_device_type() (except that instead of hard-coding CL_DEVICE_TYPE_GPU we use the device type reported by the device).
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta at gmail.com>
> ---
>  src/cl_device_id.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cl_device_id.c b/src/cl_device_id.c index b2810635..b8184486 100644
> --- a/src/cl_device_id.c
> +++ b/src/cl_device_id.c
> @@ -880,8 +880,14 @@ cl_get_device_ids(cl_platform_id    platform,
>  
>    openlog("beignet", LOG_CONS, LOG_USER);
>  
> -  /* Do we have a usable device? */
>    device = cl_get_gt_device();
> +
> +  /* Is the device of the correct type? */  if (device &&
> +     ((device->device_type | CL_DEVICE_TYPE_DEFAULT) & device_type) == 0)
> +    device = 0;
> +
> +  /* Do we have a usable device? */
>    if (device) {
>      cl_self_test_res ret = cl_self_test(device, SELF_TEST_PASS);
>      if (ret == SELF_TEST_ATOMIC_FAIL) {
> --
> 2.11.1.658.g6a0cb3eb68
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet