23 Aug
2012
23 Aug
'12
9:06 a.m.
[resent after subscribing to the ML to avoid the message to be rejected]
Le 22/08/2012 10:24, Vincent Danjean a écrit :
Le 21/08/2012 22:49, Tomasz Rybak a écrit :
Sorry - did not have time to investigate it deeply.
Description of OpenCL ICD extension does not deal with case
of different platform versions.
Vincent - any thoughts about how OpenCL should behave
in current case, when loader has version 1.2 and
tries to use ICD with version 1.1 (here NVIDIA one)?
Best regards.
--
Vincent Danjean Adresse: Laboratoire d'Informatique de Grenoble
Téléphone: +33 4 76 61 20 11 ENSIMAG - antenne de Montbonnot
Fax: +33 4 76 61 20 99 ZIRST 51, avenue Jean Kuntzmann
Email: Vincent.Danjean(a)imag.fr 38330 Montbonnot Saint Martin
Added Vincent Danjean (co-author of free OpenCL
ICD loader) to CC.
Adding Brice, the other OpenCL ICD Loader co-author
Dnia 2012-08-21, wto o godzinie 11:25 -0400,
Andreas Kloeckner pisze:
> Andreas Kloeckner <lists(a)informa.tiker.net> writes:
>> Tomasz Rybak <tomasz.rybak(a)post.pl> writes:
>>> As I wrote in my email from 2012-08-14, I experienced
>>> crashes in image-related functions in test_wrapper.py
>>> on NVIDIA hardware. I managed to find the reason of that
>>> crash and fix it (patch in attachment). Below you can find
>>> explanation.
>>>
>>> Unlike all other vendors, NVIDIA still have not released
>>> OpenCL 1.2. Image creation functions have changed in
>>> OpenCL 1.2 - now clCreateImage expects to get cl_image_desc
>>> instead of bunch of arguments like height, width, etc.
>>> PyOpenCL tests in Image constructor (pyopencl/__init__.py,
>>> line 200-ish) whether it is run on OpenCL 1.2 or 1.1,
>>> and runs appropriate code based on this. It uses
>>> get_cl_header_version() for this check which fails in
>>> some situations, e.g. on Debian. In Debian we have opencl-headers
>>> (currently in 1.2), ICD loader (1.2) and ICD implementations
>>> with different versions. This means that headers will always
>>> have version 1.2 (or higher - but it'll be the highest
>>> possible version) but platforms might have lower versions.
>>> This was the case of this segfault. PyOpenCL expected to have
>>> new clImageCreate, ICD loader was ready to give pointer
>>> to this function to PyOpenCL, but platform (NVIDIA)
>>> was not providing it.
>>>
>>> I have changed Image constructor to base usage of clCreateImage on
>>> devices' platform version. I assumed that Context always have
>>> at least one device - if not, please change this code.
>>
>> I'm wondering--isn't this an issue with the ICD loader? I had sort of
>> expected that the header needs to match the loader, and if the loader
>> exports a 1.2 interface, then all of those functions are at least safe
>> to call--i.e. I as a user don't have to go around checking versions just
>> to determine what API I can call.
>>
>> In this particular instance, I had thought the ICD loader would
>> translate the call to the old 1.1 interface, or, if impossible, provide
>> an error.
>>
>> A segfault is most definitely not an appropriate response...
>>
>> OTOH, it seems both the AMD ICD loader and the open-source ICD loader
>> (as you indicate) behave this way, so we might not get a choice in this
>> matter.
>>
>> Does the spec say anything on this? What's your assessment? Are these
>> transient bugs in the ICD loader, design flaws in the spec, or something
>> completely different?
I think this is a design flaws in the specs.
An ICD Loader has no information at all (unless we decide to hardcode
some of them) about the supported OpenCL functions by the loaded ICD.
An ICD Loader merely get a address of of array of function pointers. It
even does not know the size of the array it gets. This means, there is
no reliable way to know if the address we got for a 1.2 function is
garbage (belong the end of the table of an 1.1 implementation) or
correct.
Looking at version advertised by the ICD implementation is not a
solution: Intel implementation advertises 1.1 but implement (part of)
1.2
I'm willing to add/patch anything required in ocl-icd. We can add some
more functions to the interface (it means that a program using these
functions will not work with other OpenCL ICD loader) or provides
them in an additional library (so that it works with any ICD Loader
implementation).
But, for now, I see no other way than using hardcoded information.
If we go this path, we should think about which information we want
exactly and how we want them to be presented (ie API/ABI)
What I can propose is that, for any public symbol, we try to look
if the corresponding function exists in the targeted ICD. Some
sanity checks can be done automatically (non-null pointer, ...)
but some hard-coded information will be required.
I also ask me how/if we can divert the internal function pointer
structure provided by the implementation in order to fully fill
a whole structure (with error functions for the missing ones).
I think that is is feasible. But that it would be possible for
a strange ICD implementation to respect the standard and break
with what I imagine (ie my implementation would be border-line
with respect to the ABI specifications)
About the specific problem of clCreateImage, I tried to look at
it. If I understand correctly,
OpenCL 1.1 defines clCreateImage2D and clCreateImage3D
OpenCL 1.2 defines clCreateImage and deprecates clCreateImage2D
and clCreateImage3D
I.e, contrary to the initial message, I do not think that a
prototype changes (this would be a severe bug with respect to
ICD Loader specifications). But we have 2/3 functions with
similar prototypes, some provided by some implementations,
other provided by other implementations.
But, for now, I do not think there is any way an implementation
can currently reliably detect at run-time if an specific plate-form
implement or not theses functions.
Regards,
Vincent
I must admit I'm pretty reluctant to call a bunch
of GetInfo functions
and then do a bunch of string processing just to figure out what
function is safe to call. Maybe as a temporary workaround, but not as a
permanent thing.
Any opinions/insights?
Hi Tomasz,
any news on this front? 
23 Aug
23 Aug
11:10 a.m.
New subject: Image creation crash on NVIDIA OpenCL - patch
Hi Vincent,
Vincent Danjean <vdanjean.ml(a)free.fr> writes:
> I think this is a design flaws in the specs.
> An ICD Loader has no information at all (unless we decide to hardcode
> some of them) about the supported OpenCL functions by the loaded ICD.
The ICD loader *could* call the implementation's platform version
GetInfo, though, and route the CreateImage calls accordingly?
But if that's not accepted practice among ICD loaders, it's not useful,
as PyOpenCL has to work against all of them.
Is any of you on the CL committee and able to let them know that they
screwed up? If not, I can reach see if I can reach someone...
> An ICD Loader merely get a address of of array of
function pointers. It
> even does not know the size of the array it gets. This means, there is
> no reliable way to know if the address we got for a 1.2 function is
> garbage (belong the end of the table of an 1.1 implementation) or
> correct.
> Looking at version advertised by the ICD implementation is not a
> solution: Intel implementation advertises 1.1 but implement (part of)
> 1.2
That seems perfectly within-spec. I.e. "call the 1.2 bits at your own
risk". I don't see the ICD loader under any obligation to make it
possible to call the 1.2 bits if the ICD advertises 1.1.
> I'm willing to add/patch anything required in
ocl-icd. We can add some
> more functions to the interface (it means that a program using these
> functions will not work with other OpenCL ICD loader) or provides
> them in an additional library (so that it works with any ICD Loader
> implementation).
> But, for now, I see no other way than using hardcoded information.
> If we go this path, we should think about which information we want
> exactly and how we want them to be presented (ie API/ABI)
>
> What I can propose is that, for any public symbol, we try to look
> if the corresponding function exists in the targeted ICD. Some
> sanity checks can be done automatically (non-null pointer, ...)
> but some hard-coded information will be required.
>
> I also ask me how/if we can divert the internal function pointer
> structure provided by the implementation in order to fully fill
> a whole structure (with error functions for the missing ones).
> I think that is is feasible. But that it would be possible for
> a strange ICD implementation to respect the standard and break
> with what I imagine (ie my implementation would be border-line
> with respect to the ABI specifications)
>
>
> About the specific problem of clCreateImage, I tried to look at
> it. If I understand correctly,
> OpenCL 1.1 defines clCreateImage2D and clCreateImage3D
> OpenCL 1.2 defines clCreateImage and deprecates clCreateImage2D
> and clCreateImage3D
> I.e, contrary to the initial message, I do not think that a
> prototype changes (this would be a severe bug with respect to
> ICD Loader specifications). But we have 2/3 functions with
> similar prototypes, some provided by some implementations,
> other provided by other implementations.
What a nightmare.
> But, for now, I do not think there is any way an
implementation
> can currently reliably detect at run-time if an specific plate-form
> implement or not theses functions.
(Not sure I agree here--see first comment.)
Andreas
12:56 p.m.
New subject: Image creation crash on NVIDIA OpenCL - patch
Le 23/08/2012 13:10, Andreas Kloeckner a écrit :
This means that our ICD loader will not be useful (and used) when
vendors get place in the function pointer structure before these
extensions are normalized into a future OpenCL spec.
Looking at what happens into the structure (places not used anymore
between official OpenCL function 1.0 and 1.1, similar between OpenCL
1.1 and OpenCL 1.2), this is not theoretical at all.
In any case, the design of ICD is done so that different versions of
OpenCL ICD can be loaded by the same loader. It means that the
OpenCL version cannot be chosen/checked at compile-time but only at
execution time.
Supported OpenCL version can be, more or less, obtained by requesting
the plate-form version, we must got "OpenCL 1.X ..."
Using this, OpenCL software can decide to :
- ask for another plate-form (if available)
- adapt its code to the advertized OpenCL version
this can perhaps be done in a mid-layer that will be reusable
- crash/emit an error message
- ...
One must take care that some implementation does not implement deprecated
functions. For example, Intel OpenCL 1.1 implementation does not provide
any function for the deprecated clSetCommandQueueProperty...
In my opinion, what is missing in the current spec are (at least):
- a way to know the size of the structure we got from the ICD
An easy way to do so can be to give this size (in bytes or in entries)
as a long stored just before the current structure *and* advertize this
by declaring an extension in the ICD.
- a way for a program to know if a plate-form implements or not any
function. The clGetExtensionFunctionAddressForPlatform might be the
solution (but the previous information is required)
=> I will try to do it correctly for the next ocl-icd release
adding some hardcoded information if needed.
- a way for a program to require a specific (minimum/exact/maximum)
OpenCL version. In fact, this can be done by iterating over the
available plate-forms.
Regards,
Vincent
Hi Vincent,
Vincent Danjean <vdanjean.ml(a)free.fr> writes:
We are not in the CL committee at all. Brice was working with OpenCL
and decided to write a free OCL ICD loader when he see that none was
existing and that a few reverse engineering would allow him to write one.
He put me in the loop as I'm more experimented with software packaging.
> I think this is a design flaws in the specs.
> An ICD Loader has no information at all (unless we decide to hardcode
> some of them) about the supported OpenCL functions by the loaded ICD.
The ICD loader *could* call the implementation's platform version
GetInfo, though, and route the CreateImage calls accordingly?
But if that's not accepted practice among ICD loaders, it's not useful,
as PyOpenCL has to work against all of them.
Is any of you on the CL committee and able to let them know that they
screwed up? If not, I can reach see if I can reach someone... > An ICD
Loader merely get a address of of array of function pointers. It
> even does not know the size of the array it gets. This means, there is
> no reliable way to know if the address we got for a 1.2 function is
> garbage (belong the end of the table of an 1.1 implementation) or
> correct.
> Looking at version advertised by the ICD implementation is not a
> solution: Intel implementation advertises 1.1 but implement (part of)
> 1.2
That seems perfectly within-spec. I.e. "call the 1.2 bits at your own
risk". I don't see the ICD loader under any obligation to make it
possible to call the 1.2 bits if the ICD advertises 1.1. > I'm
willing to add/patch anything required in ocl-icd. We can add some
> more functions to the interface (it means that a program using these
> functions will not work with other OpenCL ICD loader) or provides
> them in an additional library (so that it works with any ICD Loader
> implementation).
> But, for now, I see no other way than using hardcoded information.
> If we go this path, we should think about which information we want
> exactly and how we want them to be presented (ie API/ABI)
>
> What I can propose is that, for any public symbol, we try to look
> if the corresponding function exists in the targeted ICD. Some
> sanity checks can be done automatically (non-null pointer, ...)
> but some hard-coded information will be required.
>
> I also ask me how/if we can divert the internal function pointer
> structure provided by the implementation in order to fully fill
> a whole structure (with error functions for the missing ones).
> I think that is is feasible. But that it would be possible for
> a strange ICD implementation to respect the standard and break
> with what I imagine (ie my implementation would be border-line
> with respect to the ABI specifications)
>
>
> About the specific problem of clCreateImage, I tried to look at
> it. If I understand correctly,
> OpenCL 1.1 defines clCreateImage2D and clCreateImage3D
> OpenCL 1.2 defines clCreateImage and deprecates clCreateImage2D
> and clCreateImage3D
> I.e, contrary to the initial message, I do not think that a
> prototype changes (this would be a severe bug with respect to
> ICD Loader specifications). But we have 2/3 functions with
> similar prototypes, some provided by some implementations,
> other provided by other implementations.
What a nightmare.
> But, for now, I do not think there is any
way an implementation
> can currently reliably detect at run-time if an specific plate-form
> implement or not theses functions.
(Not sure I agree here--see first comment.) Andreas
--
Vincent Danjean Adresse: Laboratoire d'Informatique de Grenoble
Téléphone: +33 4 76 61 20 11 ENSIMAG - antenne de Montbonnot
Fax: +33 4 76 61 20 99 ZIRST 51, avenue Jean Kuntzmann
Email: Vincent.Danjean(a)imag.fr 38330 Montbonnot Saint Martin
8:58 p.m.
New subject: Image creation crash on NVIDIA OpenCL - patch
Hi,
Le 23/08/2012 14:56, Vincent Danjean a écrit :
In my opinion, what is missing in the current spec
are (at least):
- a way to know the size of the structure we got from the ICD
An easy way to do so can be to give this size (in bytes or in entries)
as a long stored just before the current structure *and* advertize this
by declaring an extension in the ICD.
I was talking to quickly.
- a way for a program to know if a plate-form
implements or not any
function. The clGetExtensionFunctionAddressForPlatform might be the
solution (but the previous information is required)
=> I will try to do it correctly for the next ocl-icd release
adding some hardcoded information if needed.
From the khronos spec:
The function
clGetExtensionFunctionAddressForPlatform returns the
address of the extension function named by funcname for a given
platform.
[...]
clGetExtensionFunctionAddressForPlatform may not be queried for
core (non-extension) functions in OpenCL.
[and AMD ICD give NULL value for core-functions, indeed]
So, this is not a good way to know if a deprecated or new function
is supported or not. And even if ocl-icd can answer correctly, it
would not be portable :-(
- a way for a program to require a specific
(minimum/exact/maximum)
OpenCL version. In fact, this can be done by iterating over the
available plate-forms.
Regards,
Vincent
--
Vincent Danjean Adresse: Laboratoire d'Informatique de Grenoble
Téléphone: +33 4 76 61 20 11 ENSIMAG - antenne de Montbonnot
Fax: +33 4 76 61 20 99 ZIRST 51, avenue Jean Kuntzmann
Email: Vincent.Danjean(a)imag.fr 38330 Montbonnot Saint Martin
Andreas
9:18 p.m.
New subject: Image creation crash on NVIDIA OpenCL - patch
Vincent Danjean <vdanjean.ml(a)free.fr> writes:
Hi,
Le 23/08/2012 14:56, Vincent Danjean a écrit :
I've pinged Khronos about this:
http://www.khronos.org/message_boards/viewtopic.php?f=28&t=5268
Let's see if we get a reply, otherwise I'll bug someone by direct email.
Andreas
In my opinion, what is missing in the current
spec are (at least):
- a way to know the size of the structure we got from the ICD
An easy way to do so can be to give this size (in bytes or in entries)
as a long stored just before the current structure *and* advertize this
by declaring an extension in the ICD.
I was talking to quickly.
- a way for a program to know if a plate-form
implements or not any
function. The clGetExtensionFunctionAddressForPlatform might be the
solution (but the previous information is required)
=> I will try to do it correctly for the next ocl-icd release
adding some hardcoded information if needed.
From the khronos spec:
The function clGetExtensionFunctionAddressForPlatform returns the
address of the extension function named by funcname for a given
platform.
[...]
clGetExtensionFunctionAddressForPlatform may not be queried for
core (non-extension) functions in OpenCL.
[and AMD ICD give NULL value for core-functions, indeed]
So, this is not a good way to know if a deprecated or new function
is supported or not. And even if ocl-icd can answer correctly, it
would not be portable :-(
3014
days inactive
3014
days old
4 comments
2 participants
participants (2)
-
Andreas Kloeckner -
Vincent Danjean