Re: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to _cl_icd_dispatch table

2016-10-19 Thread Yang, Rong R 
> If you want the warning to go away, you'd need to update Beignet's headers
> to 2.0+ (we already do that in Debian (for different reasons), it doesn't
> change the 1.2 version Beignet reports via clGetDeviceInfo)
> *and* use a stricter check (OCL_ICD_IDENTIFIED_FUNCTIONS > 120), or
> suppress it via compiler option (though the latter might also suppress the
> warning if you accidentally put a function in the wrong slot - I haven't 
> tried).

Thanks for your suggestion. I am rebasing the OCL20 branch to master, after 
finish,
Beignet's header will update to 2.0.

For this patch, actually the function pointer clGetKernelSubGroupInfoKHR is 
converted
to other type (ENTRY cl_int (CL_API_CALL*)(void)) implicit, so I convert it  to 
(void *) explicit to avoid
the warning. It will be refined when rebase OCL20 branch.

Will push it later, thanks.

YangRong
___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet

Re: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to _cl_icd_dispatch table

2016-10-11 Thread Rebecca N. Palmer 

Now I see that the problem is the API cannot be used through ICD, but in 
OpenCL1.2 it is a vender extension.
Beignet may consider it a vendor extension, but ocl-icd doesn't: 
anything with a KHR or EXT name goes through the "check the dispatch 
table first" lookup process, whether or not it exists in the version of 
OpenCL the ICD says it supports.



Should we use the ICD OpenCL2.0 API table of it?

If you mean my patch, yes.


Or maybe we should not use clGetExtensionFunctionForPlatform or 
clGetExtensionFunction to get the API clGetKernelSubGroupInfoKHR?
The alternative of just calling clGetKernelSubGroupInfoKHR without such 
a lookup also goes through the address table when used via ICD, so it 
will have the same problem...plus the additional problem of being a 
compile-time error on older versions of ocl-icd that don't know about 
this function.  (If you want to see the code that does this, build 
ocl-icd then look in ocl_icd_loader_gen.c)


If you want the warning to go away, you'd need to update Beignet's 
headers to 2.0+ (we already do that in Debian (for different reasons), 
it doesn't change the 1.2 version Beignet reports via clGetDeviceInfo) 
*and* use a stricter check (OCL_ICD_IDENTIFIED_FUNCTIONS > 120), or 
suppress it via compiler option (though the latter might also suppress 
the warning if you accidentally put a function in the wrong slot - I 
haven't tried).


___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet

Re: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to _cl_icd_dispatch table

2016-10-10 Thread Pan, Xiuli 
Hi, 
The api clGetKernelSubGroupInfoKHR for OpenCL 1.2 is added for the extension  
cl_intel_subgroups.
https://www.khronos.org/registry/cl/extensions/intel/cl_intel_subgroups.txt

And I add it in 
https://cgit.freedesktop.org/beignet/commit/?id=aa3ee67c825fbef5f68b31345c060c981bf35ad3

Now I see that the problem is the API cannot be used through ICD, but in 
OpenCL1.2 it is a vender extension. Should we use the ICD OpenCL2.0 API table 
of it?
Or maybe we should not use clGetExtensionFunctionForPlatform or 
clGetExtensionFunction to get the API clGetKernelSubGroupInfoKHR?

Thanks
Xiuli


-Original Message-
From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of 
Rebecca N. Palmer
Sent: Tuesday, October 11, 2016 7:07 AM
To: Yang, Rong R <rong.r.y...@intel.com>; beignet@lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to 
_cl_icd_dispatch table

> This patch causes a warning:
> /home/yr /beignet/src/cl_khr_icd.c:187:3: warning: initialization from 
> incompatible pointer type
>clGetKernelSubGroupInfoKHR,
>
> /home/yr/maintain/beignet/src/cl_khr_icd.c:187:3: warning: (near 
> initialization for ‘cl_khr_icd_dispatch.clUnknown136’)

I see that as well, but it does work (with both old and new ocl-icd).

> clGetKernelSubGroupInfoKHR is available from OpenCL2.0, also need OpenCL 
> version check in beignet?
>
> BTW: ocl-icd 2.2.8's OCL_ICD_IDENTIFIED_FUNCTIONS is 128, is it a typo?

#ifdef CL_VERSION_* checks in current Beignet always give 1.2, the version of 
Beignet's copy of CL/cl.h, but what we actually care about here is the version 
ocl-icd (the ICD loader) was built with: the #if (OCL_ICD_IDENTIFIED_FUNCTIONS 
> 110) is an "OpenCL >=2.0" check for that.  (I got the idea from pocl,
https://anonscm.debian.org/cgit/collab-maint/pocl.git/tree/lib/CL/clGetPlatformIDs.c)

This matters because the ICD loader has a table of function names 
(function_description), while the ICD has a table of function addresses
(_cl_icd_dispatch) **but no way to tell the loader how long this table is**, 
and the ICD loader's clGetExtensionFunctionForPlatform does "search the name 
table for the requested name, if found return that slot in the address table 
(even if that's NULL or off-the-end garbage), if not found in the name table 
ask the ICD's
clGetExtensionFunctionForPlatform":
https://www.khronos.org/registry/cl/sdk/2.1/docs/man/xhtml/clGetExtensionFunctionAddressForPlatform.html
https://forge.imag.fr/plugins/scmgit/cgi-bin/gitweb.cgi?p=ocl-icd/ocl-icd.git;a=blob;f=icd_generator.rb;h=d299dbd252ac4b25e62d2e44c0c40dcd179ca3f6;hb=HEAD#l576

Hence, functions that are in the ICD loader's name table but not the ICD's 
address table are impossible to use via ICD, and trying to do so may even crash.

The reverse, i.e. having functions in the address table but not the name table, 
is harmless as long as you don't overflow the table's total length (now 
OCL_ICD_IDENTIFIED_FUNCTIONS+50 but was +20 pre-2.0, 
https://forge.imag.fr/plugins/scmgit/cgi-bin/gitweb.cgi?p=ocl-icd/ocl-icd.git;a=blob;f=icd_generator.rb;h=d299dbd252ac4b25e62d2e44c0c40dcd179ca3f6;hb=HEAD#l72);
hence my use of one #if (OCL_ICD_IDENTIFIED_FUNCTIONS > 110) for the whole 
OpenCL 2.0 block, rather than separate ones for 2.0-core (positions 123-135, 
not currently implemented in Beignet,
OCL_ICD_IDENTIFIED_FUNCTIONS>110) and clGetKernelSubGroupInfoKHR (position 136, 
>120).

___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet
___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet

Re: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to _cl_icd_dispatch table

2016-10-10 Thread Rebecca N. Palmer 

This patch causes a warning:
/home/yr /beignet/src/cl_khr_icd.c:187:3: warning: initialization from 
incompatible pointer type
   clGetKernelSubGroupInfoKHR,

/home/yr/maintain/beignet/src/cl_khr_icd.c:187:3: warning: (near initialization 
for ‘cl_khr_icd_dispatch.clUnknown136’)


I see that as well, but it does work (with both old and new ocl-icd).


clGetKernelSubGroupInfoKHR is available from OpenCL2.0, also need OpenCL 
version check in beignet?

BTW: ocl-icd 2.2.8's OCL_ICD_IDENTIFIED_FUNCTIONS is 128, is it a typo?


#ifdef CL_VERSION_* checks in current Beignet always give 1.2, the 
version of Beignet's copy of CL/cl.h, but what we actually care about 
here is the version ocl-icd (the ICD loader) was built with: the #if 
(OCL_ICD_IDENTIFIED_FUNCTIONS > 110) is an "OpenCL >=2.0" check for 
that.  (I got the idea from pocl, 
https://anonscm.debian.org/cgit/collab-maint/pocl.git/tree/lib/CL/clGetPlatformIDs.c)


This matters because the ICD loader has a table of function names 
(function_description), while the ICD has a table of function addresses 
(_cl_icd_dispatch) **but no way to tell the loader how long this table 
is**, and the ICD loader's clGetExtensionFunctionForPlatform does 
"search the name table for the requested name, if found return that slot 
in the address table (even if that's NULL or off-the-end garbage), if 
not found in the name table ask the ICD's 
clGetExtensionFunctionForPlatform":

https://www.khronos.org/registry/cl/sdk/2.1/docs/man/xhtml/clGetExtensionFunctionAddressForPlatform.html
https://forge.imag.fr/plugins/scmgit/cgi-bin/gitweb.cgi?p=ocl-icd/ocl-icd.git;a=blob;f=icd_generator.rb;h=d299dbd252ac4b25e62d2e44c0c40dcd179ca3f6;hb=HEAD#l576

Hence, functions that are in the ICD loader's name table but not the 
ICD's address table are impossible to use via ICD, and trying to do so 
may even crash.


The reverse, i.e. having functions in the address table but not the name 
table, is harmless as long as you don't overflow the table's total 
length (now OCL_ICD_IDENTIFIED_FUNCTIONS+50 but was +20 pre-2.0, 
https://forge.imag.fr/plugins/scmgit/cgi-bin/gitweb.cgi?p=ocl-icd/ocl-icd.git;a=blob;f=icd_generator.rb;h=d299dbd252ac4b25e62d2e44c0c40dcd179ca3f6;hb=HEAD#l72); 
hence my use of one #if (OCL_ICD_IDENTIFIED_FUNCTIONS > 110) for the 
whole OpenCL 2.0 block, rather than separate ones for 2.0-core 
(positions 123-135, not currently implemented in Beignet, 
OCL_ICD_IDENTIFIED_FUNCTIONS>110) and clGetKernelSubGroupInfoKHR 
(position 136, >120).


___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet

Re: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to _cl_icd_dispatch table

2016-10-10 Thread Yang, Rong R 
This patch causes a warning:
/home/yr /beignet/src/cl_khr_icd.c:187:3: warning: initialization from 
incompatible pointer type
   clGetKernelSubGroupInfoKHR,

/home/yr/maintain/beignet/src/cl_khr_icd.c:187:3: warning: (near initialization 
for ‘cl_khr_icd_dispatch.clUnknown136’)

I checked the 2.2.8's ocl_icd.h:

#ifdef CL_VERSION_2_0
  CL_API_ENTRY cl_int (CL_API_CALL*clGetKernelSubGroupInfoKHR)(
cl_kernel /* in_kernel */,
cl_device_id /*in_device*/,
cl_kernel_sub_group_info /* param_name */,
size_t /*input_value_size*/,
const void * /*input_value*/,
size_t /*param_value_size*/,
void* /*param_value*/,
size_t* /*param_value_size_ret*/ ) CL_EXT_SUFFIX__VERSION_2_0;
#else
  CL_API_ENTRY cl_int (CL_API_CALL* clUnknown136)(void);
#endif

clGetKernelSubGroupInfoKHR is available from OpenCL2.0, also need OpenCL 
version check in beignet?

BTW: ocl-icd 2.2.8's OCL_ICD_IDENTIFIED_FUNCTIONS is 128, is it a typo?
> +#if (OCL_ICD_IDENTIFIED_FUNCTIONS > 110)

> -Original Message-
> From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of
> Rebecca N. Palmer
> Sent: Saturday, October 8, 2016 21:11
> To: beignet@lists.freedesktop.org
> Subject: [Beignet] [PATCH] Add clGetKernelSubGroupInfoKHR to
> _cl_icd_dispatch table
> 
> ocl-icd >= 2.2.8 has this function in its table; as the lookup process tries 
> the
> dispatch table before the ICD's clGetExtensionFunctionAddress, requesting a
> function that is in the ICD loader's table but off the end of the ICD's table 
> will
> return a garbage pointer.
> 
> Signed-off-by: Rebecca N. Palmer <rebecca_pal...@zoho.com>
> ---
>  src/cl_khr_icd.c | 21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cl_khr_icd.c b/src/cl_khr_icd.c index 8715bbd..2cf4909 100644
> --- a/src/cl_khr_icd.c
> +++ b/src/cl_khr_icd.c
> @@ -17,7 +17,7 @@
>  #include 
> 
>  #include "cl_platform_id.h"
> -
> +#include "CL/cl_intel.h" // for clGetKernelSubGroupInfoKHR
>  /* The interop functions are not implemented in Beignet */  #define
> CL_GL_INTEROP(x) NULL
>  /* OpenCL 1.2 is not implemented in Beignet */ @@ -168,7 +168,24 @@
> struct _cl_icd_dispatch const cl_khr_icd_dispatch = {
>(void *) NULL,
>(void *) NULL,
>(void *) NULL,
> -  (void *) NULL
> +  (void *) NULL,
> +#if (OCL_ICD_IDENTIFIED_FUNCTIONS > 110)
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  (void *) NULL,
> +  clGetKernelSubGroupInfoKHR,
> +#endif
>  #endif
>  };
> 
> --
> 2.1.4
> 
> 
> ___
> Beignet mailing list
> Beignet@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet
___
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet