-
Notifications
You must be signed in to change notification settings - Fork 769
[SYCL] Exit early while trying to enqueue blocked tasks #2347
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] Exit early while trying to enqueue blocked tasks #2347
Conversation
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 need to fix the test failures revealed in the CI. |
Co-authored-by: sergei <57672082+s-kanaev@users.noreply.github.com>
32c51b7
to
10963c4
Compare
I fixed CI failures. See 2ec35d1. |
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 changes look good. Only a single comment and a question.
// We have such a graph: | ||
// | ||
// A | ||
// / | \ | ||
// B C D | ||
// | ||
// If C is blocked, we should not try to enqueue D. |
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.
Do I understand it correctly that A
depends on B
, C
and D
? Why can't we enqueue D
then?
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 discussed it with @romanovvlad and AFAIU, in the current design there are no benefits of including D
, as it is likely to be blocked as well.
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
@bader, please merge if no objections. |
This patch introduces two use-cases for host-task implementation. The issue of the first use-case (`test3`) was about performance and a long-run/wait of host-taske depening on another host-task. This issue rooted in lots of enqueue calls for blocked command and was fixed in bc8f0a4 (#2347). The second use-case shows a deadlock situation when host-task `A` depends on the blocked one `B` via call `handler::depends_on()`. The `A` host-task won't be enqueued as it's `EmptyCommand` is in blocked state. The test for this use-case is disable now and will be enabled when the deadlock is fixed. The deadlock will be fixed in follow-up patch.
It should have tested DebugInfoNone base type Signed-off-by: Sidorov, Dmitry <dmitry.sidorov@intel.com> Original commit: KhronosGroup/SPIRV-LLVM-Translator@e0aef72fee42e0a
No description provided.