[UR] [V2] Fix synchronization between command_list_manager usages#17297
[UR] [V2] Fix synchronization between command_list_manager usages#17297sarnex merged 3 commits intointel:syclfrom
Conversation
pbalcer
left a comment
There was a problem hiding this comment.
looks fine, but remove the shared_ptr. I don't see its value, and shared_ptrs are expensive to use.
unified-runtime/source/adapters/level_zero/v2/queue_immediate_in_order.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
why do we still use this->mutex (here and in other places)? shouldn't we remove it and use command list locking exclusively?
unified-runtime/source/adapters/level_zero/v2/queue_immediate_in_order.cpp
Outdated
Show resolved
Hide resolved
cfe33c4 to
53e2c7a
Compare
|
@intel/llvm-gatekeepers please merge. One test fails on unrelated changes and it has an issue #17392 |
|
@intel/llvm-gatekeepers please merge, and if there is any issue with merge, I would be really grateful if someone could tell how to solve it |
|
@intel/llvm-gatekeepers bump |
|
Unlikely ESIMD fail is related |
| ur_result_t urCommandBufferEnqueueExp( | ||
| ur_exp_command_buffer_handle_t CommandBuffer, ur_queue_handle_t UrQueue, | ||
| uint32_t NumEventsInWaitList, const ur_event_handle_t *EventWaitList, | ||
| ur_event_handle_t *Event) try { | ||
| return UrQueue->get().enqueueCommandBufferExp( | ||
| CommandBuffer, NumEventsInWaitList, EventWaitList, Event); | ||
| } catch (...) { | ||
| return exceptionToResult(std::current_exception()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why was this function reintroduced? I think it might be a rebase error
There was a problem hiding this comment.
whoops, let me know if i should revert this
There was a problem hiding this comment.
Opened a PR to fix this #17476 (not worth a revert, it's dead code but not breaking anything)
`urCommandBufferEnqueueExp` was changed in intel/llvm#16984 to `urEnqueueCommandBufferExp` but added back accidentally to the v2 L0 adapter in intel/llvm#17297
Changes:
Command_list_manager no longer synchronize its calls, instead the responsibility to ensure exclusivity belongs to the caller.
To add synchronization I implemented the mechanism similar to rust lock as suggested in #17061 (comment).