[SYCL] Fix warnings in sycl/source Part 2 (#16359)

Signed-off-by: Tikhomirova, Kseniya <[email protected]>

Author: KseniyaTikhomirova

sycl/source/detail/graph_impl.cpp

@@ -1637,7 +1637,7 @@ node modifiable_command_graph::addImpl(dynamic_command_group &DynCGF,
 
   graph_impl::WriteLock Lock(impl->MMutex);
   std::shared_ptr<detail::node_impl> NodeImpl = impl->add(DynCGFImpl, DepImpls);
-  return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
+  return sycl::detail::createSyclObjFromImpl<node>(std::move(NodeImpl));
 }
 
 node modifiable_command_graph::addImpl(const std::vector<node> &Deps) {
@@ -1649,7 +1649,7 @@ node modifiable_command_graph::addImpl(const std::vector<node> &Deps) {
 
   graph_impl::WriteLock Lock(impl->MMutex);
   std::shared_ptr<detail::node_impl> NodeImpl = impl->add(DepImpls);
-  return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
+  return sycl::detail::createSyclObjFromImpl<node>(std::move(NodeImpl));
 }
 
 node modifiable_command_graph::addImpl(std::function<void(handler &)> CGF,
@@ -1662,7 +1662,7 @@ node modifiable_command_graph::addImpl(std::function<void(handler &)> CGF,
 
   graph_impl::WriteLock Lock(impl->MMutex);
   std::shared_ptr<detail::node_impl> NodeImpl = impl->add(CGF, {}, DepImpls);
-  return sycl::detail::createSyclObjFromImpl<node>(NodeImpl);
+  return sycl::detail::createSyclObjFromImpl<node>(std::move(NodeImpl));
 }
 
 void modifiable_command_graph::addGraphLeafDependencies(node Node) {
@@ -1987,8 +1987,8 @@ void dynamic_command_group_impl::finalizeCGFList(
     // shared_ptr<detail::CGExecKernel> to store
     sycl::detail::CG *RawCGPtr = Handler.impl->MGraphNodeCG.release();
     auto RawCGExecPtr = static_cast<sycl::detail::CGExecKernel *>(RawCGPtr);
-    auto CGExecSP = std::shared_ptr<sycl::detail::CGExecKernel>(RawCGExecPtr);
-    MKernels.push_back(CGExecSP);
+    MKernels.push_back(
+        std::shared_ptr<sycl::detail::CGExecKernel>(RawCGExecPtr));
 
     // Track dynamic_parameter usage in command-list
     auto &DynamicParams = Handler.impl->MDynamicParameters;

sycl/source/detail/graph_impl.hpp

@@ -140,7 +140,8 @@ class node_impl : public std::enable_shared_from_this<node_impl> {
   /// @param NodeType Type of the command-group.
   /// @param CommandGroup The CG which stores the command information for this
   /// node.
-  node_impl(node_type NodeType, std::shared_ptr<sycl::detail::CG> CommandGroup)
+  node_impl(node_type NodeType,
+            const std::shared_ptr<sycl::detail::CG> &CommandGroup)
       : MCGType(CommandGroup->getType()), MNodeType(NodeType),
         MCommandGroup(CommandGroup) {
     if (NodeType == node_type::subgraph) {

sycl/source/detail/kernel_program_cache.hpp

@@ -442,6 +442,7 @@ class KernelProgramCache {
         return;
 
       // Save reference between the program and the fast cache key.
+      std::unique_lock<std::mutex> Lock(MKernelFastCacheMutex);
       MProgramToKernelFastCacheKeyMap[Program].emplace_back(CacheKey);
     }
 
@@ -495,16 +496,18 @@ class KernelProgramCache {
             LockedCacheKP.get().erase(NativePrg);
           }
 
-          // Remove corresponding entries from KernelFastCache.
-          auto FastCacheKeyItr =
-              MProgramToKernelFastCacheKeyMap.find(NativePrg);
-          if (FastCacheKeyItr != MProgramToKernelFastCacheKeyMap.end()) {
-            for (const auto &FastCacheKey : FastCacheKeyItr->second) {
-              std::unique_lock<std::mutex> Lock(MKernelFastCacheMutex);
-              MKernelFastCache.erase(FastCacheKey);
-              traceKernel("Kernel evicted.", std::get<2>(FastCacheKey), true);
+          {
+            // Remove corresponding entries from KernelFastCache.
+            std::unique_lock<std::mutex> Lock(MKernelFastCacheMutex);
+            if (auto FastCacheKeyItr =
+                    MProgramToKernelFastCacheKeyMap.find(NativePrg);
+                FastCacheKeyItr != MProgramToKernelFastCacheKeyMap.end()) {
+              for (const auto &FastCacheKey : FastCacheKeyItr->second) {
+                MKernelFastCache.erase(FastCacheKey);
+                traceKernel("Kernel evicted.", std::get<2>(FastCacheKey), true);
+              }
+              MProgramToKernelFastCacheKeyMap.erase(FastCacheKeyItr);
             }
-            MProgramToKernelFastCacheKeyMap.erase(FastCacheKeyItr);
           }
 
           // Remove entry from ProgramCache KeyMap.
@@ -617,16 +620,15 @@ class KernelProgramCache {
   ///
   /// This member function should only be used in unit tests.
   void reset() {
+    std::lock_guard<std::mutex> EvictionListLock(MProgramEvictionListMutex);
     std::lock_guard<std::mutex> L1(MProgramCacheMutex);
     std::lock_guard<std::mutex> L2(MKernelsPerProgramCacheMutex);
     std::lock_guard<std::mutex> L3(MKernelFastCacheMutex);
     MCachedPrograms = ProgramCache{};
     MKernelsPerProgramCache = KernelCacheT{};
     MKernelFastCache = KernelFastCacheT{};
     MProgramToKernelFastCacheKeyMap.clear();
-
     // Clear the eviction lists and its mutexes.
-    std::lock_guard<std::mutex> EvictionListLock(MProgramEvictionListMutex);
     MEvictionList.clear();
   }
 

sycl/source/detail/online_compiler/online_compiler.cpp

@@ -98,12 +98,18 @@ compileToSPIRV(string_view Src, sycl::info::device_type DeviceType,
 #else
     static const std::string OclocLibraryName = "libocloc.so";
 #endif
-    void *OclocLibrary = sycl::detail::ur::loadOsLibrary(OclocLibraryName);
+    auto CustomDeleter = [](void *StoredPtr) {
+      if (!StoredPtr)
+        return;
+      std::ignore = sycl::detail::ur::unloadOsLibrary(StoredPtr);
+    };
+    std::unique_ptr<void, decltype(CustomDeleter)> OclocLibrary(
+        sycl::detail::ur::loadOsLibrary(OclocLibraryName), CustomDeleter);
     if (!OclocLibrary)
       throw online_compile_error("Cannot load ocloc library: " +
                                  OclocLibraryName);
-    void *OclocVersionHandle =
-        sycl::detail::ur::getOsLibraryFuncAddress(OclocLibrary, "oclocVersion");
+    void *OclocVersionHandle = sycl::detail::ur::getOsLibraryFuncAddress(
+        OclocLibrary.get(), "oclocVersion");
     // The initial versions of ocloc library did not have the oclocVersion()
     // function. Those versions had the same API as the first version of ocloc
     // library having that oclocVersion() function.
@@ -129,18 +135,21 @@ compileToSPIRV(string_view Src, sycl::info::device_type DeviceType,
           std::to_string(CurrentVersionMajor) +
           ".N), where (N >= " + std::to_string(CurrentVersionMinor) + ").");
 
-    CompileToSPIRVHandle =
-        sycl::detail::ur::getOsLibraryFuncAddress(OclocLibrary, "oclocInvoke");
+    CompileToSPIRVHandle = sycl::detail::ur::getOsLibraryFuncAddress(
+        OclocLibrary.get(), "oclocInvoke");
     if (!CompileToSPIRVHandle)
       throw online_compile_error("Cannot load oclocInvoke() function");
     FreeSPIRVOutputsHandle = sycl::detail::ur::getOsLibraryFuncAddress(
-        OclocLibrary, "oclocFreeOutput");
-    if (!FreeSPIRVOutputsHandle)
+        OclocLibrary.get(), "oclocFreeOutput");
+    if (!FreeSPIRVOutputsHandle) {
+      CompileToSPIRVHandle = NULL;
       throw online_compile_error("Cannot load oclocFreeOutput() function");
+    }
+    OclocLibrary.release();
   }
 
   std::string CombinedUserArgs;
-  for (auto UserArg : UserArgs) {
+  for (const auto &UserArg : UserArgs) {
     if (UserArg == "")
       continue;
     if (CombinedUserArgs != "")

sycl/source/detail/program_manager/program_manager.cpp

@@ -799,9 +799,9 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(
       RootDevImpl->getHandleRef(), UR_DEVICE_INFO_BUILD_ON_SUBDEVICE,
       sizeof(ur_bool_t), &MustBuildOnSubdevice, nullptr);
 
-  DeviceImplPtr Dev = (MustBuildOnSubdevice == true) ? DeviceImpl : RootDevImpl;
   auto Context = createSyclObjFromImpl<context>(ContextImpl);
-  auto Device = createSyclObjFromImpl<device>(Dev);
+  auto Device = createSyclObjFromImpl<device>(
+      MustBuildOnSubdevice == true ? DeviceImpl : RootDevImpl);
   const RTDeviceBinaryImage &Img =
       getDeviceImage(KernelName, Context, Device, JITCompilationIsRequired);
 
@@ -822,7 +822,7 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(
   std::copy(DeviceImagesToLink.begin(), DeviceImagesToLink.end(),
             std::back_inserter(AllImages));
 
-  return getBuiltURProgram(std::move(AllImages), Context, {Device});
+  return getBuiltURProgram(std::move(AllImages), Context, {std::move(Device)});
 }
 
 ur_program_handle_t ProgramManager::getBuiltURProgram(
@@ -1008,7 +1008,7 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(
         }
       }
       // Change device in the cache key to reduce copying of spec const data.
-      CacheKey.second = Subset;
+      CacheKey.second = std::move(Subset);
       bool DidInsert = Cache.insertBuiltProgram(CacheKey, ResProgram);
       if (DidInsert) {
         // For every cached copy of the program, we need to increment its

sycl/source/detail/scheduler/commands.cpp

@@ -2569,9 +2569,8 @@ getCGKernelInfo(const CGExecKernel &CommandGroup, ContextImplPtr ContextImpl,
   // they can simply be launched directly.
   if (auto KernelBundleImplPtr = CommandGroup.MKernelBundle;
       KernelBundleImplPtr && !KernelBundleImplPtr->isInterop()) {
-    auto KernelName = CommandGroup.MKernelName;
-    kernel_id KernelID =
-        detail::ProgramManager::getInstance().getSYCLKernelID(KernelName);
+    kernel_id KernelID = detail::ProgramManager::getInstance().getSYCLKernelID(
+        CommandGroup.MKernelName);
 
     kernel SyclKernel =
         KernelBundleImplPtr->get_kernel(KernelID, KernelBundleImplPtr);
@@ -2775,16 +2774,16 @@ void enqueueImpKernel(
   // Initialize device globals associated with this.
   std::vector<ur_event_handle_t> DeviceGlobalInitEvents =
       ContextImpl->initializeDeviceGlobals(Program, Queue);
-  std::vector<ur_event_handle_t> EventsWithDeviceGlobalInits;
   if (!DeviceGlobalInitEvents.empty()) {
+    std::vector<ur_event_handle_t> EventsWithDeviceGlobalInits;
     EventsWithDeviceGlobalInits.reserve(RawEvents.size() +
                                         DeviceGlobalInitEvents.size());
     EventsWithDeviceGlobalInits.insert(EventsWithDeviceGlobalInits.end(),
                                        RawEvents.begin(), RawEvents.end());
     EventsWithDeviceGlobalInits.insert(EventsWithDeviceGlobalInits.end(),
                                        DeviceGlobalInitEvents.begin(),
                                        DeviceGlobalInitEvents.end());
-    EventsWaitList = EventsWithDeviceGlobalInits;
+    EventsWaitList = std::move(EventsWithDeviceGlobalInits);
   }
 
   ur_result_t Error = UR_RESULT_SUCCESS;
@@ -3642,6 +3641,7 @@ ur_result_t ExecCGCommand::enqueueImpQueue() {
       // we don't need to enqueue anything.
       return UR_RESULT_SUCCESS;
     }
+    assert(MQueue && "Empty node should have an associated queue");
     const detail::AdapterPtr &Adapter = MQueue->getAdapter();
     ur_event_handle_t Event;
     ur_result_t Result = Adapter->call_nocheck<UrApiKind::urEnqueueEventsWait>(

sycl/source/handler.cpp

@@ -543,11 +543,10 @@ event handler::finalize() {
       // In-order queues create implicit linear dependencies between nodes.
       // Find the last node added to the graph from this queue, so our new
       // node can set it as a predecessor.
-      auto DependentNode = GraphImpl->getLastInorderNode(MQueue);
       std::vector<std::shared_ptr<ext::oneapi::experimental::detail::node_impl>>
           Deps;
-      if (DependentNode) {
-        Deps.push_back(DependentNode);
+      if (auto DependentNode = GraphImpl->getLastInorderNode(MQueue)) {
+        Deps.push_back(std::move(DependentNode));
       }
       NodeImpl = GraphImpl->add(NodeType, std::move(CommandGroup), Deps);
 
@@ -571,7 +570,7 @@ event handler::finalize() {
     }
 
     // Associate an event with this new node and return the event.
-    GraphImpl->addEventForNode(EventImpl, NodeImpl);
+    GraphImpl->addEventForNode(EventImpl, std::move(NodeImpl));
 
     return detail::createSyclObjFromImpl<event>(EventImpl);
   }

sycl/source/queue.cpp

@@ -199,12 +199,12 @@ event queue::mem_advise(const void *Ptr, size_t Length, int Advice,
 /// TODO: Unused. Remove these when ABI-break window is open.
 event queue::submit_impl(std::function<void(handler &)> CGH,
                          const detail::code_location &CodeLoc) {
-  return submit_with_event_impl(CGH, {}, CodeLoc, true);
+  return submit_with_event_impl(std::move(CGH), {}, CodeLoc, true);
 }
 event queue::submit_impl(std::function<void(handler &)> CGH,
                          const detail::code_location &CodeLoc,
                          bool IsTopCodeLoc) {
-  return submit_with_event_impl(CGH, {}, CodeLoc, IsTopCodeLoc);
+  return submit_with_event_impl(std::move(CGH), {}, CodeLoc, IsTopCodeLoc);
 }
 
 event queue::submit_impl(std::function<void(handler &)> CGH, queue SecondQueue,
@@ -219,27 +219,27 @@ event queue::submit_impl(std::function<void(handler &)> CGH, queue SecondQueue,
 
 void queue::submit_without_event_impl(std::function<void(handler &)> CGH,
                                       const detail::code_location &CodeLoc) {
-  submit_without_event_impl(CGH, {}, CodeLoc, true);
+  submit_without_event_impl(std::move(CGH), {}, CodeLoc, true);
 }
 void queue::submit_without_event_impl(std::function<void(handler &)> CGH,
                                       const detail::code_location &CodeLoc,
                                       bool IsTopCodeLoc) {
-  submit_without_event_impl(CGH, {}, CodeLoc, IsTopCodeLoc);
+  submit_without_event_impl(std::move(CGH), {}, CodeLoc, IsTopCodeLoc);
 }
 
 event queue::submit_impl_and_postprocess(
     std::function<void(handler &)> CGH, const detail::code_location &CodeLoc,
     const detail::SubmitPostProcessF &PostProcess) {
   detail::SubmissionInfo SI{};
   SI.PostProcessorFunc() = std::move(PostProcess);
-  return submit_with_event_impl(CGH, SI, CodeLoc, true);
+  return submit_with_event_impl(std::move(CGH), SI, CodeLoc, true);
 }
 event queue::submit_impl_and_postprocess(
     std::function<void(handler &)> CGH, const detail::code_location &CodeLoc,
     const detail::SubmitPostProcessF &PostProcess, bool IsTopCodeLoc) {
   detail::SubmissionInfo SI{};
   SI.PostProcessorFunc() = std::move(PostProcess);
-  return submit_with_event_impl(CGH, SI, CodeLoc, IsTopCodeLoc);
+  return submit_with_event_impl(std::move(CGH), SI, CodeLoc, IsTopCodeLoc);
 }
 
 event queue::submit_impl_and_postprocess(