Unify with Codeplay Graph extension. #4
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
Merge [SYCL_EXT_CODEPLAY_GRAPHS](codeplaysoftware/standards-proposals#135) into SYCL_EXT_ONEAPI_GRAPH. This is a first cut at merging and follow-up changes to reconcile some differences will likely need to be made, either as commits to this branch before merging, or as subsequent PRs.
EwanC
commented
Oct 21, 2022
Makes the context accepted by the command_graph::finalize() method const, consistent with most other usages in the SYCL spec.
Bensuo
reviewed
Oct 28, 2022
This was referenced Nov 1, 2022
Remove the requirement for the graphs extension to support a graph constructed using a device queue being executed on another compatible queue. This may not be possible in the DPC++ prototype based on lazy queues.
Co-authored-by: Pablo Reble <[email protected]>
Break the graph terminology section down into a generic description of a graph, nodes, and edges. Followed by subsections for how that is realized in the explicit API and record & replay API individually.
Add wording on whether a user can combine the explicit graph building API with the record & replay API on the same modifiable graph object. It is specified as being allowed for the user to mix mechanisms, so long as the two mechanisms are used sequentially. However, it is forbidden if the mechanisms are interleaved and an exception must be thrown by the implementation. We decided this because it is not specified in the record & replay API whether commands are added to the graph eagerly during recording, or on `queue::end_recording`. When each mechanism is used sequentially however, the ordering of nodes being added is well defined. See Issue #11
Bensuo
approved these changes
Nov 7, 2022
Collaborator
Bensuo
left a comment
Bensuo
left a comment
There was a problem hiding this comment.
LGTM just a few minor comments.
reble
reviewed
Nov 7, 2022
reble
reviewed
Nov 8, 2022
* Move `make_edge` from a free function to a member of `command_graph<modifiable>`. * Remove USM limitation from spec wording, as it was an implementation detail.
reble
reviewed
Nov 8, 2022
reble
approved these changes
Nov 9, 2022
added 2 commits
November 9, 2022 12:13
* Abbreviate wording on record & replay edges around USM pointers, and add restriction that offsets into system allocations are not supported. * Fix typo
julianmi
reviewed
Nov 9, 2022
Collaborator
julianmi
left a comment
julianmi
left a comment
There was a problem hiding this comment.
LGTM. I have added minor comments.
* Make the use of preconditions and namespaces on the definitions of new functions consistent * Add diagram for topology of dotp example * Tweak language
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.
Merge SYCL_EXT_CODEPLAY_GRAPHS into SYCL_EXT_ONEAPI_GRAPH.
This is a first cut at merging and follow-up changes to reconcile some differences will likely need to be made, either as commits to this branch before merging, or as subsequent PRs.