[SYCL][DOC] Graph API extensions#5626
Conversation
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
|
Interesting concepts! |
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
|
...not anymore. ;) |
You do not have an extension for this? :-) |
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_GRAPH.asciidoc
Outdated
Show resolved
Hide resolved
This reverts commit ef24789.
5956e20 to
d410644
Compare
keryell
left a comment
There was a problem hiding this comment.
This sounds like an interesting feature also for SYCL SC.
Co-authored-by: John Pennycook <john.pennycook@intel.com>
Co-authored-by: Ronan Keryell <ronan@keryell.fr>
- Make modifying kernel or host code in update undefined behaviour. - Use host task consistently in spec
gmlueck
left a comment
There was a problem hiding this comment.
Looks good. Just a couple small items below.
Simplify the definition of a node in the Record & Replay API. Move the wording around sub-graphs to the sub-graph section, and use "command" terminology rather than "kernel" to be more generic. Actions Gordon's feedback from: * Lack of clarity over inclusion of queue shortcut functions intel#5626 (comment) * Say "command" rather than "kernel" intel#5626 (comment)
Gordon and Greg left some spec feedback on our upstream PR, address the superficial comments.
Address [Steffen's feedback](intel#5626 (comment)) and move the `info::device::graph_support_level` to enum up a namespace level to `info::graph_support_level`. Using `info::device::graph_support` to return a `info::graph_support_level` is analogous with the `info::device::device_type` query in the main spec which returns a `info::device_type`. Or `info::device::local_mem_type` which returns a `info::local_mem_type`.
| to be inferred. | ||
|
|
||
| It is valid to combine these two mechanisms, however it is invalid to modify | ||
| a graph using the explicit API while that graph is currently being recorded to, |
There was a problem hiding this comment.
is it "... being recorded too". instead of ".. to" ?
There was a problem hiding this comment.
I think that "to" rather than "too" is correct, but rephrased this wording to "it is invalid to modify a graph using the explicit API while that graph is currently recording commands" to avoid any confusion over grammar.
* [SYCL][Doc] Change `graph_support_level` namespace Address [Steffen's feedback](intel#5626 (comment)) and move the `info::device::graph_support_level` to enum up a namespace level to `info::graph_support_level`. Using `info::device::graph_support` to return a `info::graph_support_level` is analogous with the `info::device::device_type` query in the main spec which returns a `info::device_type`. Or `info::device::local_mem_type` which returns a `info::local_mem_type`. * Add Steffen as a contributor
Simplify the definition of a node in the Record & Replay API. Move the wording around sub-graphs to the sub-graph section, and use "command" terminology rather than "kernel" to be more generic. Actions Gordon's feedback from: * Lack of clarity over inclusion of queue shortcut functions intel#5626 (comment) * Say "command" rather than "kernel" intel#5626 (comment)
[Review feedback](intel#5626 (comment)) questioned the phrase "graph is currently being recorded to" as to whether "to" should be "too". I don't think it should be "too", but rephrasing to "graph is currently recording any queues" avoids any confusion. Also updated the contributors to add Maxime and Jaime (who made the comment).
[Review feedback](intel#5626 (comment)) questioned the phrase "graph is currently being recorded to" as to whether "to" should be "too". I don't think it should be "too", but rephrasing to "graph is currently recording any queues" avoids any confusion. Also updated the contributors to add Maxime and Jaime (who made the comment).
- Change wording around undefined behaviour when creating a cycle with no checks - UB is created at point of adding the cycle, not at finalize.
…hs (3/4) (#10033) # Backend integration and feature additions for SYCL Graphs This is the third patch of a series that adds support for an [experimental command graph extension](#5626) A snapshot of the complete work can be seen in draft PR #9375 which has support all the specification defined ways of adding nodes and edges to the graph, including both Explicit and Record & Replay graph construction. The two types of nodes currently implemented are kernel execution and memcpy commands. See https://github.com/reble/llvm#implementation-status for the status of our total work. ## Scope This third patch focuses on integrating the graphs runtime with the backend support added in #9992 as well as any remaining runtime features and bug fixes, and should include no ABI-breaking changes: * Graphs runtime changes to use PI/UR command-buffers. * Various improvements to the Graphs runtime classes. * New memory manager methods for appending copies to a command-buffer. * Changes to the Scheduler and related CG classes to enable Graphs. * Device info query for command-graph support. * Minor changes to some runtime classes to enable Graphs. ## Following Split PRs Future follow-up PRs with the remainder of our work on the extension will include: * Add end-to-end tests for SYCL Graph extension. (4/4) * NFC changes - Design doc and codeowner update. ## Authors Co-authored-by: Pablo Reble <pablo.reble@intel.com> Co-authored-by: Julian Miller <julian.miller@intel.com> Co-authored-by: Ben Tracy <ben.tracy@codeplay.com> Co-authored-by: Ewan Crawford <ewan@codeplay.com> Co-authored-by: Maxime France-Pillois <maxime.francepillois@codeplay.com>
|
Precommit failure is a known issue (#10380) |
This extension is adding an API that targets separating definition and execution of a SYCL Kernel Graph. This patch contains specification only.