[SYCL][Graph] Backend integration and feature additions for SYCL Graphs (3/4) #10033
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
- Hook up graphs impl to PI/UR - Changes to scheduler and related classes to support graphs - New memory manager methods for command buffer copies - Device info query for command buffer support
Fix adding empty nodes to the graph results in two root nodes
…uffers (#238) Adds support required to handle: - host to buffer memcpy for 1d and 2d buffers - buffer to host memcpy for 1d and 2d buffers This commit also fixes a bug in buffer to buffer memcpy enabling to copy from/to buffers accessed with user-defined offsets. Adds basic tests to check all use-cases of mixed host/buffer memcpy, and buffer to buffer memcpy with user-defined offsets. Addresses Issue: #196
Fix some build errors I've seen with clang-17 after #238 was merged. 1) Arithmitic on void pointer, fixed by passing a `char*` rather than `void*` when an offset is needed ``` /home/ewan/Development/dpcpp/sycl/source/detail/memory_manager.cpp:1316:44: error: arithmetic on a pointer to void 1316 | SrcAccessRangeWidthBytes, DstMem + DstXOffBytes, Deps.size(), | ~~~~~~ ^ /home/ewan/Development/dpcpp/sycl/source/detail/memory_manager.cpp:1372:44: error: arithmetic on a pointer to void 1372 | DstAccessRangeWidthBytes, SrcMem + SrcXOffBytes, Deps.size(), | ~~~~~~ ^ 2) Fixup use of `numEventsInWaitList` when should be `numSyncPointsInWaitList` /home/ewan/Development/dpcpp/sycl/plugins/unified_runtime/ur/adapters/level_zero/ur_level_zero_command_buffer.cpp:611:13: error: use of undeclared identifier 'numEventsInWaitList' 611 | size, numEventsInWaitList, pSyncPointWaitList, pSyncPoint); ```
Gets rid of the warning "warning: moving a temporary object prevents copy elision"
Adding a empty node to a recorded in-order queue resulted in inconsistent dependencies between nodes. This patch fixes this issues and simplifies the adding of empty nodes. Unitests have been added to check node dependencies when recording an in_order queue with and without empty nodes.
Co-authored-by: Maxime France-Pillois [email protected]
EwanC
approved these changes
Jul 10, 2023
reble
approved these changes
Jul 11, 2023
Matching implementation change to specification PR #255
- Remove UR from createURCommandBuffers() name - Improve assert in findRealDeps()
reble
added a commit
to reble/llvm
that referenced
this pull request
Jul 12, 2023
reble
added a commit
to reble/llvm
that referenced
this pull request
Jul 13, 2023
steffenlarsen
approved these changes
Jul 14, 2023
|
Precommit failure in RHEL is unrelated and known. Merging this. |
veselypeta
pushed a commit
to veselypeta/llvm
that referenced
this pull request
Sep 21, 2023
…hs (3/4) (intel#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](intel#5626) A snapshot of the complete work can be seen in draft PR intel#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 intel#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 <[email protected]> Co-authored-by: Julian Miller <[email protected]> Co-authored-by: Ben Tracy <[email protected]> Co-authored-by: Ewan Crawford <[email protected]> Co-authored-by: Maxime France-Pillois <[email protected]>
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.
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
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:
Following Split PRs
Future follow-up PRs with the remainder of our work on the extension will include:
Authors
Co-authored-by: Pablo Reble [email protected]
Co-authored-by: Julian Miller [email protected]
Co-authored-by: Ben Tracy [email protected]
Co-authored-by: Ewan Crawford [email protected]
Co-authored-by: Maxime France-Pillois [email protected]