Skip to content

Commit 12134d7

Browse files
[SYCL] Fix warnings in source/detail code (#16334)
Fixes places with copy instead of usage by reference or move. Protected potential but unlikely constant overflow with exception. --------- Signed-off-by: Tikhomirova, Kseniya <[email protected]>
1 parent a8c8dc8 commit 12134d7

File tree

6 files changed

+21
-13
lines changed

6 files changed

+21
-13
lines changed

sycl/source/detail/helpers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ retrieveKernelBinary(const QueueImplPtr &Queue, const char *KernelName,
9494
DeviceImage = &detail::ProgramManager::getInstance().getDeviceImage(
9595
KernelName, Context, Device);
9696
Program = detail::ProgramManager::getInstance().createURProgram(
97-
*DeviceImage, Context, {Device});
97+
*DeviceImage, Context, {std::move(Device)});
9898
}
9999
return {DeviceImage, Program};
100100
}

sycl/source/detail/persistent_device_code_cache.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ std::vector<std::vector<char>> PersistentDeviceCodeCache::getItemFromDisc(
320320
std::vector<std::vector<char>>
321321
PersistentDeviceCodeCache::getCompiledKernelFromDisc(
322322
const std::vector<device> &Devices, const std::string &BuildOptionsString,
323-
const std::string SourceStr) {
323+
const std::string &SourceStr) {
324324
assert(!Devices.empty());
325325
std::vector<std::vector<char>> Binaries(Devices.size());
326326
std::string FileNames;
@@ -518,7 +518,7 @@ std::string PersistentDeviceCodeCache::getCacheItemPath(
518518

519519
std::string PersistentDeviceCodeCache::getCompiledKernelItemPath(
520520
const device &Device, const std::string &BuildOptionsString,
521-
const std::string SourceString) {
521+
const std::string &SourceString) {
522522

523523
std::string cache_root{getRootDir()};
524524
if (cache_root.empty()) {

sycl/source/detail/persistent_device_code_cache.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class PersistentDeviceCodeCache {
170170
static std::string
171171
getCompiledKernelItemPath(const device &Device,
172172
const std::string &BuildOptionsString,
173-
const std::string SourceString);
173+
const std::string &SourceString);
174174

175175
/* Program binaries built for one or more devices are read from persistent
176176
* cache and returned in form of vector of programs. Each binary program is
@@ -185,7 +185,7 @@ class PersistentDeviceCodeCache {
185185
static std::vector<std::vector<char>>
186186
getCompiledKernelFromDisc(const std::vector<device> &Devices,
187187
const std::string &BuildOptionsString,
188-
const std::string SourceStr);
188+
const std::string &SourceStr);
189189

190190
/* Stores build program in persistent cache
191191
*/

sycl/source/detail/program_manager/program_manager.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ static ur_program_handle_t
7676
createBinaryProgram(const ContextImplPtr Context,
7777
const std::vector<device> &Devices,
7878
const uint8_t **Binaries, size_t *Lengths,
79-
const std::vector<ur_program_metadata_t> Metadata) {
79+
const std::vector<ur_program_metadata_t> &Metadata) {
8080
const AdapterPtr &Adapter = Context->getAdapter();
8181
ur_program_handle_t Program;
8282
std::vector<ur_device_handle_t> DeviceHandles;
@@ -230,7 +230,7 @@ ProgramManager::createURProgram(const RTDeviceBinaryImage &Img,
230230
"SPIR-V online compilation is not supported in this context");
231231

232232
// Get program metadata from properties
233-
auto ProgMetadata = Img.getProgramMetadataUR();
233+
const auto &ProgMetadata = Img.getProgramMetadataUR();
234234

235235
// Load the image
236236
const ContextImplPtr Ctx = getSyclObjImpl(Context);
@@ -990,7 +990,15 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(
990990
// emplace all subsets of the current set of devices into the cache.
991991
// Set of all devices is not included in the loop as it was already added
992992
// into the cache.
993-
for (int Mask = 1; Mask < (1 << URDevicesSet.size()) - 1; ++Mask) {
993+
int Mask = 1;
994+
if (URDevicesSet.size() > sizeof(Mask) * 8 - 1) {
995+
// Protection for the algorithm below. Although overflow is very unlikely
996+
// to be reached.
997+
throw sycl::exception(
998+
make_error_code(errc::runtime),
999+
"Unable to cache built program for more than 31 devices");
1000+
}
1001+
for (; Mask < (1 << URDevicesSet.size()) - 1; ++Mask) {
9941002
std::set<ur_device_handle_t> Subset;
9951003
int Index = 0;
9961004
for (auto It = URDevicesSet.begin(); It != URDevicesSet.end();
@@ -1124,7 +1132,7 @@ ProgramManager::getUrProgramFromUrKernel(ur_kernel_handle_t Kernel,
11241132

11251133
std::string
11261134
ProgramManager::getProgramBuildLog(const ur_program_handle_t &Program,
1127-
const ContextImplPtr Context) {
1135+
const ContextImplPtr &Context) {
11281136
size_t URDevicesSize = 0;
11291137
const AdapterPtr &Adapter = Context->getAdapter();
11301138
Adapter->call<UrApiKind::urProgramGetInfo>(Program, UR_PROGRAM_INFO_DEVICES,

sycl/source/detail/program_manager/program_manager.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ class ProgramManager {
220220
void addImages(sycl_device_binaries DeviceImages);
221221
void debugPrintBinaryImages() const;
222222
static std::string getProgramBuildLog(const ur_program_handle_t &Program,
223-
const ContextImplPtr Context);
223+
const ContextImplPtr &Context);
224224

225225
uint32_t getDeviceLibReqMask(const RTDeviceBinaryImage &Img);
226226

sycl/source/detail/scheduler/commands.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ namespace detail {
5959
template <typename MemOpFuncT, typename... MemOpArgTs>
6060
ur_result_t callMemOpHelper(MemOpFuncT &MemOpFunc, MemOpArgTs &&...MemOpArgs) {
6161
try {
62-
MemOpFunc(MemOpArgs...);
62+
MemOpFunc(std::forward<MemOpArgTs>(MemOpArgs)...);
6363
} catch (sycl::exception &e) {
6464
return static_cast<ur_result_t>(get_ur_error(e));
6565
}
@@ -70,7 +70,7 @@ template <typename MemOpRet, typename MemOpFuncT, typename... MemOpArgTs>
7070
ur_result_t callMemOpHelperRet(MemOpRet &MemOpResult, MemOpFuncT &MemOpFunc,
7171
MemOpArgTs &&...MemOpArgs) {
7272
try {
73-
MemOpResult = MemOpFunc(MemOpArgs...);
73+
MemOpResult = MemOpFunc(std::forward<MemOpArgTs>(MemOpArgs)...);
7474
} catch (sycl::exception &e) {
7575
return static_cast<ur_result_t>(get_ur_error(e));
7676
}
@@ -2891,7 +2891,7 @@ ur_result_t ExecCGCommand::enqueueImpCommandBuffer() {
28912891
&RawEvents[0]);
28922892
}
28932893

2894-
ur_exp_command_buffer_sync_point_t OutSyncPoint;
2894+
ur_exp_command_buffer_sync_point_t OutSyncPoint{};
28952895
ur_exp_command_buffer_command_handle_t OutCommand = nullptr;
28962896
switch (MCommandGroup->getType()) {
28972897
case CGType::Kernel: {

0 commit comments

Comments
 (0)