Add __SYCL_EXPORT to declaration of contextSetExtendedDeleter #1531
Conversation
Signed-off-by: Stuart Adams <stuart.adams@codeplay.com>
nyalloc
force-pushed
the
stuart/export_contextSetExtendedDeleter
branch
from
db77c55 to
3126f99
Compare
Signed-off-by: Stuart Adams <stuart.adams@codeplay.com>
| void contextSetExtendedDeleter(const cl::sycl::context &constext, | ||
| pi_context_extended_deleter func, | ||
| void *user_data); | ||
| __SYCL_EXPORT void contextSetExtendedDeleter(const cl::sycl::context &constext, |
There was a problem hiding this comment.
I don't see any users of it. Why does this need to be exported from libsycl?
There was a problem hiding this comment.
Please also document the recently added PI API: piextContextSetExtendedDeleter in pi.h
There was a problem hiding this comment.
Why is it not implemented for OpenCL?
There was a problem hiding this comment.
Hi, this function is used in the cuBLAS backend to OneMKL. If it isn't exported, the cuBLAS backend does not work.
uxlfoundation/oneMath#2 (comment)
Good idea, the function will be documented in subsequent commits.
It has no equivelant in OpenCL backend because it isn't required by any OpenCL driven projects. Is there a way you would like this to be marked as CUDA specific?
There was a problem hiding this comment.
I think using any symbols from sycl::detail namespace is dangerous as it's by design intended for internal implementation details.
I suggest oneMKL to use "public/standard/stable" API if possible.
tagging @mkrainiuk, @mehdi-goli.
There was a problem hiding this comment.
This is meant to be a temporary fix until there is a SYCL extension to cover for this functionality, that's why its exposing some implementation details, because there is no public API for this at this point.
There was a problem hiding this comment.
Is there a SYCL extension in flight to support this?
What's exactly the functionality achieved here (it's hard to tell for sure because there is not documentation of this extension)?
I'd like to approve this to unblock MKL, but please create an issue (or just PR) to document the piextContextSetExtendedDeleter and also to discuss if there is a better way to achieve what's needed.
…c_abi_checks * origin/sycl: [SYCL][Driver] Enforce unique filenames when -save-temps is used (intel#1545) [SYCL] [xmethods] Allow replacing xmethod script (intel#1532) [SYCL] Add tests for inline asm feature (intel#1444) [SYCL][Doc] Add device_specific_kernel_queries extension. (intel#1540) [SYCL][USM] Remove unused header and unnecessary includes (intel#1537) Fix check-llvm dependencies (intel#1547) [SYCL] Add __SYCL_EXPORT to declaration of contextSetExtendedDeleter (intel#1531) [SYCL][Doc] Add static local memory query extension. (intel#1539) [SYCL][Doc] Add sycl_bitcast extension (intel#1541) [SYCL][NFC] Temporarily disable sporadically failing test (intel#1533) [SYCL][NFC] Adjust codeowners for sycl directory (intel#1529) [SYCL] Fix processing of spec consts referenced twice (intel#1524) [SYCL] Use correct macro name in export.hpp (intel#1527) [Driver][NFC] Fix -help information for -Xs options (intel#1530) [SYCL][Doc] Add Graph Scheduler design documentation (intel#1457) [SYCL] Add diagnostics for long double in device code (intel#1512) [SYCL] Add a mutex to state-modifying program functions (intel#1204) [SYCL][Test] Add Devicelib tests (intel#1256) [SYCL] Refactor semantic checks for variable types (intel#1513)
|
I have the feeling it is related to internal Khronos discussion https://gitlab.khronos.org/sycl/Specification/issues/289 |
|
@StuartDAdams , please create an issue (or PR) with the definition of the |
Bug fix. __SYCL_EXPORT attribute missing from contextSetExtendedDeleter definition, meaning it was not publically visible.