-
Notifications
You must be signed in to change notification settings - Fork 795
[SYCL][Graph] Add support for handler-less graph submission #20690
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
base: sycl
Are you sure you want to change the base?
Conversation
5dc81c5 to
098a334
Compare
098a334 to
3617bc9
Compare
e3a6ba2 to
da7910d
Compare
da7910d to
7642921
Compare
| hasCommandGraph() ? getCommandGraph().get() : nullptr, | ||
| detail::CGType::Kernel); | ||
| hasCommandGraph() ? getCommandGraph().get() : nullptr, Type); | ||
| } else if (inOrder && MNoLastEventMode && CommandFuncContainsHostTask) { |
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.
I think that checking MNoLastEventMode here works (as this is what the handler path does). One of my longer term goals for the handler-less path however, was to avoid using MNoLastEventMode synchronization flag. Instead of checking that flag, my plan was to rely on the LastEvent being set. If SchedulerBypass is true, then we just unset it, since the lower layers take care of the ordering. If it is false, then we have to set it, since the kernel submission to the scheduler requires ordering on the SYCL layer.
The MNoLastEventMode is still used in queue_impl::wait, which is generic, so this may be a subject to future changes. The current handler-less path however updates that flag only for compatibility with the handler path.
I think there are three cases here:
- The LastEvent is set, then we simply add the dependency
- The LastEvent is not set - we are in the No Last Event Mode - we have to insert the barrier
- The LastEvent is not set - nothing was submitted to this queue yet - then we might use the MEmpty flag? But probably we have to move the MEmpty.store() to after this check.
Please let me know if this thinking makes sense and if it would be possible to replace the use of the MNoLastEventMode flag here.
submit_graph_direct_with_event_implandsubmit_graph_direct_without_event_implto the ABI which invoke a handler-less path for graph submission forqueue::ext_oneapi_graphand the free functionexecute_graph.submit_directutility to be more general: support submissions which may contain host task, move scheduler bypass logic to callback functor, and parameterize submission CGType.