Skip to content

Commit

Permalink
[SYCL] Fix warnings in sycl/source Part 2 (intel#16359)
Browse files Browse the repository at this point in the history
Signed-off-by: Tikhomirova, Kseniya <[email protected]>
  • Loading branch information
KseniyaTikhomirova authored Dec 19, 2024
1 parent 9a8613a commit f541207
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 44 deletions.
10 changes: 5 additions & 5 deletions sycl/source/detail/graph_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion sycl/source/detail/graph_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
24 changes: 13 additions & 11 deletions sycl/source/detail/kernel_program_cache.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}

Expand Down
25 changes: 17 additions & 8 deletions sycl/source/detail/online_compiler/online_compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 != "")
Expand Down
8 changes: 4 additions & 4 deletions sycl/source/detail/program_manager/program_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions sycl/source/detail/scheduler/commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>(
Expand Down
7 changes: 3 additions & 4 deletions sycl/source/handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
}
Expand Down
12 changes: 6 additions & 6 deletions sycl/source/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down

0 comments on commit f541207

Please sign in to comment.