-
Notifications
You must be signed in to change notification settings - Fork 29
Add support for events, programs, and kernel submission to dpctl. #80
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
Add support for events, programs, and kernel submission to dpctl. #80
Conversation
diptorupd
commented
Sep 30, 2020
* New dpctl C API wrappers for sycl::event, sycl::kernel and sycl::program. * Wrappers to create an OpenCL interoperability program from either string or SPIR-V IL files. * Support for submitting an interoperability kernel to a sycl::queue. We support using either a range or an nd_range to submit the kernel. * Event wait function is now exposed via the C API. * New test cases for the C API. * The dpctl Cython wrapper now supports program creation and kernel submission. * Other refactoring to standardize function names, attribute names, comments.
Add new line at end of file.
* Remove empty doxygen comment blocks * Fix indentation * Fix grammatical typos
390c747
to
e48ba39
Compare
- The default queue points to a Level-0 device is Level-0 drivers are installed. However, we always compile our program for OpenCL and this causes a crash if the test kernels are submitted to the default queue. Fixing it by selecting the GPU:0 queue that is known to be an OpenCL device. A follow up commit will fix this in a proper way.
cdef void DPPLQueue_Memcpy (const DPPLSyclQueueRef Q, | ||
void *Dest, const void *Src, size_t Count) \ | ||
except + | ||
void *Dest, const void *Src, size_t Count) |
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.
@diptorupd as I remember you wanted to make Memcpy to return SyclEvent. In the next PR?
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, I will open a follow up. Refer #84
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.
LGTM.
/*! | ||
* @brief Create a Sycl program from an OpenCL SPIR-V binary file. | ||
* | ||
* Sycl 1.2 does expose any method to create a sycl::program from a SPIR-V IL |
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.
Did you mean 'does not expose'?
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.
good catch. Fixed now.
* OpenCL kernel. | ||
* | ||
* The feature to create a Sycl kernel from a SPIR-V IL binary will be available | ||
* in Sycl 2.0, at which point we may deprecate this function. |
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.
It is better to say, "at which point this function may become deprecated."
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 changed the comment. Thanks.
* @param cgh My Param doc | ||
* @param Arg My Param doc | ||
*/ | ||
bool set_kernel_arg (handler &cgh, size_t idx, __dppl_keep void *Arg, |
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.
We will eventually need to support complexes as well.
This is not straightforward with ctypes
: https://stackoverflow.com/questions/13373291/complex-number-in-ctypes
So down the road we might just introduce our own scalar type in dpctl
.
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.
Opened #85 to track the issue.
…ctl into feature/program_interfacev2