Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dlopen option #2

Closed
MatejMagat305 opened this issue Mar 11, 2023 · 16 comments
Closed

Dlopen option #2

MatejMagat305 opened this issue Mar 11, 2023 · 16 comments

Comments

@MatejMagat305
Copy link

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
Copy link
Owner

kenba commented Mar 11, 2023

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
Copy link
Author

MatejMagat305 commented Mar 11, 2023

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
Copy link
Owner

kenba commented Mar 12, 2023

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.

@kenba kenba transferred this issue from kenba/cl3 Mar 12, 2023
@MatejMagat305
Copy link
Author

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
Copy link
Owner

kenba commented Mar 12, 2023

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
Copy link

@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 added a commit that referenced this issue Nov 10, 2024
@kenba
Copy link
Owner

kenba commented Nov 10, 2024

Thank you @dmitry-zakablukov , I think I get it now.
I've modified the develop branch to use pkg-config:

pkg_config::probe_library("OpenCL").unwrap();

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
Copy link

@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
Copy link

@kenba , please have a look at kenba/cl3#33

@kenba
Copy link
Owner

kenba commented Nov 16, 2024

@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
Copy link

@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
Copy link
Owner

kenba commented Nov 17, 2024

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 KhronosGroup/OpenCL-Headers#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
Copy link

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
Copy link
Owner

kenba commented Nov 20, 2024

@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
Copy link

@kenba , yes, this should work.

@kenba
Copy link
Owner

kenba commented Dec 7, 2024

Incorporated in release tag 0.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants