Skip to content

Commit 2752822

Browse files
[SYCL] Fix coverity hits in queue & graph (#20617)
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
1 parent a48398b commit 2752822

File tree

3 files changed

+23
-18
lines changed

3 files changed

+23
-18
lines changed

sycl/source/detail/graph/graph_impl.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ graph_impl::graph_impl(const sycl::context &SyclContext,
327327

328328
graph_impl::~graph_impl() {
329329
try {
330-
clearQueues();
330+
clearQueues(false /*Needs lock*/);
331331
for (auto &MemObj : MMemObjs) {
332332
MemObj->markNoLongerBeingUsedInGraph();
333333
}
@@ -564,17 +564,21 @@ void graph_impl::removeQueue(sycl::detail::queue_impl &RecordingQueue) {
564564
MRecordingQueues.erase(RecordingQueue.weak_from_this());
565565
}
566566

567-
bool graph_impl::clearQueues() {
568-
bool AnyQueuesCleared = false;
569-
for (auto &Queue : MRecordingQueues) {
567+
void graph_impl::clearQueues(bool NeedsLock) {
568+
graph_impl::RecQueuesStorage SwappedQueues;
569+
{
570+
graph_impl::WriteLock Guard(MMutex, std::defer_lock);
571+
if (NeedsLock) {
572+
Guard.lock();
573+
}
574+
std::swap(MRecordingQueues, SwappedQueues);
575+
}
576+
577+
for (auto &Queue : SwappedQueues) {
570578
if (auto ValidQueue = Queue.lock(); ValidQueue) {
571579
ValidQueue->setCommandGraph(nullptr);
572-
AnyQueuesCleared = true;
573580
}
574581
}
575-
MRecordingQueues.clear();
576-
577-
return AnyQueuesCleared;
578582
}
579583

580584
bool graph_impl::checkForCycles() {
@@ -1970,8 +1974,7 @@ void modifiable_command_graph::begin_recording(
19701974
}
19711975

19721976
void modifiable_command_graph::end_recording() {
1973-
graph_impl::WriteLock Lock(impl->MMutex);
1974-
impl->clearQueues();
1977+
impl->clearQueues(true /*Needs lock*/);
19751978
}
19761979

19771980
void modifiable_command_graph::end_recording(queue &RecordingQueue) {

sycl/source/detail/graph/graph_impl.hpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,7 @@ class graph_impl : public std::enable_shared_from_this<graph_impl> {
190190

191191
/// Remove all queues which are recording to this graph, also sets all queues
192192
/// cleared back to the executing state.
193-
///
194-
/// @return True if any queues were removed.
195-
bool clearQueues();
193+
void clearQueues(bool NeedsLock);
196194

197195
/// Associate a sycl event with a node in the graph.
198196
/// @param EventImpl Event to associate with a node in map.
@@ -561,10 +559,12 @@ class graph_impl : public std::enable_shared_from_this<graph_impl> {
561559
/// Device associated with this graph. All graph nodes will execute on this
562560
/// device.
563561
sycl::device MDevice;
562+
563+
using RecQueuesStorage =
564+
std::set<std::weak_ptr<sycl::detail::queue_impl>,
565+
std::owner_less<std::weak_ptr<sycl::detail::queue_impl>>>;
564566
/// Unique set of queues which are currently recording to this graph.
565-
std::set<std::weak_ptr<sycl::detail::queue_impl>,
566-
std::owner_less<std::weak_ptr<sycl::detail::queue_impl>>>
567-
MRecordingQueues;
567+
RecQueuesStorage MRecordingQueues;
568568
/// Map of events to their associated recorded nodes.
569569
std::unordered_map<std::shared_ptr<sycl::detail::event_impl>, node_impl *>
570570
MEventsMap;

sycl/source/detail/queue_impl.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,8 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
602602
bool CallerNeedsEvent);
603603

604604
void setCommandGraphUnlocked(
605-
std::shared_ptr<ext::oneapi::experimental::detail::graph_impl> Graph) {
605+
const std::shared_ptr<ext::oneapi::experimental::detail::graph_impl>
606+
&Graph) {
606607
MGraph = Graph;
607608
MExtGraphDeps.reset();
608609

@@ -614,7 +615,8 @@ class queue_impl : public std::enable_shared_from_this<queue_impl> {
614615
}
615616

616617
void setCommandGraph(
617-
std::shared_ptr<ext::oneapi::experimental::detail::graph_impl> Graph) {
618+
const std::shared_ptr<ext::oneapi::experimental::detail::graph_impl>
619+
&Graph) {
618620
std::lock_guard<std::mutex> Lock(MMutex);
619621
setCommandGraphUnlocked(Graph);
620622
}

0 commit comments

Comments
 (0)