Bug 73512

Created attachment 91884 [details] [review]
0001-opencl-improved-auto-gen-.icd.patch

I've not checked this patch. Checking.

Created attachment 91885 [details] [review]
[PATCH v2] opencl: improved auto-gen .icd

Ok. Well. I've tested patch and some updated.

--
v2: Use @OPENCL_VERSION@:0 for library
    replace /etc with @sysconfdir@ macros

Created attachment 91886 [details] [review]
[PATCH v2] opencl: improved auto-gen .icd

oops. I've attached old patch. Sorry.

According to the icd spec: http://www.khronos.org/registry/cl/extensions/khr/cl_khr_icd.txt

The vendors directory must go in /etc/OpenCL and also only the library name is included in the *.icd file, not the full path, so I don't think this patch is correct.

What problem does this patch fix?

(In reply to comment #4)
> According to the icd spec:
> http://www.khronos.org/registry/cl/extensions/khr/cl_khr_icd.txt
> 
> The vendors directory must go in /etc/OpenCL and also only the library name
> is included in the *.icd file, not the full path, so I don't think this
> patch is correct.
> 
> What problem does this patch fix?
1. we're not installing .so to main package. we are installing it to -devel subpackage. So .icd should contain like .so.@LIBVER@
2. Fabian, Can you try to use liMesaOpenCL.so.1 in .icd file. What clinfo will do say ?

Hey,

this can all be a result of me being uninformed (not knowing that only the library name is contained in the .icd file).

But I think that the .icd file is still not corect, as it contains only the unversioned library name libMesaOpenCL.so - which is - as Igor metions - not packaged in the main packages (only in devel subpackages).

So I'm not sure if the original icd file should contain the versioned library, or if we should do this downstream in Fedora.

Created attachment 91973 [details] [review]
[PATCH v3] opencl: improved auto-gen .icd

v2: Use @OPENCL_VERSION@:0 for library
    replace /etc with @sysconfdir@ macros

v3: Drop libdir from icd, because libMesaOpenCL isn't private

(In reply to comment #7)
> Created attachment 91973 [details] [review] [review]
> [PATCH v3] opencl: improved auto-gen .icd
> 
> v2: Use @OPENCL_VERSION@:0 for library
>     replace /etc with @sysconfdir@ macros
> 
> v3: Drop libdir from icd, because libMesaOpenCL isn't private

If we install the *.icd file to @sysconfdir@  and not /etc then standards compliant ICD loaders will not work with clover.  The way I interpret the spec, we have no choice, but to install it to /etc .  Why is it necessary to use @sysconfdir@ ?

(In reply to comment #8)
> (In reply to comment #7)
> > Created attachment 91973 [details] [review] [review] [review]
> > [PATCH v3] opencl: improved auto-gen .icd
> > 
> > v2: Use @OPENCL_VERSION@:0 for library
> >     replace /etc with @sysconfdir@ macros
> > 
> > v3: Drop libdir from icd, because libMesaOpenCL isn't private
> 
> If we install the *.icd file to @sysconfdir@  and not /etc then standards
> compliant ICD loaders will not work with clover.  The way I interpret the
> spec, we have no choice, but to install it to /etc .  Why is it necessary to
> use @sysconfdir@ ?

why I can't install mesa in /usr/local or in /opt ? I think no problems there..

Should I update patch w/o/ this macros ?

Created attachment 92004 [details] [review]
[PATCH v4] opencl: improved auto-gen .icd

v2: Use @OPENCL_VERSION@:0 for library
    replace /etc with @sysconfdir@ macros

v3: Drop libdir from icd, because libMesaOpenCL isn't private

v4: install ocl vendor always to /etc

(In reply to comment #8)
> (In reply to comment #7)
> > Created attachment 91973 [details] [review] [review] [review]
> > [PATCH v3] opencl: improved auto-gen .icd
> > 
> > v2: Use @OPENCL_VERSION@:0 for library
> >     replace /etc with @sysconfdir@ macros
> > 
> > v3: Drop libdir from icd, because libMesaOpenCL isn't private
> 
> If we install the *.icd file to @sysconfdir@  and not /etc then standards
> compliant ICD loaders will not work with clover.  The way I interpret the
> spec, we have no choice, but to install it to /etc .  Why is it necessary to
> use @sysconfdir@ ?

Yes. I'm sorry. https://forge.imag.fr/plugins/scmgit/cgi-bin/gitweb.cgi?p=ocl-icd/ocl-icd.git;a=blob;f=ocl_icd_loader.c;h=ab419b2dccb82db6d632cae6dc86e5151a320c07;hb=HEAD#l52

Only /etc will work. Fixed. Patch here.

Created attachment 92091 [details] [review]
[PATCH] opencl: use versioned .so in mesa.icd

We must have versioned library in mesa.icd, because ICD loader would
fail if the mesa-devel package wasn't installed.

Created attachment 98450 [details] [review]
[PATCH] opencl: use versioned .so in mesa.icd

rebased for master

Created attachment 99773 [details]
[PATCH rebased] opencl: use versioned .so in mesa.icd

http://cgit.freedesktop.org/mesa/mesa/commit/?id=4d7e0fa8c731776ad5d630f37b36c535f1907371