Skip to content

[SYCL] Reuse discarded L0 events in scope of command list #7256

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

Merged
merged 35 commits into from
Nov 18, 2022
Merged

[SYCL] Reuse discarded L0 events in scope of command list #7256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8a377c7
[SYCL] Reuse discarded L0 events in scope of command list
againull Oct 20, 2022
3135fd3
Don't use separate structure to handle discarded events, use pi_event…
againull Nov 2, 2022
81c7a11
Reuse only ze_event handles, still create pi_event objects
againull Nov 3, 2022
0448cb8
Simplify barrier insertion in the beginning of command list
againull Nov 3, 2022
6541fb3
Enforce round robin behavior when reusing discarded events
againull Nov 3, 2022
9603869
Handle LastCommandInBatchHostVisible mode properly in discard_events …
againull Nov 3, 2022
3e72dd4
Remove redundant changes
againull Nov 3, 2022
9446140
Remove redundant changes
againull Nov 3, 2022
c8ceca3
Remove redundant code
againull Nov 3, 2022
d649e28
Revert "Simplify barrier insertion in the beginning of command list"
againull Nov 4, 2022
eb5b2cb
Always insert a barrier in the beginning of command list
againull Nov 4, 2022
1181324
Fix mistake
againull Nov 4, 2022
563b05f
Fix mistake
againull Nov 4, 2022
235e039
Add to cache in executeCommandList
againull Nov 4, 2022
a32aac6
Add guards and clarifying comments
againull Nov 8, 2022
41d518c
Merge remote-tracking branch 'origin/sycl' into reuse_events_in_cmd_l…
againull Nov 9, 2022
69d31e6
Merge remote-tracking branch 'origin/sycl' into reuse_events_in_cmd_list
againull Nov 16, 2022
7c9f819
Get rid of LastDiscardedEvent and its methods
againull Nov 16, 2022
75db6a1
Get rid of StartingBarrierEvents despite of affecting batching heuris…
againull Nov 16, 2022
3f33705
Make signalled event to be referenced by first command of the next cm…
againull Nov 16, 2022
ec1ba6b
Add comments and rename methods
againull Nov 16, 2022
0749827
Formatting
againull Nov 16, 2022
e46d7ac
Update comments
againull Nov 17, 2022
c60f72c
Rename LastCommandList->LastUsedCommandList
againull Nov 17, 2022
5bd9c19
Document env variable
againull Nov 17, 2022
849d1f6
Rename appendWaitAndResetIfLastEventDiscarded->resetDiscardedEvent
againull Nov 17, 2022
6e42f6d
Rename addEventToCache/getEventFromCache for readability
againull Nov 17, 2022
6cecfb4
Create helper query and add several comments
againull Nov 17, 2022
17b0775
Add query only mode for getQueueIndex
againull Nov 17, 2022
cacce1b
Add a comment about batch closure
againull Nov 17, 2022
b5a1c29
Remove unnecessary and confusing conditions
againull Nov 17, 2022
a8358f5
Revert "Remove unnecessary and confusing conditions"
againull Nov 17, 2022
e9aeab3
Fix _pi_queue::doReuseDiscardedEvents method
againull Nov 17, 2022
8bd411d
Add TODO to treat host proxy event as regular event
againull Nov 18, 2022
6f8d5ee
Add clarifying comments on conditions
againull Nov 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename LastCommandList->LastUsedCommandList
  • Loading branch information
@againull
againull committed Nov 17, 2022
commit c60f72c77245ab7dc77d4ab42b6c6774f6faae76
12 changes: 6 additions & 6 deletions sycl/plugins/level_zero/pi_level_zero.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ pi_result _pi_queue::executeCommandList(pi_command_list_ptr_t CommandList,
}
}

this->LastCommandList = CommandList;
this->LastUsedCommandList = CommandList;

if (!Device->useImmediateCommandLists()) {
// Batch if allowed to, but don't batch if we know there are no kernels
Expand Down Expand Up @@ -1821,7 +1821,7 @@ pi_result _pi_queue::executeCommandList(pi_command_list_ptr_t CommandList,

// Close the command list and have it ready for dispatch.
ZE_CALL(zeCommandListClose, (CommandList->first));
this->LastCommandList = CommandListMap.end();
this->LastUsedCommandList = CommandListMap.end();
// Offload command list to the GPU for asynchronous execution
auto ZeCommandList = CommandList->first;
auto ZeResult = ZE_CALL_NOCHECK(
Expand Down Expand Up @@ -2015,7 +2015,7 @@ pi_result _pi_queue::insertStartBarrierIfDiscardEventsMode(
// If current command list is different from the last command list then insert
// a barrier waiting for the last command event.
if (ReuseDiscardedEvents && isInOrderQueue() && isDiscardEvents() &&
CmdList != LastCommandList && LastCommandEvent) {
CmdList != LastUsedCommandList && LastCommandEvent) {
ZE_CALL(zeCommandListAppendBarrier,
(CmdList->first, nullptr, 1, &(LastCommandEvent->ZeEvent)));
LastCommandEvent = nullptr;
Expand Down Expand Up @@ -2089,10 +2089,10 @@ pi_result _pi_ze_event_list_t::createAndRetainPiZeEventList(
// to insert a barrier in the new command list waiting for that event.
auto QueueGroup = CurQueue->getQueueGroup(UseCopyEngine);
auto NextImmCmdList = QueueGroup.ImmCmdLists[QueueGroup.NextIndex];
Copy link
Contributor

Choose a reason for hiding this comment

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

it's fragile to use "NextIndex" directly here, maybe add a boolean to getQueueIndex that we just want to know the index (but not advance it)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, fixed as you suggested.

if (CurQueue->LastCommandList != CurQueue->CommandListMap.end() &&
CurQueue->LastCommandList != NextImmCmdList) {
if (CurQueue->LastUsedCommandList != CurQueue->CommandListMap.end() &&
CurQueue->LastUsedCommandList != NextImmCmdList) {
CurQueue->signalEventFromCmdListIfLastEventDiscarded(
CurQueue->LastCommandList);
CurQueue->LastUsedCommandList);
}
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion sycl/plugins/level_zero/pi_level_zero.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ struct _pi_queue : _pi_object {
// This data member keeps track of the last used command list and allows to
// handle switch of immediate command lists because immediate command lists
// are never closed unlike regular command lists.
pi_command_list_ptr_t LastCommandList = CommandListMap.end();
pi_command_list_ptr_t LastUsedCommandList = CommandListMap.end();

// Vector of 2 lists of reusable events: host-visible and device-scope.
// They are separated to allow faster access to stored events depending on
Expand Down