[SYCL] Visit explicit dependencies during finished command cleanup #2265
Conversation
|
I'll take a look at the test failures |
rbegam
left a comment
rbegam
left a comment
There was a problem hiding this comment.
LGTM. Please fix the test fails.
Previously, only implicit dependencies (represented by DepDesc) of each command node were visited during finished command cleanup. That meant that command nodes with no implicit dependencies were never cleaned up unless waited for directly (via queue::wait or event::wait on that event). This patch changes graph traversal during cleanup, so that all command dependencies are visited, whether represented by DepDesc or not. Signed-off-by: Sergey Semenov <[email protected]>
sergey-semenov
force-pushed
the
handleunwaitedcase
branch
from
a0c6a8f to
ad9449a
Compare
sergey-semenov
changed the title
|
Sorry for the delay. I ended up switching to a different solution due to issues when cleaning up commands during graph builder destruction. @rbegam please, take a look. |
|
An alternate solution would be to kill off command nodes without explicit dependencies after their enqueuing. That would address more problematic cases than this change alone. Moving this PR to draft for now. |
|
It looks like now you have accumulated some experience about this foggy unexplored part of the SYCL specification. |
|
Superseded by #2492 |
…2265) ImageOpMask pointer was passed to closure as reference. Closure is executed outside of original stack frame which renders reference invalid. Fixes some ASan errors from issue #2233. Original commit: KhronosGroup/SPIRV-LLVM-Translator@29d4cd3
3 participants
Previously, only implicit dependencies (represented by DepDesc) of each
command node were visited during finished command cleanup. That meant
that command nodes with no implicit dependencies were never cleaned up
unless waited for directly (via queue::wait or event::wait on that
event). This patch changes graph traversal during cleanup, so that all
command dependencies are visited, whether represented by DepDesc or not.
Signed-off-by: Sergey Semenov [email protected]