Skip to content
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][DOC] Adding design document for command graph extension #183

Merged
merged 5 commits into from
Jun 5, 2023

Conversation

reble
Copy link
Owner

@reble reble commented May 22, 2023

This is an initial draft of our implementation design documentation.

@reble reble added the Graph Implementation Related to DPC++ implementation and testing label May 22, 2023
@reble reble requested review from EwanC, Bensuo and julianmi May 22, 2023 22:40
Copy link
Collaborator

@EwanC EwanC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some other things I think it'd be worth documenting. Could be done in this PR, or in a follow-up:

  • Relationship with a handler for processing command-groups to use in a node - had to fix this for capturing of temporaries bug.
  • Sub-graphs - we create an empty node as an implementation detail to marshal all the dependencies.
  • breadth-first approach - Once [SYCL][Graph] Breadth-first schedule #182 is merged we should write down that implementation detail.

sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
@reble reble marked this pull request as ready for review May 30, 2023 20:19
@reble reble requested a review from EwanC May 30, 2023 20:20
Copy link
Collaborator

@EwanC EwanC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good starting point.

Would also like to have sections with my previous suggestions, and also taking the comment content from

/* Command-buffer Extension
The PI interface for submitting a PI command-buffer takes a list
of events to wait on, and an event representing the completion of
that particular submission of the command-buffer.
However, in `zeCommandQueueExecuteCommandLists` there are no parameters to
take a waitlist and also the only sync primitive returned is to block on
host.
In order to get the PI command-buffer enqueue semantics we want with L0
this adapter adds extra commands to the L0 command-list representing a
PI command-buffer.
Prefix - Commands added to the start of the L0 command-list by L0 adapter.
Suffix - Commands added to the end of the L0 command-list by L0 adapter.
These extra commands operate on L0 event synchronisation primitives used by
the command-list to interact with the external PI wait-list and PI return
event required for the enqueue interface.
The `pi_ext_command_buffer` class for this adapter contains a SignalEvent
which signals the completion of the command-list in the suffix, and
is reset in the prefix. This signal is detected by a new PI return
event created on PI command-buffer enqueue.
There is also a WaitEvent used by the `pi_ext_command_buffer` class
in the prefix to wait on any dependencies passed in the enqueue wait-list.
┌──────────┬────────────────────────────────────────────────┬─────────┐
│ Prefix │ Commands added to PI command-buffer by PI user │ Suffix │
└──────────┴────────────────────────────────────────────────┴─────────┘
┌───────────────────┬──────────────────────────────┐
Prefix │Reset signal event │ Barrier waiting on wait event│
└───────────────────┴──────────────────────────────┘
┌─────────────────────────────────────────┐
Suffix │Signal the PI command-buffer signal event│
└─────────────────────────────────────────┘
For a call to `piextEnqueueCommandBuffer` with an event_list `EL`,
command-buffer `CB`, and return event `RE` our implementation has to create
and submit two new command-lists for the above approach to work. One before
the command-list with extra commands associated with `CB`, and the other
after `CB`.
Command-list created on `piextEnqueueCommandBuffer` to execution before `CB`:
┌───────────────────────────────────────────────────────────┐
│Barrier on `EL` than signals `CB` WaitEvent when completed │
└───────────────────────────────────────────────────────────┘
Command-list created on `piextEnqueueCommandBuffer` to execution after `CB`:
┌─────────────────────────────────────────────────────────────┐
│Barrier on `CB` SignalEvent that signals `RE` when completed │
└─────────────────────────────────────────────────────────────┘
*/

But can write all that up in a follow-up PR.

sycl/doc/design/CommandGraph.md Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
sycl/doc/design/CommandGraph.md Outdated Show resolved Hide resolved
Co-authored-by: Ewan Crawford <[email protected]>
Co-authored-by: Ben Tracy <[email protected]>
@reble reble merged commit 19024c1 into sycl-graph-develop Jun 5, 2023
@Bensuo Bensuo deleted the pablo/design-doc branch August 7, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph Implementation Related to DPC++ implementation and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants