kenba / opencl3

A Rust implementation of the Khronos OpenCL 3.0 API.
Apache License 2.0
102 stars 13 forks source link

opencl3 cannot be compiled with OpenCl 1.2 features only #30

Closed vmx closed 3 years ago

vmx commented 3 years ago

Currently it is not possible to compile opencl3 with only OpenCL 1.2 features enabled.

Background is that I only use OpenCL 1.2 features in my code and MacOS only supports OpenCL 1.2, hence I'd like to compile a version without OpenCL 2.0 support, as else there would be linker issues. (btw: I also wonder why they are included although they are not used, perhaps it's because #[inline] is used a lot).

There are compile-time features flags to support building for OpenCL 1.2 only, but these don't work as I'd expect them to work. One issue is that the cl3 dependency needs to be set to default-features = "false", else their default features (CL_VERSION_1_2 and CL_VERSION_2_0) will always be set, independent of which features opencl3 sets.

Some functions in cl3 were introduced in OpenCL 2.0, but are not behind the CL_VERSION_2_0 feature flag (e.g. the SVM functionality), hence those cannot be disabled in opencl3 at compile time.

In opencl3 only some functionality is behind a feature flag, some are not (e.g. the whole svm module should require at least OpenCL 2.0).

The task would be to go over the whole code base and define the feature flags according to the spec. I guess the functions should be annotated with all versions they are available in. So if e.g. something was already part of OpenCL 1.2, but is still there in OpenCL 2.0, it should have both features defined. It would be great if there would be some document that lists all the OpenCL API and by which version it was introduced (I haven't found something like this yet).

PS: Please excuse the tone of this issue, it's harsher than I wanted. It's been a long day, but I wanted to get this bug report out :)

kenba commented 3 years ago

Volker, I'm sorry to hear that the cl3 and opencl3 versioning is causing you so much trouble.
Unfortunately, I've only been using OpenCL 2.0 (or above) ICDs to test the newer features and I was unaware that there compatibility issues with OpenCL 1.2.

Please feel free to submit a PR for this and cl3 to fix the build issues you have on your Mac.

vmx commented 3 years ago

PR for cl3 is ready: https://github.com/kenba/cl3/pull/9. The features now work like in cl-sys, where the feature is specified on the version they got introduced.

While working on it, I noticed a few things. I'm happy to open separate bug reports, but I though I mention them here first:

kenba commented 3 years ago

@vmx your PR is now in version 0.4.0 of the library on crates.io.

Personally, I would prefer to see cl-sys properly maintained rather than create a new fork. However, that may not be possible...

Although It's a nice idea to use the features to deprecate functions, I agree with you that it is probably more trouble than it is worth. I tried to use Rust deprecated attributes, but clippy wasn't happy with them; they are clearly intended for rust versions, not the way that our OpenCL features are set up. The functions are commented as deprecated and that will have to do for now.

I ran cargo fmt and cargo clippy on both this library and cl3. They work fine on this library and I'll look at integrating them into github actions when I have some time. However, neither clippy nor fmt worked to my liking on cl3, so I don't plan to use them on that library.

vmx commented 3 years ago

Sadly this issue isn't fixed in this release. The cl dependency in the Cargo.toml needs to be:

cl3 = { version = "0.4.2", default-features = false }

Else cl is compiled with the default features (hence also CL_VERSION_2_0) enabled. The top section

CL_VERSION_1_2 = ["cl3/CL_VERSION_1_2"]
CL_VERSION_2_0 = ["cl3/CL_VERSION_2_0"]
CL_VERSION_2_1 = ["cl3/CL_VERSION_2_1"]
CL_VERSION_2_2 = ["cl3/CL_VERSION_2_2"]
CL_VERSION_3_0 = ["cl3/CL_VERSION_3_0"]

already makes sure that CL_VERSION_1_2 and CL_VERSION_2_0 are correctly set by default.

kenba commented 3 years ago

No problem Volker, I'll change the cl3 line.

kenba commented 3 years ago

@vmx I've committed the change and updated the library to version 0.4.0 on crates.io.

vmx commented 3 years ago

Thanks for the fix, I can confirm that it works now with version 0.4.1.