-
Notifications
You must be signed in to change notification settings - Fork 744
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] Add Level-Zero interop with specification of ownership for Queue. #4066
Conversation
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
sycl/include/CL/sycl/detail/pi.h
Outdated
@@ -36,10 +36,11 @@ | |||
// 2. A number of types needed to define pi_device_binary_property_set added. | |||
// 3. Added new ownership argument to piextContextCreateWithNativeHandle. | |||
// 4. Add interoperability interfaces for kernel. | |||
// 5. Added new ownership argument to piextQueueCreateWithNativeHandle. |
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.
The numbering (5.) is not in sync with PI versions
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.
should it match with the major/minor version? I've followed the inst from line#25 above and increased both by 1.
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.
Following instructions in line #25 is all fine. Just let's have the comments about the changes tell what was the version where it was made. So, in your case I'd say have "5" changed to "4.6". And, please add a comment about this numbering in the comment above.
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.
done.
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
ping @smaslov-intel |
|
||
if (Queue->OwnZeCommandQueue) { | ||
ZE_CALL(zeCommandQueueDestroy, (Queue->ZeComputeCommandQueue)); | ||
Queue->ZeComputeCommandQueue = nullptr; |
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 think we should still null-ify these, even if we don't own the handles
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.
done.
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.
few more comments
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
Signed-off-by: rehana begam <[email protected]>
hi @smaslov-intel, could you please review? the pre-commit fail is for a time-out and I've already restarted it. thanks. |
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
I re-ran the precommit tests and there are no time outs. all tests are passing. |
This parameter was introduced in intel#4066 but apparently didn't make it in the ROCm plugin.
This parameter was introduced in #4066 but apparently didn't make it in the ROCm plugin.
The L0 backend interop functions take the ownership of the passed L0 handles. This is problematic for some cases as the users can not use the original handle when they go out of scope and destroyed.
This PR provides a way for users of SYCL - L0 interop to retain the ownership of the L0 handles for the Queue.
Signed-off-by: rehana begam [email protected]