Skip to content

[SYCL][L0] Optimize barrier for in-order queue #9446

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

Merged
merged 4 commits into from
May 22, 2023
Merged
Changes from all commits
Commits
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
69 changes: 56 additions & 13 deletions sycl/plugins/level_zero/pi_level_zero.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,13 @@ static const zeCommandListBatchConfig ZeCommandListBatchCopyConfig = [] {
return ZeCommandListBatchConfig(IsCopy{true});
}();

// Control if wait with barrier is implemented by signal of an event
// as opposed by true barrier command for in-order queue.
static const bool InOrderBarrierBySignal = [] {
const char *UrRet = std::getenv("UR_L0_IN_ORDER_BARRIER_BY_SIGNAL");
return (UrRet ? std::atoi(UrRet) : true);
}();

_pi_queue::_pi_queue(std::vector<ze_command_queue_handle_t> &ComputeQueues,
std::vector<ze_command_queue_handle_t> &CopyQueues,
pi_context Context, pi_device Device,
Expand Down Expand Up @@ -5744,20 +5751,56 @@ pi_result piEnqueueEventsWaitWithBarrier(pi_queue Queue,
std::scoped_lock<ur_shared_mutex> lock(Queue->Mutex);

// Helper function for appending a barrier to a command list.
auto insertBarrierIntoCmdList =
[&Queue](pi_command_list_ptr_t CmdList,
const _pi_ze_event_list_t &EventWaitList, pi_event &Event,
bool IsInternal) {
if (auto Res = createEventAndAssociateQueue(
Queue, &Event, PI_COMMAND_TYPE_USER, CmdList, IsInternal))
return Res;
auto insertBarrierIntoCmdList = [&Queue](
pi_command_list_ptr_t CmdList,
const _pi_ze_event_list_t &EventWaitList,
pi_event &Event, bool IsInternal) {
// For in-order queue and empty wait-list just use the last command
// event as the barrier event.
if (Queue->isInOrderQueue() && !EventWaitList.Length &&
Queue->LastCommandEvent && !Queue->LastCommandEvent->IsDiscarded) {
PI_CALL(piEventRetain(Queue->LastCommandEvent));
Event = Queue->LastCommandEvent;
return PI_SUCCESS;
}

Event->WaitList = EventWaitList;
ZE_CALL(zeCommandListAppendBarrier,
(CmdList->first, Event->ZeEvent, EventWaitList.Length,
EventWaitList.ZeEventList));
return PI_SUCCESS;
};
if (auto Res = createEventAndAssociateQueue(
Queue, &Event, PI_COMMAND_TYPE_USER, CmdList, IsInternal))
return Res;

Event->WaitList = EventWaitList;

// For in-order queue we don't need a real barrier, just wait for requested
// events in potentially different queues and add a "barrier" event signal
// because it is already guaranteed that previous commands in this queue
// are completed when the signal is started.
//
// TODO: this and other special handling of in-order queues to be
// updated when/if Level Zero adds native support for in-order queues.
//
if (Queue->isInOrderQueue() && InOrderBarrierBySignal) {
if (EventWaitList.Length) {
ZE_CALL(
zeCommandListAppendWaitOnEvents,
(CmdList->first, EventWaitList.Length, EventWaitList.ZeEventList));
}
ZE_CALL(zeCommandListAppendSignalEvent, (CmdList->first, Event->ZeEvent));
Comment on lines +5783 to +5787
Copy link
Contributor

@againull againull May 19, 2023

Choose a reason for hiding this comment

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

Looks good to me but it would be nice to get confirmation from @jandres742 here that it's going to work as expected - i.e. Event->ZeEvent is signaled only after we waited for all events in EventWaitList.ZeEventList.

Copy link
Contributor

Choose a reason for hiding this comment

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

Event->ZeEvent is signaled only after we waited for all events in EventWaitList.ZeEventList.

correct. if it doesnt work, then that would be a bug, but currently this works and is stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jandres742 Thanks a lot for confirming!

} else {
ZE_CALL(zeCommandListAppendBarrier,
(CmdList->first, Event->ZeEvent, EventWaitList.Length,
EventWaitList.ZeEventList));
}
return PI_SUCCESS;
};

// If the queue is in-order then each command in it effectively acts as a
// barrier, so we don't need to do anything except if we were requested
// a "barrier" event to be created. Or if we need to wait for events in
// potentially different queues.
//
if (Queue->isInOrderQueue() && NumEventsInWaitList == 0 && !OutEvent) {
return PI_SUCCESS;
}

pi_event InternalEvent;
bool IsInternal = OutEvent == nullptr;
Expand Down