[SYCL] Do not store last event for in-order queues #18277
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
igchor
force-pushed
the
in_order_queue_no_event
branch
from
ce2652e to
dac0398
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
dac0398 to
375b895
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
a7b84a1 to
ce4ac8a
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
ce4ac8a to
7ce77ca
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
7ce77ca to
39c5740
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
39c5740 to
1e2bf93
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
e94b095 to
e6158b5
Compare
igchor
force-pushed
the
in_order_queue_no_event
branch
from
e6158b5 to
156335d
Compare
unless Host Tasks are used. Without Host Tasks, we can just rely on UR for ordering. Having no last event means that ext_oneapi_get_last_event() needs to submit a barrier to return an event to the user. Similarly, ext_oneapi_submit_barrier() now always submits a barrier, even for in-order queues. Whenever Host Tasks are used we need to start recording all events. This is needed because of how kernel submission synchronizes with Host Tasks. With a following scenario: q.host_task(); q.submit_kernel(); q.host_task(): The kernel won't even be submitted to UR until the first Host Task completes. To properly synchronize the second Host Task we need to keep the event describing kernel submission.
For opencl, always store the last event to support queue_empty(), just don't use it for synchronization
|
@intel/llvm-gatekeepers I belive this is ready to be merged. I'm not really happy about having to introduce a separate path for opencl (for queue_empty()) but I'll address it in separate PR. |
|
@intel/llvm-gatekeepers could you please merge this? |
|
We still need a review from @intel/sycl-graphs-reviewers |
|
Jenkins precommit is a known infrastructural issue. |
igchor
added a commit
to igchor/llvm
that referenced
this pull request
May 21, 2025
unless Host Tasks are used. Without Host Tasks, we can just rely on UR for ordering. Having no last event means that ext_oneapi_get_last_event() needs to submit a barrier to return an event to the user. Similarly, ext_oneapi_submit_barrier() now always submits a barrier, even for in-order queues. Whenever Host Tasks are used we need to start recording all events. This is needed because of how kernel submission synchronizes with Host Tasks. With a following scenario: q.host_task(); q.submit_kernel(); q.host_task(): The kernel won't even be submitted to UR until the first Host Task completes. To properly synchronize the second Host Task we need to keep the event describing kernel submission.
uditagarwal97
pushed a commit
that referenced
this pull request
Jun 18, 2025
Optimizes the `enqueue()` function of sycl graphs to bypass the scheduler whenever possible and avoid creating events when not needed. * Refactors the executable graph `enqueue()` to have different paths depending on workload: * The direct path will be used when there are no host-tasks or accessor requirements in the graph and the execution dependencies are considered safe to bypass the scheduler. * The scheduler path will be used when there are requirements in the graph but no host-tasks or, if the execution dependencies require using the scheduler. * The multiple partitions path will be used when the graph contains `host-tasks` which requires scheduling multiple graph partitions. The implementation was also changed to avoid adding unnecessary event dependencies to partition executions and avoiding copying `CGData` when possible. * Extends the changes in #18277 to sycl graphs. This means that no implicit events will be created when using in-order queues and graphs without `host-tasks`. Also updates the handler to only request events from the graph `enqueue()` when they are needed.
github-actions bot
pushed a commit
to oneapi-src/unified-runtime
that referenced
this pull request
Jun 19, 2025
Optimizes the `enqueue()` function of sycl graphs to bypass the scheduler whenever possible and avoid creating events when not needed. * Refactors the executable graph `enqueue()` to have different paths depending on workload: * The direct path will be used when there are no host-tasks or accessor requirements in the graph and the execution dependencies are considered safe to bypass the scheduler. * The scheduler path will be used when there are requirements in the graph but no host-tasks or, if the execution dependencies require using the scheduler. * The multiple partitions path will be used when the graph contains `host-tasks` which requires scheduling multiple graph partitions. The implementation was also changed to avoid adding unnecessary event dependencies to partition executions and avoiding copying `CGData` when possible. * Extends the changes in intel/llvm#18277 to sycl graphs. This means that no implicit events will be created when using in-order queues and graphs without `host-tasks`. Also updates the handler to only request events from the graph `enqueue()` when they are needed.
kbenzie
pushed a commit
to oneapi-src/unified-runtime
that referenced
this pull request
Jun 19, 2025
Optimizes the `enqueue()` function of sycl graphs to bypass the scheduler whenever possible and avoid creating events when not needed. * Refactors the executable graph `enqueue()` to have different paths depending on workload: * The direct path will be used when there are no host-tasks or accessor requirements in the graph and the execution dependencies are considered safe to bypass the scheduler. * The scheduler path will be used when there are requirements in the graph but no host-tasks or, if the execution dependencies require using the scheduler. * The multiple partitions path will be used when the graph contains `host-tasks` which requires scheduling multiple graph partitions. The implementation was also changed to avoid adding unnecessary event dependencies to partition executions and avoiding copying `CGData` when possible. * Extends the changes in intel/llvm#18277 to sycl graphs. This means that no implicit events will be created when using in-order queues and graphs without `host-tasks`. Also updates the handler to only request events from the graph `enqueue()` when they are needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
unless Host Tasks are used.
Without Host Tasks, we can just rely on UR for ordering. Having no last event means that ext_oneapi_get_last_event() needs to submit a barrier to return an event to the user. Similarly, ext_oneapi_submit_barrier() now always submits a barrier, even for in-order queues.
Whenever Host Tasks are used we need to start recording all events. This is needed because of how kernel submission synchronizes with Host Tasks. With a following scenario:
q.host_task();
q.submit_kernel();
q.host_task():
The kernel won't even be submitted to UR until the first Host Task completes. To properly synchronize the second Host Task we need to keep the event describing kernel submission.