From f541207ba1d5615cf97de6a6268b807ae23c7f1a Mon Sep 17 00:00:00 2001 From: Kseniya Tikhomirova Date: Thu, 19 Dec 2024 18:22:35 +0100 Subject: [PATCH] [SYCL] Fix warnings in sycl/source Part 2 (#16359) Signed-off-by: Tikhomirova, Kseniya --- sycl/source/detail/graph_impl.cpp | 10 ++++---- sycl/source/detail/graph_impl.hpp | 3 ++- sycl/source/detail/kernel_program_cache.hpp | 24 ++++++++++-------- .../online_compiler/online_compiler.cpp | 25 +++++++++++++------ .../program_manager/program_manager.cpp | 8 +++--- sycl/source/detail/scheduler/commands.cpp | 10 ++++---- sycl/source/handler.cpp | 7 +++--- sycl/source/queue.cpp | 12 ++++----- 8 files changed, 55 insertions(+), 44 deletions(-) diff --git a/sycl/source/detail/graph_impl.cpp b/sycl/source/detail/graph_impl.cpp index c2cc22a84a00b..67ccf0a1885e7 100644 --- a/sycl/source/detail/graph_impl.cpp +++ b/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 NodeImpl = impl->add(DynCGFImpl, DepImpls); - return sycl::detail::createSyclObjFromImpl(NodeImpl); + return sycl::detail::createSyclObjFromImpl(std::move(NodeImpl)); } node modifiable_command_graph::addImpl(const std::vector &Deps) { @@ -1649,7 +1649,7 @@ node modifiable_command_graph::addImpl(const std::vector &Deps) { graph_impl::WriteLock Lock(impl->MMutex); std::shared_ptr NodeImpl = impl->add(DepImpls); - return sycl::detail::createSyclObjFromImpl(NodeImpl); + return sycl::detail::createSyclObjFromImpl(std::move(NodeImpl)); } node modifiable_command_graph::addImpl(std::function CGF, @@ -1662,7 +1662,7 @@ node modifiable_command_graph::addImpl(std::function CGF, graph_impl::WriteLock Lock(impl->MMutex); std::shared_ptr NodeImpl = impl->add(CGF, {}, DepImpls); - return sycl::detail::createSyclObjFromImpl(NodeImpl); + return sycl::detail::createSyclObjFromImpl(std::move(NodeImpl)); } void modifiable_command_graph::addGraphLeafDependencies(node Node) { @@ -1987,8 +1987,8 @@ void dynamic_command_group_impl::finalizeCGFList( // shared_ptr to store sycl::detail::CG *RawCGPtr = Handler.impl->MGraphNodeCG.release(); auto RawCGExecPtr = static_cast(RawCGPtr); - auto CGExecSP = std::shared_ptr(RawCGExecPtr); - MKernels.push_back(CGExecSP); + MKernels.push_back( + std::shared_ptr(RawCGExecPtr)); // Track dynamic_parameter usage in command-list auto &DynamicParams = Handler.impl->MDynamicParameters; diff --git a/sycl/source/detail/graph_impl.hpp b/sycl/source/detail/graph_impl.hpp index 6144e3f51b9da..2d714bcc96d66 100644 --- a/sycl/source/detail/graph_impl.hpp +++ b/sycl/source/detail/graph_impl.hpp @@ -140,7 +140,8 @@ class node_impl : public std::enable_shared_from_this { /// @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 CommandGroup) + node_impl(node_type NodeType, + const std::shared_ptr &CommandGroup) : MCGType(CommandGroup->getType()), MNodeType(NodeType), MCommandGroup(CommandGroup) { if (NodeType == node_type::subgraph) { diff --git a/sycl/source/detail/kernel_program_cache.hpp b/sycl/source/detail/kernel_program_cache.hpp index 9f06d0ebcde8d..968cb9b24b053 100644 --- a/sycl/source/detail/kernel_program_cache.hpp +++ b/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 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 Lock(MKernelFastCacheMutex); - MKernelFastCache.erase(FastCacheKey); - traceKernel("Kernel evicted.", std::get<2>(FastCacheKey), true); + { + // Remove corresponding entries from KernelFastCache. + std::unique_lock 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,6 +620,7 @@ class KernelProgramCache { /// /// This member function should only be used in unit tests. void reset() { + std::lock_guard EvictionListLock(MProgramEvictionListMutex); std::lock_guard L1(MProgramCacheMutex); std::lock_guard L2(MKernelsPerProgramCacheMutex); std::lock_guard L3(MKernelFastCacheMutex); @@ -624,9 +628,7 @@ class KernelProgramCache { MKernelsPerProgramCache = KernelCacheT{}; MKernelFastCache = KernelFastCacheT{}; MProgramToKernelFastCacheKeyMap.clear(); - // Clear the eviction lists and its mutexes. - std::lock_guard EvictionListLock(MProgramEvictionListMutex); MEvictionList.clear(); } diff --git a/sycl/source/detail/online_compiler/online_compiler.cpp b/sycl/source/detail/online_compiler/online_compiler.cpp index f5e62f8a1d4d8..138fc880edb92 100644 --- a/sycl/source/detail/online_compiler/online_compiler.cpp +++ b/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 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 != "") diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index e23673b3ee341..ad9cf4d9b92f8 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/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(ContextImpl); - auto Device = createSyclObjFromImpl(Dev); + auto Device = createSyclObjFromImpl( + 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 diff --git a/sycl/source/detail/scheduler/commands.cpp b/sycl/source/detail/scheduler/commands.cpp index 7a20d8122dc36..31db161f88726 100644 --- a/sycl/source/detail/scheduler/commands.cpp +++ b/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,8 +2774,8 @@ void enqueueImpKernel( // Initialize device globals associated with this. std::vector DeviceGlobalInitEvents = ContextImpl->initializeDeviceGlobals(Program, Queue); - std::vector EventsWithDeviceGlobalInits; if (!DeviceGlobalInitEvents.empty()) { + std::vector EventsWithDeviceGlobalInits; EventsWithDeviceGlobalInits.reserve(RawEvents.size() + DeviceGlobalInitEvents.size()); EventsWithDeviceGlobalInits.insert(EventsWithDeviceGlobalInits.end(), @@ -2784,7 +2783,7 @@ void enqueueImpKernel( 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( diff --git a/sycl/source/handler.cpp b/sycl/source/handler.cpp index e7b32c0fe38ae..19d0fe95706cf 100644 --- a/sycl/source/handler.cpp +++ b/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> 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(EventImpl); } diff --git a/sycl/source/queue.cpp b/sycl/source/queue.cpp index 399d67af8afc8..831c3d0a87555 100644 --- a/sycl/source/queue.cpp +++ b/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 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 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 CGH, queue SecondQueue, @@ -219,12 +219,12 @@ event queue::submit_impl(std::function CGH, queue SecondQueue, void queue::submit_without_event_impl(std::function 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 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( @@ -232,14 +232,14 @@ event queue::submit_impl_and_postprocess( 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 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(