-
Notifications
You must be signed in to change notification settings - Fork 30
[cleanup/tensor, step 3] Implement dpctl_capi loader in dpctl4pybind11 header file #932
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
Conversation
The class implements a singleton pattern that populates global variables exported by Cython CAPI functions via call to import_dpctl() and then populates internal struct members with pointers populated by Cython. This loads both Cython 'public api' functions, types, and integer values. dpctl::tensor::usm_ndarray and dpctl::memory::usm_memory classes, as well as type-casters for dpctl.SyclQueue <-> sycl::queue, dpctl.SyclDevice <-> sycl::device, etc. use dpctl_capi. The singleton patterns helps avoid static initialization order fiasco problem when using multiple translation units. Single dpctl_capi loads typenum values, the dpctl::tensor::detail::usm_ndarray_types struct no longer needs to be a singleton, and becomes an ordinary struct with a static method. tensor_py.cpp was changed to reflect that.
View rendered docs @ https://intelpython.github.io/dpctl/pulls/932/index.html |
Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_179 ran successfully. |
// Import Cython CAPI for dpctl | ||
import_dpctl(); | ||
|
||
this->Py_SyclDeviceType_ = &Py_SyclDeviceType; |
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.
All these are global pointers that are initialized by import_dpctl
? We need comments at least here. Moreover, can we create aliases for when we are initializing the static variables SyclDevice_GetDeviceRef
etc. that have a name such as gSyclDevice_GetDeviceRef
? The idea is to make the code easier to follow.
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.
Overall looks good.
Array API standard conformance tests for dpctl=0.14.0dev0=py310h8c27c75_178 ran successfully. |
Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞 |
This PR creates
dpctl::detail::dpctl_capi
singleton class that load Cython-generated CAPI functions implemented in Cython modules of dpctl, and re-exports them as class members.This PR removes another singleton
dpctl::tensor::detail::usm_ndarray_types
which is not a simple struct with trivial constructor.This moves all initialization of static variables exported and implemented by Python into a single place, thus avoiding static order initialization fiasco.