Remove cl3 `Info` enums to support new OpenCL versions and extensions.
Closed this issue · 6 comments
Change interface to cl3
for issue kenba/cl3#13.
Is the idea to remove the info enums also from opencl3
, or to move the enums from cl3
to the higher level opencl3
?
In case they are removed: I see the point, though it would remove type information, and to me one of rust strength is its type system. So I prefer if higher level APIs make use of types. Though perhaps there could even be yet another layer that is higher then opencl3
, which uses even more Rust idioms.
@vmx my intention is to remove all the Info
types from both cl3
and opencl3
.
I agree with you that it's nice to have the type information that the Info
types provided. My original intention was to use the Info
types in the get_*_info
functions match statements for exhaustive matching. However, as I've incorporated OpenCL extensions and changes to the API I now see that these types as effectively "open ended", with new values being added as the spec evolves.
Since data for new values can always be returned as a Vec<u8>
, it makes sense to me to allow users to call the get_*_info
functions with cl_device_info
, cl_platform_info
, etc. types using whatever value they want, because those values may be valid for an OpenCL device in the future. It also makes it simpler to maintain the library, since it is no longer necessary to update the the Info
types as well whenever a new value is added.
I've tried to provide methods in the opencl3
structs (e.g.: Device
, Platform
, etc.) to access all of the information that can be obtained using the Info
types in the correct form, e.g. Device::vendor_id
returns the vendor_id as a cl_uint
, etc. I think that I should also add get_data
methods to all of the structs to simply call the cl3
get_*_data
function with a given info value and return a Result<Vec<u8>>
to make the opencl3
API more future proof.
I've implemented the Info
types change in the develop branches of cl3
and opencl3
. I'd welcome your opinion on the changes Volker.
@kenba I had another look (especially at 02d5f68). What I hadn't in mind anymore (sorry for not having a closer look before commenting) was that you provide methods in opencl3
. This means that (hopefully) the usual code path wouldn't even need to make info calls directly. Given then I think it makes sense to make those changes as it really makes things simpler and more flexible.
@vmx Thank you Volker. I have implemented the changes in both libraries and updated them both to version 0.6.0 in crates.io.
BTW I've also implemented UUID
and LUID
types (see issue kenba/cl3#14), which I'm now having second thoughts about. Because if the driver returns a Vec<u8>
of the wrong size, the cl3
library will now panic. I'm considering reverting this change back to just returning a Vec<u8>
. What do you think?
BTW I've also implemented
UUID
andLUID
types (see issue kenba/cl3#14), which I'm now having second thoughts about. Because if the driver returns aVec<u8>
of the wrong size, thecl3
library will now panic. I'm considering reverting this change back to just returning aVec<u8>
. What do you think?
I like this change. The sizes of the IDs is specified in the OpenCL header files: https://github.com/KhronosGroup/OpenCL-Headers/blob/1aa1139b58a515877a923cce6b254e09d1b2fb2c/CL/cl_ext.h#L687-L688 Hence I don't see a reason why one shouldn't just depend on that information.
@vmx I agree with you that the OpenCL header files specify the sizes of the UUIDs and LUIDs so we should be able to depend on that information. However, we've both encountered OpenCL devices that do not to strictly conform to the OpenCL specification, see kenba/cl3#2 and #8 that you raised Volker.
I have implemented a fix for the panic issue in the cl3
library , see kenba/cl3#15. Since you like the new UUID and LUID types, the fix keeps them while eliminating the potential panic. That library has been published on crates.io as issue 0.6.1 and is picked up by opencl3 issue 0.6.0 after a cargo update.
BTW I have no more changes planned for the library, so this version (0.6.0) should be stable.