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

Implement cl_khr_icd 2.0.0 loader managed dispatch #1354

Merged
merged 4 commits into from
Nov 24, 2023

Conversation

Kerilk
Copy link
Contributor

@Kerilk Kerilk commented Nov 20, 2023

This implements the loader managed dispatch strategy presented here: KhronosGroup/OpenCL-Docs#1003

@pjaaskel
Copy link
Member

Thanks Brice! I'll launch the CIs. I added some questions in the extension PR. @franz can you add an ICD2-compliant ICD loader to one of the CIs?

@franz
Copy link
Contributor

franz commented Nov 23, 2023

I can add it as soon as it lands in ocl-icd's ubuntu ppa.

@Kerilk
Copy link
Contributor Author

Kerilk commented Nov 23, 2023

Thanks for the reviews. Not sure when this will be ready. I want to make sure I can implement instance layers on top of this (WIP is here https://github.com/Kerilk/pocl/tree/instance).
Also, I don't know if pocl is unloadable. I know it does not cleanup cleanup the pocl_devices global variable, so I would lean on the side of no. I will add a query to the platform CL_PLATFORM_UNLOADABLE_KHR and have it return no for now.

@franz
Copy link
Contributor

franz commented Nov 23, 2023

@Kerilk it does not cleanup everything, but it does have cleanup - some drivers implement the cleanup hooks, and also there is code to unload LLVM (as much as possible). It's disabled by default, IIRC there were some problems with reliability, and there wasn't any official API to unload the drivers / platforms, only the compiler. However if you try export POCL_ENABLE_UNINIT=1 and in the program call clUnloadPlatformCompiler after releasing all cl_* objects, PoCL should (at least with CPU driver) stop the driver threads and unload almost everything (except some static variables), to the point that both Valgrind and AddressSanitizer show no memory leaks. It should be possible to release remaining static variables in PoCL (if it's needed), but obviously we can't do anything about static variables of libLLVM or other libraries PoCL links to.

@pjaaskel pjaaskel merged commit ba4085f into pocl:main Nov 24, 2023
35 checks passed
@pjaaskel
Copy link
Member

There was some changes planned still to the icd2 in the OpenCL WG so let's revert this one. @franz can you do it in your next PR (my local PoCL repo is unclean)?

pjaaskel added a commit to pjaaskel/pocl that referenced this pull request Nov 28, 2023
This reverts commit ba4085f, reversing
changes made to a421006.
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

Successfully merging this pull request may close these issues.

3 participants