Skip to content

Commit 2758b1b

Browse files
authored
[SYCL] Do not take any locks in submit for in-order queue (#18687)
in NoEvents mode. The lock was mainly needed to protect storing LastEvent, which is no longer necessary. Since the lock also provided a memory barrier, change memory order for load/stores on MNoEventMode to acquire/release to match the guarantees.
1 parent 8c528be commit 2758b1b

File tree

2 files changed

+8
-16
lines changed

2 files changed

+8
-16
lines changed

sycl/source/detail/queue_impl.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -351,17 +351,11 @@ queue_impl::submit_impl(const detail::type_erased_cgfo_ty &CGF,
351351

352352
auto requiresPostProcess = SubmitInfo.PostProcessorFunc() || Streams.size();
353353
auto noLastEventPath = !isHostTask && !isGraphSubmission &&
354-
MNoLastEventMode.load(std::memory_order_relaxed) &&
354+
MNoLastEventMode.load(std::memory_order_acquire) &&
355355
!requiresPostProcess;
356356

357357
if (noLastEventPath) {
358-
std::unique_lock<std::mutex> Lock(MMutex);
359-
360-
// Check if we are still in no last event mode. There could
361-
// have been a concurrent submit.
362-
if (MNoLastEventMode.load(std::memory_order_relaxed)) {
363-
return finalizeHandlerInOrderNoEventsUnlocked(Handler);
364-
}
358+
return finalizeHandlerInOrderNoEventsUnlocked(Handler);
365359
}
366360

367361
detail::EventImplPtr EventImpl;
@@ -751,10 +745,11 @@ ur_native_handle_t queue_impl::getNative(int32_t &NativeHandleDesc) const {
751745
bool queue_impl::queue_empty() const {
752746
// If we have in-order queue with non-empty last event, just check its status.
753747
if (isInOrder()) {
754-
std::lock_guard<std::mutex> Lock(MMutex);
755-
if (MEmpty)
748+
if (MEmpty.load(std::memory_order_acquire))
756749
return true;
757750

751+
std::lock_guard<std::mutex> Lock(MMutex);
752+
758753
if (MDefaultGraphDeps.LastEventPtr &&
759754
!MDefaultGraphDeps.LastEventPtr->isDiscarded())
760755
return MDefaultGraphDeps.LastEventPtr

sycl/source/detail/queue_impl.hpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -735,11 +735,8 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
735735
detail::EventImplPtr
736736
finalizeHandlerInOrderNoEventsUnlocked(HandlerType &Handler) {
737737
assert(isInOrder());
738-
assert(MGraph.expired());
739-
assert(MDefaultGraphDeps.LastEventPtr == nullptr);
740-
assert(MNoLastEventMode);
741738

742-
MEmpty = false;
739+
MEmpty.store(false, std::memory_order_release);
743740

744741
synchronizeWithExternalEvent(Handler);
745742

@@ -826,7 +823,7 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
826823
const CGType Type = getSyclObjImpl(Handler)->MCGType;
827824
std::lock_guard<std::mutex> Lock{MMutex};
828825

829-
MEmpty = false;
826+
MEmpty.store(false, std::memory_order_release);
830827

831828
// The following code supports barrier synchronization if host task is
832829
// involved in the scenario. Native barriers cannot handle host task
@@ -1048,7 +1045,7 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
10481045
std::atomic<bool> MNoLastEventMode = false;
10491046

10501047
// Used exclusively in getLastEvent and queue_empty() implementations
1051-
bool MEmpty = true;
1048+
std::atomic<bool> MEmpty = true;
10521049

10531050
std::vector<EventImplPtr> MStreamsServiceEvents;
10541051
std::mutex MStreamsServiceEventsMutex;

0 commit comments

Comments
 (0)