[SYCL] Fix handling of discarded events in preview lib #19005
Conversation
igchor
force-pushed
the
discard_event_fix
branch
from
f7b6b96 to
fc67f5c
Compare
|
I think the PR description has to go into inline code comments somewhere (although I might be wrong here). IMO, Am I missing anything why that reasoning shouldn't apply here? |
igchor
force-pushed
the
discard_event_fix
branch
from
fc67f5c to
8ba9fb4
Compare
Well, I'm not sure if it makes sense to put a comment describing a bug that has been fixed but I agree that we should describe why we need to mark the event as discarded in addCG(). Done (altough I made the description a bit more general than what I put in the PR description). |
sycl/source/handler.cpp
Outdated
| // For preview mode, handler.finalize() is expected to return nullptr | ||
| // if the event is discarded. |
There was a problem hiding this comment.
This should be moved/copied to handler.hpp, IMO.
Is there a reasonable place to assert for this as well?
There was a problem hiding this comment.
Move to handler.hpp. Yes, we can assert that in queue, inside parseEvent(). Done.
The queue implementation assumed that handler.finalize() will return either a valid (non-discarded) event or nullptr (that's why parseEvent is nop in preview mode). However, that was not always true. It was possible for scheduler to only mark an event as discarded after handler.finalize() completed (during actual command execution). This resulted in discarded event being stored in LastEventPtr in the queue and calls to wait() on that discarded event (which is not allowed) in MT scenarios. Fix this, by modyfing addCG() to mark the event as discarded immediately. This allows handler to return nullptr for preview mode.
Co-authored-by: aelovikov-intel <[email protected]>
The queue implementation assumed that handler.finalize() will return either a valid (non-discarded) event or nullptr (that's why parseEvent is nop in preview mode).
However, that was not always true. It was possible for scheduler to only mark an event as discarded after handler.finalize() completed (during actual command execution). This resulted in discarded event being stored in LastEventPtr in the queue and calls to wait() on that discarded event (which is not allowed) in MT scenarios.
Fix this, by modyfing addCG() to mark the event as discarded immediately and to return nullptr is the event is discarded.