[UR][L0] Correctly wait on barrier on urEnqueueEventsWaitWithBarrier #962
Conversation
jandres742
force-pushed
the
fixwaitbarrierwithevent
branch
5 times, most recently
from
b0e0a75 to
7368013
Compare
|
@smaslov-intel : please take a look. |
| @@ -964,8 +966,7 @@ ur_result_t CleanupCompletedEvent(ur_event_handle_t Event, bool QueueLocked, | |||
| ur_result_t EventCreate(ur_context_handle_t Context, ur_queue_handle_t Queue, | |||
| bool HostVisible, ur_event_handle_t *RetEvent) { | |||
|
|
|||
| bool ProfilingEnabled = | |||
| !Queue || (Queue->Properties & UR_QUEUE_FLAG_PROFILING_ENABLE) != 0; | |||
| bool ProfilingEnabled = !Queue || Queue->isProfilingEnabled(); | |||
There was a problem hiding this comment.
I know you did not change this, but it looks weird that when Queue == null then we create an event from a profiled pool
There was a problem hiding this comment.
@smaslov-intel : I know, I saw that and it looks weird. Maybe that was added as a WA, or a better logic could be used. i'm planning on addressing that in another patch.
| @@ -168,7 +168,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueEventsWaitWithBarrier( | |||
| // 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 (Queue->isInOrderQueue() && InOrderBarrierBySignal && | |||
| !Queue->isProfilingEnabled()) { | |||
There was a problem hiding this comment.
It's not clear to me how profiling affects synchronization.
There was a problem hiding this comment.
@smaslov-intel : please see here
zeCommandListAppendSignalEvent
The duration of an event created from an event pool that was created using ZE_EVENT_POOL_FLAG_KERNEL_TIMESTAMP or ZE_EVENT_POOL_FLAG_KERNEL_MAPPED_TIMESTAMP flags is undefined
Therefore,, when profiling is used, we cannot just used signalEvent if EventWaitList.Lenght == 0. We need to fallback directly to barrier to have correct timestamps.
There was a problem hiding this comment.
OK, but please add source code comment explaining this.
There was a problem hiding this comment.
thanks @smaslov-intel. I have added comment.
jandres742
force-pushed
the
fixwaitbarrierwithevent
branch
3 times, most recently
from
ba1a51d to
7fd9daf
Compare
When event list is null, a barrier is still needed for all previous commands if profiling is enabled, so fix it. Signed-off-by: Jaime Arteaga <[email protected]>
Signed-off-by: Jaime Arteaga <[email protected]>
|
@kbenzie : please merge when possible |
|
I have updated the target branch of this PR from the |
…vent [UR][L0] Correctly wait on barrier on urEnqueueEventsWaitWithBarrier
…vent [UR][L0] Correctly wait on barrier on urEnqueueEventsWaitWithBarrier
When event list is null, a barrier is still needed for all previous commands if profiling is enabled, so fix it.
LLVM testing: intel/llvm#11541