[SYCL][Graph] Add a shortcut for adding leaves as dependencies #339
Conversation
This file contains 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
mfrancepillois
added
the
Graph Implementation
EwanC
reviewed
Nov 2, 2023
sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc
Outdated
Show resolved
Hide resolved
Bensuo
reviewed
Nov 2, 2023
sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc
Outdated
Show resolved
Hide resolved
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
EwanC
approved these changes
Nov 2, 2023
sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc
Outdated
Show resolved
Hide resolved
Bensuo
reviewed
Nov 3, 2023
Bensuo
reviewed
Nov 3, 2023
Comment on lines
491
to
492
| adding all leaves of a graph as dependencies. | ||
| Therefore the created node will depend on all the current leaves in the graph. |
There was a problem hiding this comment.
Suggested change
| adding all leaves of a graph as dependencies. | |
| Therefore the created node will depend on all the current leaves in the graph. | |
| adding all leaves of a graph as dependencies of the newly created node. |
There was a problem hiding this comment.
Do we need to specify when we actually evaluate the node dependencies? For a multi-threaded scenario the set of leaves wouldn't necessarily be constant.
There was a problem hiding this comment.
I merged the two sentences in one but I kept the notion of "current" leaves.
There was a problem hiding this comment.
IMO, in a multi-threading scenario this property could lead to an indeterminate graph since the order of adding nodes is not deterministic, but it cannot lead to an invalid state of the graph.
Bensuo
approved these changes
Nov 6, 2023
Adds a node property that allows users to easily add all leaves of a graph as dependencies when creating a node with the explicit API. Updates the spec with this new feature. Adds unitests that check this behaviour.
mfrancepillois
force-pushed
the
maxime/depends-on-leaves
branch
from
9807b57
to
2ddc424
Compare
|
Upstream PR intel#11855 |
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.
Adds a node property that allows users to easily add all leaves of a graph as dependencies when creating a node with the explicit API.
Updates the spec with this new feature.
Adds unitests that check this behaviour.