Skip to content

Commit 37a9023

Browse files
[SYCL][NFC] Fix Coverity hits (#16752)
Also, make kernel_bundle_impl member variable naming consistent.
1 parent 4cbae16 commit 37a9023

File tree

2 files changed

+36
-35
lines changed

2 files changed

+36
-35
lines changed

sycl/source/detail/kernel_bundle_impl.hpp

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -353,16 +353,16 @@ class kernel_bundle_impl {
353353
kernel_bundle_impl(const context &Context, syclex::source_language Lang,
354354
const std::string &Src, include_pairs_t IncludePairsVec)
355355
: MContext(Context), MDevices(Context.get_devices()),
356-
MState(bundle_state::ext_oneapi_source), Language(Lang), Source(Src),
357-
IncludePairs(IncludePairsVec) {}
356+
MState(bundle_state::ext_oneapi_source), MLanguage(Lang), MSource(Src),
357+
MIncludePairs(IncludePairsVec) {}
358358

359359
// oneapi_ext_kernel_compiler
360360
// construct from source bytes
361361
kernel_bundle_impl(const context &Context, syclex::source_language Lang,
362362
const std::vector<std::byte> &Bytes)
363363
: MContext(Context), MDevices(Context.get_devices()),
364-
MState(bundle_state::ext_oneapi_source), Language(Lang), Source(Bytes) {
365-
}
364+
MState(bundle_state::ext_oneapi_source), MLanguage(Lang),
365+
MSource(Bytes) {}
366366

367367
// oneapi_ext_kernel_compiler
368368
// interop constructor
@@ -372,8 +372,8 @@ class kernel_bundle_impl {
372372
syclex::source_language Lang)
373373
: kernel_bundle_impl(Ctx, Devs, DevImage) {
374374
MState = bundle_state::executable;
375-
KernelNames = KNames;
376-
Language = Lang;
375+
MKernelNames = std::move(KNames);
376+
MLanguage = Lang;
377377
}
378378

379379
// oneapi_ext_kernel_compiler
@@ -382,17 +382,18 @@ class kernel_bundle_impl {
382382
const std::vector<kernel_id> &KernelIDs,
383383
std::vector<std::string> KNames, std::string Pfx,
384384
syclex::source_language Lang)
385-
: kernel_bundle_impl(Ctx, Devs, KernelIDs, bundle_state::executable) {
385+
: kernel_bundle_impl(std::move(Ctx), std::move(Devs), KernelIDs,
386+
bundle_state::executable) {
386387
assert(Lang == syclex::source_language::sycl_jit);
387388
// Mark this bundle explicitly as "interop" to ensure that its kernels are
388389
// enqueued with the info from the kernel object passed by the application,
389390
// cf. `enqueueImpKernel` in `commands.cpp`. While runtime-compiled kernels
390391
// loaded via the program manager have `kernel_id`s, they can't be looked up
391392
// from the (unprefixed) kernel name.
392393
MIsInterop = true;
393-
KernelNames = KNames;
394-
Prefix = Pfx;
395-
Language = Lang;
394+
MKernelNames = std::move(KNames);
395+
MPrefix = std::move(Pfx);
396+
MLanguage = Lang;
396397
}
397398

398399
std::string trimXsFlags(std::string &str) {
@@ -493,13 +494,14 @@ class kernel_bundle_impl {
493494
DeviceVec.push_back(Dev);
494495
}
495496

496-
if (Language == syclex::source_language::sycl_jit) {
497+
if (MLanguage == syclex::source_language::sycl_jit) {
497498
// Build device images via the program manager.
498499
// TODO: Support persistent caching.
499500

500-
const std::string &SourceStr = std::get<std::string>(this->Source);
501+
const std::string &SourceStr = std::get<std::string>(MSource);
501502
auto [Binaries, CompilationID] = syclex::detail::SYCL_JIT_to_SPIRV(
502-
SourceStr, IncludePairs, BuildOptions, LogPtr, RegisteredKernelNames);
503+
SourceStr, MIncludePairs, BuildOptions, LogPtr,
504+
RegisteredKernelNames);
503505

504506
auto &PM = detail::ProgramManager::getInstance();
505507
PM.addImages(Binaries);
@@ -519,12 +521,12 @@ class kernel_bundle_impl {
519521
}
520522

521523
return std::make_shared<kernel_bundle_impl>(
522-
MContext, MDevices, KernelIDs, KernelNames, Prefix, Language);
524+
MContext, MDevices, KernelIDs, KernelNames, Prefix, MLanguage);
523525
}
524526

525527
ur_program_handle_t UrProgram = nullptr;
526528
// SourceStrPtr will be null when source is Spir-V bytes.
527-
const std::string *SourceStrPtr = std::get_if<std::string>(&this->Source);
529+
const std::string *SourceStrPtr = std::get_if<std::string>(&MSource);
528530
bool FetchedFromCache = false;
529531
if (PersistentDeviceCodeCache::isEnabled() && SourceStrPtr) {
530532
FetchedFromCache = extKernelCompilerFetchFromCache(
@@ -533,7 +535,7 @@ class kernel_bundle_impl {
533535

534536
if (!FetchedFromCache) {
535537
const auto spirv = [&]() -> std::vector<uint8_t> {
536-
if (Language == syclex::source_language::opencl) {
538+
if (MLanguage == syclex::source_language::opencl) {
537539
// if successful, the log is empty. if failed, throws an error with
538540
// the compilation log.
539541
std::vector<uint32_t> IPVersionVec(Devices.size());
@@ -548,17 +550,16 @@ class kernel_bundle_impl {
548550
return syclex::detail::OpenCLC_to_SPIRV(*SourceStrPtr, IPVersionVec,
549551
BuildOptions, LogPtr);
550552
}
551-
if (Language == syclex::source_language::spirv) {
552-
const auto &SourceBytes =
553-
std::get<std::vector<std::byte>>(this->Source);
553+
if (MLanguage == syclex::source_language::spirv) {
554+
const auto &SourceBytes = std::get<std::vector<std::byte>>(MSource);
554555
std::vector<uint8_t> Result(SourceBytes.size());
555556
std::transform(SourceBytes.cbegin(), SourceBytes.cend(),
556557
Result.begin(),
557558
[](std::byte B) { return static_cast<uint8_t>(B); });
558559
return Result;
559560
}
560-
if (Language == syclex::source_language::sycl) {
561-
return syclex::detail::SYCL_to_SPIRV(*SourceStrPtr, IncludePairs,
561+
if (MLanguage == syclex::source_language::sycl) {
562+
return syclex::detail::SYCL_to_SPIRV(*SourceStrPtr, MIncludePairs,
562563
BuildOptions, LogPtr,
563564
RegisteredKernelNames);
564565
}
@@ -623,7 +624,7 @@ class kernel_bundle_impl {
623624
}
624625

625626
return std::make_shared<kernel_bundle_impl>(MContext, MDevices, DevImg,
626-
KernelNames, Language);
627+
KernelNames, MLanguage);
627628
}
628629

629630
std::string adjust_kernel_name(const std::string &Name,
@@ -638,29 +639,29 @@ class kernel_bundle_impl {
638639
}
639640

640641
bool ext_oneapi_has_kernel(const std::string &Name) {
641-
auto it = std::find(KernelNames.begin(), KernelNames.end(),
642-
adjust_kernel_name(Name, Language));
643-
return it != KernelNames.end();
642+
auto it = std::find(MKernelNames.begin(), MKernelNames.end(),
643+
adjust_kernel_name(Name, MLanguage));
644+
return it != MKernelNames.end();
644645
}
645646

646647
kernel
647648
ext_oneapi_get_kernel(const std::string &Name,
648649
const std::shared_ptr<kernel_bundle_impl> &Self) {
649-
if (KernelNames.empty())
650+
if (MKernelNames.empty())
650651
throw sycl::exception(make_error_code(errc::invalid),
651652
"'ext_oneapi_get_kernel' is only available in "
652653
"kernel_bundles successfully built from "
653654
"kernel_bundle<bundle_state:ext_oneapi_source>.");
654655

655-
std::string AdjustedName = adjust_kernel_name(Name, Language);
656+
std::string AdjustedName = adjust_kernel_name(Name, MLanguage);
656657
if (!ext_oneapi_has_kernel(Name))
657658
throw sycl::exception(make_error_code(errc::invalid),
658659
"kernel '" + AdjustedName +
659660
"' not found in kernel_bundle");
660661

661-
if (Language == syclex::source_language::sycl_jit) {
662+
if (MLanguage == syclex::source_language::sycl_jit) {
662663
auto &PM = ProgramManager::getInstance();
663-
auto KID = PM.getSYCLKernelID(Prefix + AdjustedName);
664+
auto KID = PM.getSYCLKernelID(MPrefix + AdjustedName);
664665

665666
for (const auto &DevImgWithDeps : MDeviceImages) {
666667
const auto &DevImg = DevImgWithDeps.getMain();
@@ -954,12 +955,12 @@ class kernel_bundle_impl {
954955

955956
// ext_oneapi_kernel_compiler : Source, Languauge, KernelNames, IncludePairs
956957
// Language is for both state::source and state::executable.
957-
syclex::source_language Language = syclex::source_language::opencl;
958-
const std::variant<std::string, std::vector<std::byte>> Source;
958+
syclex::source_language MLanguage = syclex::source_language::opencl;
959+
const std::variant<std::string, std::vector<std::byte>> MSource;
959960
// only kernel_bundles created from source have KernelNames member.
960-
std::vector<std::string> KernelNames;
961-
std::string Prefix;
962-
include_pairs_t IncludePairs;
961+
std::vector<std::string> MKernelNames;
962+
std::string MPrefix;
963+
include_pairs_t MIncludePairs;
963964
};
964965

965966
} // namespace detail

sycl/source/detail/program_manager/program_manager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(
796796
// built for the root device
797797
DeviceImplPtr RootDevImpl = DeviceImpl;
798798
while (!RootDevImpl->isRootDevice()) {
799-
auto ParentDev = detail::getSyclObjImpl(
799+
const auto &ParentDev = detail::getSyclObjImpl(
800800
RootDevImpl->get_info<info::device::parent_device>());
801801
// Sharing is allowed within a single context only
802802
if (!ContextImpl->hasDevice(ParentDev))

0 commit comments

Comments
 (0)