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.

Sign up for GitHub

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

Jump to bottom

[L0] Enable Copy engine support with in-order command lists #2636

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[L0] Enable Copy engine support with in-order command lists #2636

wants to merge 9 commits into from

Conversation

konradkusiak97
Copy link
Contributor

@konradkusiak97 konradkusiak97 commented Jan 29, 2025 •
edited
Loading

Currently, copy engine is only available to use for certain commands in the out-of-order command-buffer path. This PR adds required synchronization between the 2 command lists, when both are in-order. Hence, it enables using the copy engine with in-order path.

For the immediateAppend path, this only works for now if the environment variable UR_L0_USE_DRIVER_COUNTER_BASED_EVENTS=0 is set.

intel/llvm PR

@konradkusiak97 konradkusiak97 requested a review from a team as a code owner January 29, 2025 12:21
@konradkusiak97 konradkusiak97 requested a review from EwanC January 29, 2025 12:21
@konradkusiak97 konradkusiak97 force-pushed the addInOrderCopyEngineSupport branch from b47d4ef to 85f8911 Compare January 29, 2025 12:23
@github-actions github-actions bot added level-zero L0 adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Jan 29, 2025
@konradkusiak97 konradkusiak97 mentioned this pull request Jan 29, 2025
Merged
EwanC
EwanC reviewed Jan 31, 2025
View reviewed changes
fabiomestre
fabiomestre reviewed Jan 31, 2025
View reviewed changes
@konradkusiak97
Copy link
Contributor Author

I've refactored quite a bit this PR hopefully addressing all the comments but if you could give it another look, it'd be great.

EwanC
EwanC approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@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.

LGTM, but would like Fábio to check this over as well before merging

@konradkusiak97 konradkusiak97 force-pushed the addInOrderCopyEngineSupport branch 2 times, most recently from 285ea5e to 1629bbf Compare February 12, 2025 12:35
@EwanC EwanC added ready to merge Added to PR's which are ready to merge and removed ready to merge Added to PR's which are ready to merge labels Feb 13, 2025
@martygrant
Copy link
Contributor

Unified Runtime -> intel/llvm Repo Move Notice

Information

The source code of Unified Runtime has been moved to intel/llvm under the unified-runtime top-level directory,
all future development will now be carried out there. This was done in intel/llvm#17043.

The code will be mirrored to oneapi-src/unified-runtime and the specification will continue to be hosted at oneapi-src.github.io/unified-runtime.

The contribution guide has been updated with new instructions for contributing to Unified Runtime.

PR Migration

All open PRs including this one will be labelled auto-close and shall be automatically closed after 30 days.
To allow for some breathing space, this automation will not be enabled until next week (27/02/2025).

Should you wish to continue with your PR you will need to migrate it to intel/llvm.
We have provided a script to help automate this process.

This is an automated comment.

@konradkusiak97
Enabled in order for copy engine withoou sync so far
990394f
@konradkusiak97
Enabled the use of Copy engine with in order command lists
a27726c 
Added a createSyncPointBetweenCopyAndCompute() function which appends a
signal event to the previous command list to wait on. This can happen in
a scenario when we use Compute -> Copy -> Compute, when the last Compute
needs to wait for Copy to preserve in-order behaviour. For the
immediateAppend path, this works only if the counter-based events are
explicitly not used.
@konradkusiak97
Apply review comments
8f49c1d
@konradkusiak97
Removed the need for isFirstNode
fd99073
@konradkusiak97
Simplified logic inside the SyncBetween function
52370f8
@konradkusiak97
Removed unnecessary arguments and made the function accept a vector o…
27de5f3 
…f waitEvents
@konradkusiak97
Added extra function for code reuse in prefetch and adviceUSM and cle…
ee44744 
…aned up
@konradkusiak97
Removed IsCopy
3a1c96a
@konradkusiak97
Moved calls to createSyncPointBetweenCopyAndCompute() inside createSy…
060afd5 
…ncPointAndGetZeEvents()
@konradkusiak97 konradkusiak97 force-pushed the addInOrderCopyEngineSupport branch from 1629bbf to 060afd5 Compare February 24, 2025 19:23
@martygrant
Copy link
Contributor

Unified Runtime -> intel/llvm Repo Move Notice

Following on from the previous notice, we have now enabled workflows to automatically label and close PRs because the Unified Runtime source code has moved to intel/llvm.

This PR has now been marked with the auto-close label and will be automatically closed after 30 days.

Please review the previous notice for more information, including assistance with migrating your PR to intel/llvm.

Should there be a reason for this PR to remain open, manually remove the auto-close label.

This is an automated comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EwanC EwanC EwanC approved these changes

@fabiomestre fabiomestre fabiomestre approved these changes

Assignees
No one assigned
Labels
auto-close command-buffer Command Buffer feature addition/changes/specification level-zero L0 adapter specific issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@konradkusiak97 @martygrant @EwanC @fabiomestre