From 30612c6633ac7b9c7a0d3432eafd77801968a456 Mon Sep 17 00:00:00 2001 From: "Garcia Orozco, David" Date: Mon, 24 Mar 2025 13:26:13 -0700 Subject: [PATCH 1/5] [SYCL] Fix 'move instead of copy' Coverity hits --- sycl/source/detail/config.hpp | 2 +- sycl/source/detail/jit_compiler.cpp | 8 +++++--- sycl/source/detail/scheduler/scheduler.cpp | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/sycl/source/detail/config.hpp b/sycl/source/detail/config.hpp index 3e0a591e27d14..f031f2409ac79 100644 --- a/sycl/source/detail/config.hpp +++ b/sycl/source/detail/config.hpp @@ -652,7 +652,7 @@ template <> class SYCLConfig { const char *ValStr = getCachedValue(); if (!ValStr) - return DefaultValue; + return std::move(DefaultValue); return std::string{ValStr}; } diff --git a/sycl/source/detail/jit_compiler.cpp b/sycl/source/detail/jit_compiler.cpp index 3ccd0b6a1c067..544a0bd6f9d79 100644 --- a/sycl/source/detail/jit_compiler.cpp +++ b/sycl/source/detail/jit_compiler.cpp @@ -1044,14 +1044,16 @@ jit_compiler::fuseKernels(QueueImplPtr Queue, std::shared_ptr KernelBundleImplPtr; if (TargetFormat == ::jit_compiler::BinaryFormat::SPIRV) { detail::getSyclObjImpl(get_kernel_bundle( - Queue->get_context(), {Queue->get_device()}, {FusedKernelId})); + Queue->get_context(), {Queue->get_device()}, + {std::move(FusedKernelId)})); } std::unique_ptr FusedCG; FusedCG.reset(new detail::CGExecKernel( NDRDesc, nullptr, nullptr, std::move(KernelBundleImplPtr), - std::move(CGData), std::move(FusedArgs), FusedOrCachedKernelName, {}, {}, - CGType::Kernel, KernelCacheConfig, false /* KernelIsCooperative */, + std::move(CGData), std::move(FusedArgs), + std::move(FusedOrCachedKernelName), {}, {}, CGType::Kernel, + KernelCacheConfig, false /* KernelIsCooperative */, false /* KernelUsesClusterLaunch*/, 0 /* KernelWorkGroupMemorySize */)); return FusedCG; } diff --git a/sycl/source/detail/scheduler/scheduler.cpp b/sycl/source/detail/scheduler/scheduler.cpp index 856cbdc3cca14..8d75fcf3213bb 100644 --- a/sycl/source/detail/scheduler/scheduler.cpp +++ b/sycl/source/detail/scheduler/scheduler.cpp @@ -593,7 +593,7 @@ ur_kernel_handle_t Scheduler::completeSpecConstMaterialization( [[maybe_unused]] std::vector &SpecConstBlob) { #if SYCL_EXT_JIT_ENABLE && !_WIN32 return detail::jit_compiler::get_instance().materializeSpecConstants( - Queue, BinImage, KernelName, SpecConstBlob); + std::move(Queue), BinImage, KernelName, SpecConstBlob); #else // SYCL_EXT_JIT_ENABLE && !_WIN32 if (detail::SYCLConfig::get() > 0) { std::cerr << "WARNING: Materialization of spec constants not supported by " From 4c465e806ef809b424ca4d61fe56644f6ece041b Mon Sep 17 00:00:00 2001 From: "Garcia Orozco, David" Date: Tue, 25 Mar 2025 06:47:12 -0700 Subject: [PATCH 2/5] Instead of move pass const ref to `completeSpecConstMaterialization` --- sycl/source/detail/jit_compiler.cpp | 6 +++--- sycl/source/detail/jit_compiler.hpp | 2 +- sycl/source/detail/scheduler/scheduler.cpp | 4 ++-- sycl/source/detail/scheduler/scheduler.hpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sycl/source/detail/jit_compiler.cpp b/sycl/source/detail/jit_compiler.cpp index 544a0bd6f9d79..13a73a5ba8fc6 100644 --- a/sycl/source/detail/jit_compiler.cpp +++ b/sycl/source/detail/jit_compiler.cpp @@ -137,7 +137,7 @@ translateBinaryImageFormat(ur::DeviceBinaryType Type) { } } -::jit_compiler::BinaryFormat getTargetFormat(QueueImplPtr &Queue) { +::jit_compiler::BinaryFormat getTargetFormat(const QueueImplPtr &Queue) { auto Backend = Queue->getDeviceImplPtr()->getBackend(); switch (Backend) { case backend::ext_oneapi_level_zero: @@ -154,7 +154,7 @@ ::jit_compiler::BinaryFormat getTargetFormat(QueueImplPtr &Queue) { } } -::jit_compiler::TargetInfo getTargetInfo(QueueImplPtr &Queue) { +::jit_compiler::TargetInfo getTargetInfo(const QueueImplPtr &Queue) { ::jit_compiler::BinaryFormat Format = getTargetFormat(Queue); return ::jit_compiler::TargetInfo::get( Format, static_cast<::jit_compiler::DeviceArchitecture>( @@ -659,7 +659,7 @@ updatePromotedArgs(const ::jit_compiler::SYCLKernelInfo &FusedKernelInfo, } ur_kernel_handle_t jit_compiler::materializeSpecConstants( - QueueImplPtr Queue, const RTDeviceBinaryImage *BinImage, + const QueueImplPtr &Queue, const RTDeviceBinaryImage *BinImage, const std::string &KernelName, const std::vector &SpecConstBlob) { #ifndef _WIN32 diff --git a/sycl/source/detail/jit_compiler.hpp b/sycl/source/detail/jit_compiler.hpp index 1b85afc1d4cda..ba5911820bc78 100644 --- a/sycl/source/detail/jit_compiler.hpp +++ b/sycl/source/detail/jit_compiler.hpp @@ -44,7 +44,7 @@ class jit_compiler { fuseKernels(QueueImplPtr Queue, std::vector &InputKernels, const property_list &); ur_kernel_handle_t - materializeSpecConstants(QueueImplPtr Queue, + materializeSpecConstants(const QueueImplPtr &Queue, const RTDeviceBinaryImage *BinImage, const std::string &KernelName, const std::vector &SpecConstBlob); diff --git a/sycl/source/detail/scheduler/scheduler.cpp b/sycl/source/detail/scheduler/scheduler.cpp index 8d75fcf3213bb..c36ff2acbb21a 100644 --- a/sycl/source/detail/scheduler/scheduler.cpp +++ b/sycl/source/detail/scheduler/scheduler.cpp @@ -587,13 +587,13 @@ void Scheduler::cleanupAuxiliaryResources(BlockingT Blocking) { } ur_kernel_handle_t Scheduler::completeSpecConstMaterialization( - [[maybe_unused]] QueueImplPtr Queue, + [[maybe_unused]] const QueueImplPtr &Queue, [[maybe_unused]] const RTDeviceBinaryImage *BinImage, [[maybe_unused]] const std::string &KernelName, [[maybe_unused]] std::vector &SpecConstBlob) { #if SYCL_EXT_JIT_ENABLE && !_WIN32 return detail::jit_compiler::get_instance().materializeSpecConstants( - std::move(Queue), BinImage, KernelName, SpecConstBlob); + Queue, BinImage, KernelName, SpecConstBlob); #else // SYCL_EXT_JIT_ENABLE && !_WIN32 if (detail::SYCLConfig::get() > 0) { std::cerr << "WARNING: Materialization of spec constants not supported by " diff --git a/sycl/source/detail/scheduler/scheduler.hpp b/sycl/source/detail/scheduler/scheduler.hpp index f3ce947b32e5d..43c70ea55e2fd 100644 --- a/sycl/source/detail/scheduler/scheduler.hpp +++ b/sycl/source/detail/scheduler/scheduler.hpp @@ -453,7 +453,7 @@ class Scheduler { void deferMemObjRelease(const std::shared_ptr &MemObj); ur_kernel_handle_t completeSpecConstMaterialization( - QueueImplPtr Queue, const RTDeviceBinaryImage *BinImage, + const QueueImplPtr &Queue, const RTDeviceBinaryImage *BinImage, const std::string &KernelName, std::vector &SpecConstBlob); void releaseResources(BlockingT Blocking = BlockingT::BLOCKING); From db6a8e37b456834aca77047e4155fceae9a46950 Mon Sep 17 00:00:00 2001 From: "Garcia Orozco, David" Date: Tue, 25 Mar 2025 07:30:03 -0700 Subject: [PATCH 3/5] Make strings not const so move ctor is called automatically --- sycl/source/detail/config.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sycl/source/detail/config.hpp b/sycl/source/detail/config.hpp index f031f2409ac79..df50ba01adbcc 100644 --- a/sycl/source/detail/config.hpp +++ b/sycl/source/detail/config.hpp @@ -647,12 +647,12 @@ template <> class SYCLConfig { public: static std::string get() { - const std::string DefaultValue{""}; + std::string DefaultValue{""}; const char *ValStr = getCachedValue(); if (!ValStr) - return std::move(DefaultValue); + return DefaultValue; return std::string{ValStr}; } @@ -675,7 +675,7 @@ template <> class SYCLConfig { public: static std::string get() { - const std::string DefaultValue{""}; + std::string DefaultValue{""}; const char *ValStr = getCachedValue(); From 8f37c823572c03e0726212862594b81f4c40363c Mon Sep 17 00:00:00 2001 From: "Garcia Orozco, David" Date: Tue, 25 Mar 2025 08:44:22 -0700 Subject: [PATCH 4/5] Revert change to `getSyclObjImpl` call --- sycl/source/detail/jit_compiler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sycl/source/detail/jit_compiler.cpp b/sycl/source/detail/jit_compiler.cpp index 13a73a5ba8fc6..c5fdfe730c55c 100644 --- a/sycl/source/detail/jit_compiler.cpp +++ b/sycl/source/detail/jit_compiler.cpp @@ -1044,8 +1044,7 @@ jit_compiler::fuseKernels(QueueImplPtr Queue, std::shared_ptr KernelBundleImplPtr; if (TargetFormat == ::jit_compiler::BinaryFormat::SPIRV) { detail::getSyclObjImpl(get_kernel_bundle( - Queue->get_context(), {Queue->get_device()}, - {std::move(FusedKernelId)})); + Queue->get_context(), {Queue->get_device()}, {FusedKernelId})); } std::unique_ptr FusedCG; From 51840e57cbac88910c69866147dc022414e3fe7b Mon Sep 17 00:00:00 2001 From: "Garcia Orozco, David" Date: Tue, 25 Mar 2025 09:31:27 -0700 Subject: [PATCH 5/5] Remove dead code --- sycl/source/detail/jit_compiler.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sycl/source/detail/jit_compiler.cpp b/sycl/source/detail/jit_compiler.cpp index c5fdfe730c55c..e24e49cf68463 100644 --- a/sycl/source/detail/jit_compiler.cpp +++ b/sycl/source/detail/jit_compiler.cpp @@ -1042,10 +1042,6 @@ jit_compiler::fuseKernels(QueueImplPtr Queue, FusedOrCachedKernelName); std::shared_ptr KernelBundleImplPtr; - if (TargetFormat == ::jit_compiler::BinaryFormat::SPIRV) { - detail::getSyclObjImpl(get_kernel_bundle( - Queue->get_context(), {Queue->get_device()}, {FusedKernelId})); - } std::unique_ptr FusedCG; FusedCG.reset(new detail::CGExecKernel(