-
Notifications
You must be signed in to change notification settings - Fork 808
[SYCL] fix for __sycl_unregister_lib() on Windows and tests #19633
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
[SYCL] fix for __sycl_unregister_lib() on Windows and tests #19633
Conversation
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
PVC failures is unrelated: #19662 |
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
5a5c73f
to
c6afa75
Compare
Signed-off-by: Chris Perkins <chris.perkins@intel.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Looks like a solid solution to me! 🚀
@intel/dpcpp-tools-reviewers - ping for review |
M.getOrInsertFunction("__sycl_register_lib", RegTargetTy); | ||
|
||
// `atexit` takes a `void(*)()` function pointer. In LLVM IR, this is | ||
// typically represented as `i32 (ptr)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im not sure i understand this comment, to me it seems like the abi would be different so we can't use i32 (ptr)
to represent void(*)()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simplified it. I just wanted to describe atexit.
__sycl_unregister_lib()
is not being called on Windows when using shared libraries due to a limitation in LLVM/clang To work around this, on Windows we register both__sycl_register_lib()
and anstd::atexit
handler that will call__sycl_unregister_lib()
. Further, it was discovered that freeing of the device globals during device images destruction is duplicate and unnecessary. The~context_impl
destructor handles that (and handles it correctly, because a context is needed to free USM memory). So we remove the unneeded duplication. Adding a test that stresses__sycl_unregister_lib()
and makes sure there are no resource leaks