Skip to content

[Coverity] Variable copied when it could be moved #18412

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.

Sign up for GitHub

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

Jump to bottom
Closed
AlexeySachkov opened this issue May 12, 2025 · 0 comments · Fixed by #18473
Closed

[Coverity] Variable copied when it could be moved #18412

AlexeySachkov opened this issue May 12, 2025 · 0 comments · Fixed by #18473
Labels
bug Something isn't working sycl-graph

Comments

@AlexeySachkov
Copy link
Contributor

intel/llvm Coverity scan reports a bunch of issues about missing opportunities to use std::move in graph implementation:

CID 489935:

llvm/sycl/source/detail/graph_impl.cpp

Lines 144 to 149 in 713a9dd

if (Node->MCGType == sycl::detail::CGType::CodeplayHostTask) {
if (Node->MPartitionNum != -1) {
HostTaskList.push_front(Node);
}
return;
}

CID 489937: CGData.MEvents.push_back(Event);

llvm/sycl/source/detail/graph_impl.cpp

Lines 1023 to 1042 in 713a9dd

if (CommandBuffer) {
for (std::vector<sycl::detail::EventImplPtr>::iterator It =
MExecutionEvents.begin();
It != MExecutionEvents.end();) {
auto Event = *It;
if (!Event->isCompleted()) {
auto &AttachedEventsList = Event->getPostCompleteEvents();
CGData.MEvents.reserve(CGData.MEvents.size() +
AttachedEventsList.size() + 1);
CGData.MEvents.push_back(Event);
// Add events of the previous execution of all graph partitions.
CGData.MEvents.insert(CGData.MEvents.end(),
AttachedEventsList.begin(),
AttachedEventsList.end());
++It;
} else {
// Remove completed events
It = MExecutionEvents.erase(It);
}
}

CID 490022:

llvm/sycl/source/detail/graph_impl.cpp

Line 1863 in 713a9dd

impl->beginRecording(QueueImpl);

CID 490343:

llvm/sycl/source/detail/graph_impl.hpp

Lines 897 to 901 in 713a9dd

void addEventForNode(std::shared_ptr<sycl::detail::event_impl> EventImpl,
std::shared_ptr<node_impl> NodeImpl) {
if (!(EventImpl->hasCommandGraph()))
EventImpl->setCommandGraph(shared_from_this());
MEventsMap[EventImpl] = NodeImpl;

@AlexeySachkov AlexeySachkov added bug Something isn't working sycl-graph labels May 12, 2025
fabiomestre added a commit to reble/llvm that referenced this issue May 14, 2025
@fabiomestre
Fix coverity issues by passing shared pointer by reference.
3ff8738 
Fixes: intel#18412
@fabiomestre fabiomestre mentioned this issue May 14, 2025
Merged
fabiomestre added a commit to reble/llvm that referenced this issue May 14, 2025
@fabiomestre
Fix coverity issues by passing shared pointer by reference.
b85e029 
Fixes: intel#18412
fabiomestre added a commit to reble/llvm that referenced this issue May 14, 2025
@fabiomestre
Fix coverity issues by passing shared pointer by reference.
4aee6ce 
Fixes: intel#18412
fabiomestre added a commit to reble/llvm that referenced this issue May 14, 2025
@fabiomestre
Fix coverity issues by passing shared pointer by reference.
0e74543 
Fixes: intel#18412
fabiomestre added a commit to reble/llvm that referenced this issue May 16, 2025
@fabiomestre
Fix coverity issues by passing shared pointer by reference.
344af51 
Fixes: intel#18412
kbenzie pushed a commit that referenced this issue May 19, 2025
@fabiomestre
[SYCL][Graph]Fix coverity issues by passing shared pointer by referen…
ff0a7ac 
…ce. (#18473)

Changes some functions in the sycl graph implementation to pass shared
pointers by reference instead of by value.

Fixes: #18412
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working sycl-graph
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[SYCL][Graph]Fix coverity issues by passing shared pointer by reference. reble/llvm
1 participant
@AlexeySachkov