kenba / opencl-sys-rs

OpenCL C FFI bindings for the Rust programming language.
Apache License 2.0
3 stars 0 forks source link

Dlopen option #2

Open MatejMagat305 opened 1 year ago

MatejMagat305 commented 1 year ago

Hi, I know that it look like sily isue, but sometime is not able know direct path to opencl, (mainly in android are 12 path to library), I sugest add to option with dlopen similar https://github.com/krrishnarraj/libopencl-stub and https://github.com/vlang/vsl/tree/master/vcl

kenba commented 1 year ago

I'm sorry but I don't understand precisely what your issue is?
If it is for the path to the OpenCL ICD lib file, the code is in the opencl-sys-rs repo.

Please submit a pull request with your proposed solution following the CONTRIBUTING guidelines.

MatejMagat305 commented 1 year ago

so, maybe I m silly programer, this is more question than sugestion (but of course, if tell me i will try make PR to contribute), so my question is: is this library suitable to have option dlopen ? I mean that if I do not give flag it compile normaly, but if I give flag (for example "dlopencl") it would be using internary dlopen with multiple path (for platform) and call first which can open

why: in normal linux and windows is small variable of path, but in android we have many ...

kenba commented 1 year ago

You are not a silly programmer, I simply didn't understand what you wanted.

I think that what you want is to add a new OpenCL SDK path to search for the ICD.
The code currently resides in: https://github.com/kenba/opencl-sys-rs/blob/main/build.rs.
There is a list of known SDKs and their environment variables - you just ned to add yours to it, or use OPENCL_ROOT if you can.

MatejMagat305 commented 1 year ago

well, if I understand code, program at build will find opencl ".dll" and it will be embeded into binary ..., that way work in linux, mac and windows ...,

but in android it does not work for example if I build binary for android_xyz_samsung it would not work on android_abc_lenovo, because it has diferent path and name to "opencl.so"..., of course sometime in other OS it can be similar problem, but here is linking ...

I would need to build not "static" (concreate) path, but I would need build "dynamic" (at begin in runtime search in list of paths and make dlopen of available), I was parcipiate in similar situation in Vlang and can I in summer sugest PR?

of course this way has small performance penalty, but it would make library more flexible ...

kenba commented 1 year ago

Not quite. The program searches for OpenCL.so on linux., it only searches for OpenCL.dll on Windows.
Since android is a effectively a port of linux, it should "dlopen" OpenCL.so as long as you ensure that OpenCL.so is on the dynamic library search path, i.e. one of the environment variables in build.rs must define the location of OpenCL.so . Is your issue specific to your android device, see: https://stackoverflow.com/questions/48453097/opencl-dlopen-issue?

dmitry-zakablukov commented 3 weeks ago

@kenba , I think the proposal is to add the ability to link dynamically with OpenCL library in the runtime, and not during the compilation stage.

I have a similar requirement: I need to code a program that can be launched on any machine, even on those that do not have OpenCL library in the search path. And after startup the program should try to dlopen OpenCL library. If no success, than the message should appear: "No OpenCL detected".

If I decide to link with opencl-sys-rs in my program, than the program will fail to launch on a machine without OpenCL library in the search path.

So the question is: can the dlopen feature be added to opencl-sys-rs crate to open OpenCL library in runtime?

kenba commented 2 weeks ago

Thank you @dmitry-zakablukov , I think I get it now.
I've modified the develop branch to use pkg-config: https://github.com/kenba/opencl-sys-rs/blob/fd619e987cf69e47f69257974ae10524274e5dc4/build.rs#L53

I've tested it on Ubuntu which required me to install pkg-config for it to work.
Please can you try it on your machine?

dmitry-zakablukov commented 2 weeks ago

@kenba , well, this is still the linkage on the compilation stage, but now with the help of pkg-config.

I am currently working on dynamic load of OpenCl library and its integration in cl3 package. The merge requests to cl3 will be in a few days or week.

dmitry-zakablukov commented 2 weeks ago

@kenba , please have a look at https://github.com/kenba/cl3/pull/33

kenba commented 1 week ago

@dmitry-zakablukov I have changed this library based on the guidance from here: Making a *-sys crate.
That is: I've added static and dynamic features and removed default features.

Static linking now requires the static feature in addition to the other required OpenCL features.
For example, cl3 requires the following changes to its Cargo.toml to support its current tests:

[features]

static  = ["opencl-sys/static"]
dynamic = ["opencl-sys/dynamic"]
...
# Default features:
default = ["static", "CL_VERSION_1_1", "CL_VERSION_1_2"]

Please see the advice for "sys" packages from the Cargo book: sys-packages for accompanying changes to the cl3 crate.

dmitry-zakablukov commented 1 week ago

@kenba , how should dynamic feature work now? If I enable this feature, cl3 crate will not compile because of missing functions in opencl-sys-rs.

Side note: static and dynamic features is mutually exclusive. If we add both of them, we should handle the case when a user enables both of them. I think it is better to add only one feature, dynamic for example. Then the "static" case will be the case of not(dynamic).

kenba commented 1 week ago

The dynamic feature should be a feature of the cl3 crate to permit linking using dlopen.

From sys-packages:

The library crate should provide declarations for types and functions in libfoo, but not higher-level abstractions.

So opencl-sys-rs should not support dynamic linking, it should be provided by the higher level crate, namely: cl3.

The dynamic cl3 code should be based on utils.rs and should use the cl_icd.rs module from this repo, see https://github.com/KhronosGroup/OpenCL-Headers/pull/230.

The dependency on opencl-dynamic-sys should be removed, since it does not conform to the requirements for a sys-package.

If the static and dynamic features are mutually exclusive, then it is better for the dynamic case to be not(static).

dmitry-zakablukov commented 1 week ago

I'm not quite understand how cl_icd is connected with the issue and how can it help to write a dynamic load support, because not all system libraries have an equivalent for cl_icd, but may have the same issue with dynamic load. So, I think the approach should be more generic, rather than using cl_icd.

Yes, dynamic feature may be expressed as not(static), the thing is that in this case the dynamic linkage will be default, unless a static feature is specified. I do not know if it is a desired behavior for you.

I understand now that opencl-dynamic-sys crate is not suitable for your. On the other hand, I already implemented all the code I needed in forked versions of cl3 and opencl3 crates. So you may adopt some ideas or reject all of them from the opened pull-request of mine.

I'm really looking forward for the future version of cl3 crate with the proper implementation of the dynamic linkage!

kenba commented 1 week ago

@dmitry-zakablukov I was wrong about cl_icd, I thought that the types defined in cl_function_types.rs could be used in mod.rs, but I could get it to work, so I've used your OpenCl struct.

I've copied the container directory from your opencl-dynamic-sys repo, renamed to runtime and the dynamic_runtime.rs and static_runtime.rs files from your PR, renamed to dynamic_library.rs and static_library.rs.

I've updated the platform.rs file to support dynamic linking and removed default features from Cargo.toml and added them to the [dev-dependencies] where they can be changed for testing. I've fixed the clippy lints but I have not been able to test it in dynamic mode.

Please let me know if you think these changes will work in dynamic mode.

dmitry-zakablukov commented 6 days ago

@kenba , yes, this should work.