-
Notifications
You must be signed in to change notification settings - Fork 794
[SYCL] Enqueue dependencies of blocked command #2543
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
Conversation
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
|
Need to fix unit-test |
|
LGTM. I think this change likely will not pessimize the problem which was fixed by bc8f0a4. |
Is there any benchmark to check this assumption/hypothesis? |
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
No, there is not. Only unit tests. |
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
|
I think that we should prefer #2561 instead of this one. |
|
Closing this PR as there is a more preferable solution in #2561 |
Document how llvm intrinsics are lowered by SPIRV-LLVM-Translator. Signed-off-by: Lu, John <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@45154b58186cb22
Let's have this part of dependency graph:
EC1andEC2are of helper typeEmptyCommand. Both are blocked.C1andC2are of any other type which represents some meaningful operation i.e. the one that actually does something useful, likeExecCGCommandwith a host task orUpdateHostReqCommandfor host accessor.This part of graph is result of two submissions: the first one adds
EC1 -> C1and the second addsEC2 -> C2 -> EC1.The submission process in both cases adds the corresponding
Cxcommand in first place and then adds the empty commandECx. WhenCxis added to the graph it's also added to list of leaves for corresponding memory record. Now, whenECxis added to graph it removes correspondingCxfrom list of leaves.ECxis added to the list instead ofCx. Blocked state ofECxprevents it from being enqueued.Upon the second submission,
C2isn't going to be enqueued right away as it depends on blockedEC1.When
C1finishes (whether it is a host task or host accessor's update command) SYCL RT will unblockEC1and trigger enqueue process for leaves of memory recordsC1was depending by. Eventually, RT will enqueue theEC1which will set its event into complete state. Though,EC2isn't going to be enqueued due to it's blocked state. Neither does its dependencies.PR with test: #2540
PR with comment fix: #2541
A more workaroundey way is available at #2542