-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL] Use scheduler in queue shortcuts to avoid waiting for deps #11758
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] Use scheduler in queue shortcuts to avoid waiting for deps #11758
Conversation
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
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.
Sycl-Graph LGTM
@aelovikov-intel Could you please have another look? |
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.
Stylistically it's good, the comments are very helpful as well.
I've reviewed the logic to the best of my abilities/knowledge in the area but the latter is very limited and I might have missed something. That said, you're the expert in the area and I don't know if anybody available could help with the review, so let's merge it.
Last commit was a comment typo fix, merging based on the previous test run. |
@KseniyaTikhomirova @sergey-semenov there is a post-commit failure for this PR which should be addressed, as the error has occurred in a more recent PR. The errors are:
|
@martygrant Thanks for letting me know, opened #12523 |
Fixes warnings introduced in #11758
It seems there are remaining warnings caused by this (https://github.com/intel/llvm/actions/runs/7701911776/job/20989034380):
|
Opened #12539 |
Fixes remaining warnings introduced in #11758
Commit #11758 added extra event as dependency to fix host task vs kernel dependencies for inorder queue. That logic missed part when we try to bypass scheduler for kernel with no dependencies. This commit fixes it and allows to bypass scheduler if previous commands are enqueued (no host task is involved). --------- Signed-off-by: Tikhomirova, Kseniya <[email protected]>
Some queue shortcuts attempt to bypass the scheduler to avoid the
overhead of creating a command node. In cases where dependencies
couldn't be passed to the backend directly, there was a blocking wait
during submission.
This patch changes that behavior to bypass the scheduler only if all
dependency events can be used directly: host task dependencies have been
completed, others have been enqueued and there are no cross-context
dependencies.