-
Notifications
You must be signed in to change notification settings - Fork 4
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
[SYCL][Graph] Clarify graph in-order and out-of-order properties #370
Conversation
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.
In the syclQueue
parameter description of
command_graph(const queue& syclQueue,
const property_list& propList = {});
There's the line "All other properties of the queue are ignored for the purposes of graph creation." Can we add a link there to the Queue Properties section, something like - "See the Queue Properties section for more general information about how queue properties interact with command_graph
objects."
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
I have added this suggestion. I'm a bit unsure about it though. The details of the queue properties are unrelated to this function (since all of them are ignored). Since this is a description of a parameter which ideally should be succint, it could be crossing the fine line between providing detail and bloating the spec. Don't have a huge objection though. |
queue have no effect on how the nodes within the graph are executed (e.g. the graph | ||
nodes without dependency edges may execute out-of-order even when using an in-order | ||
queue). For further information about how the properties of a queue affect graphs | ||
<<Queue Properties, see the section on Queue Properties>> |
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.
<<Queue Properties, see the section on Queue Properties>> | |
<<Queue Properties, see the section on Queue Properties>>. |
queue are ignored for the purposes of graph creation. | ||
queue are ignored for the purposes of graph creation. See the | ||
<<Queue Properties, Queue Properties>> section for more general information | ||
about how queue properties interact with command_graph objects. |
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.
about how queue properties interact with command_graph objects. | |
about how queue properties interact with `command_graph` objects. |
In the email where we got feedback about the in-order/out-of-order properties not being clear, this parameter description of the spec was a place the reader was looking and had questions about. I think the benefit of being able to point to a place with related information in a large spec is worth the extra sentence of verbosity. |
Upstream PR: intel#13681 |
No description provided.