clvk
clvk copied to clipboard
clvk not compatible with applications that use global destructors
The approach of using a global class instance to destruct the Vulkan instance/devices does not work when the OpenCL application also uses global destructors for cleaning up OpenCL objects.
For example, Halide releases its OpenCL objects via a function marked with __attribute__((destructor))
. On a platform I'm working on, this function is called after clvk's cleanup routine, which causes the application to crash at exit.
The obvious fix would be to have clvk initialize Vulkan during the first call to clGetPlatformIDs
, and destroy everything when the final cl_device_id
handle is destroyed. I'm sure we've probably discussed this in the past though and can't remember what the primary concern was. Is this issue that we'd leave Vulkan state around for OpenCL applications that don't clean up properly?
This is a grey area. Whether what Halide does is allowed or forbidden is not specified.
The problem with what you're suggesting is that there is no explicit way to trigger the cleanup. Only partitioned devices are refcounted. Even if you assume that devices are refcounted, then there is still the issue of platforms.
Ideally, we'd want to extend OpenCL with a concept similar to a Vulkan instance.
I've raised this with Halide, and the suggested fix is to remove __attribute__((destructor))
from this routine in Halide, and just rely on the OpenCL implementation to clean everything up properly at application exit (i.e. Halide would not necessarily call clRelease*
on all of the objects it created).
For clvk, this would require that we keep track of the objects that the application has created and destroy them before we destroy the Vulkan instance (otherwise we get Vulkan validation errors). Does this sound OK?
Removing the __attribute__((destructor))
sounds good but I don't think OpenCL implementations are required (or should be required) to perform this cleanup. I think it's better for Halide apps to be required to call a cleanup routine (which would be absolutely required for direct Vulkan support).