Skip to content

[SYCL] Make queue's non-USM event ownership temporary #1561

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

Merged
merged 3 commits into from
Apr 22, 2020

Conversation

sergey-semenov
Copy link
Contributor

@sergey-semenov sergey-semenov commented Apr 21, 2020

Previously, queues shared ownership of non-USM events with the
command dependency graph, so such events were not released until:

  1. The command associated with the event was cleaned up.
  2. The associated queue was destroyed or its wait method was called.
  3. All copies of the event on the application side were destroyed.

This patch changes queue's shared event pointers to weak ones in order
to drop the second requirement and allow releasing finished events earlier.

Signed-off-by: Sergey Semenov [email protected]

@sergey-semenov sergey-semenov requested a review from a team as a code owner April 21, 2020 13:28
@sergey-semenov sergey-semenov requested a review from s-kanaev April 21, 2020 13:28
@sergey-semenov
Copy link
Contributor Author

TBD: add a test

Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should have a test here for this case.

@s-kanaev
Copy link
Contributor

Previously, queues shared ownership of non-USM events with the
command dependency graph until queue::wait() was called. This patch
changes queue's shared event pointers to weak ones in order to allow
releasing finished events earlier.

What is the effect without this patch?

@sergey-semenov
Copy link
Contributor Author

Previously, queues shared ownership of non-USM events with the
command dependency graph until queue::wait() was called. This patch
changes queue's shared event pointers to weak ones in order to allow
releasing finished events earlier.

What is the effect without this patch?

I expanded the commit message a bit.

Signed-off-by: Sergey Semenov <[email protected]>
Signed-off-by: Sergey Semenov <[email protected]>
@sergey-semenov sergey-semenov requested a review from s-kanaev April 22, 2020 13:49
@sergey-semenov sergey-semenov requested a review from s-kanaev April 22, 2020 14:49
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bader bader merged commit 686e32b into intel:sycl Apr 22, 2020
@sergey-semenov sergey-semenov deleted the queueeventownership branch April 23, 2020 07:43
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 29, 2020
…versioning

* origin/sycl:
  [XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL (intel#1557)
  [SYCL] Initial ABI checks implementation (intel#1528)
  [SYCL] Support connection with multiple plugins (intel#1490)
  [SYCL] Add a new header file with the reduction class definition (intel#1558)
  [SYCL] Add test for SYCL kernels with accessor and spec constant (intel#1536)
  [SYCL][CUDA] Move interop tests (intel#1570)
  [Driver][SYCL] Remove COFF object format designator for Windows device compiles (intel#1574)
  [SYCL] Fix conflicting visibility attributes (intel#1571)
  [SYCL][DOC] Update the SYCL Runtime Interface document with design details (intel#680)
  [SYCL] Improve image accessors support on a host device (intel#1502)
  [SYCL] Make queue's non-USM event ownership temporary (intel#1561)
  [SYCL] Added support of rounding modes for non-host devices (intel#1463)
  [SYCL] SemaSYCL significant refactoring (intel#1517)
  [SYCL] Support 0-dim accessor in handler::copy(accessor, accessor) (intel#1551)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants