-
Notifications
You must be signed in to change notification settings - Fork 33
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
Feature/improved sycl queue support #1083
Conversation
fd04d93
to
d7c2405
Compare
- Creates a new data model DpnpNdArrayModel to represent DpnpNdArray type objects natively. The data model differs from numba's ArrayModel by having an extra member to store a sycl::queue pointer. - Introduces a _usmarraystruct.h header to define the C struct for the DpnpNdArrayModel. - Renames numba_dpex.core.datamodel.models.ArrayModel to USMArrayModel. - Updates kernel launcher and parfor lowering functions to account for the new data model.
5fd5074
to
7b9cccd
Compare
- Updates the *_like overloads to extract the sycl_queue for the input array. It was not possible previously as the sycl_queue attribute was not present. - Update unit tests. - Add new unit tests.
7b9cccd
to
7a5a317
Compare
@chudur-budur The changes are good to go and ready for review. |
ping @chudur-budur |
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.
Looks good but there are some issues and questions that I think need to be addressed before merge.
numba_dpex/dpnp_iface/_intrinsic.py
Outdated
builder, | ||
sig, | ||
args, | ||
sycl_queue_arg_pos=sycl_queue_arg_pos, |
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.
Can we just declare these two values like a constant at the top of the file and use it everywhere?
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.
Generally, I will like to avoid global variables. I also wish to find a way to not have to do the hard coded variables, just could not find a way out of properly using keyword args.
- Addresses the review comment to pass required arguments to _get_queue_ref explicitly.
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
…_support Feature/improved sycl queue support 81af300
Adds an extra member to the data model used for a DpnpNdArray typed object that will store a
DpctlSyclQueueRef
pointer. The change makes it possible to extract the sycl queue pointer used to allocate a dpnp.ndarray from the dpnp.ndarray native representation inside numba-dpex.With the change all dpnp
*_like
constructor overloads can now use the exact same queue object from the passed in dpnp.ndarray argument if needed.