From af831bd3690405e96d5592da4aecac5ff39d6b94 Mon Sep 17 00:00:00 2001 From: Steffen Larsen Date: Fri, 10 Sep 2021 12:24:55 +0300 Subject: [PATCH 1/2] [SYCL] Remove unnecessary kernel ID protector mutex The kernel ID cache is only written to during `ProgramManager::AddImages` which is done at the start of the execution. All subsequent accesses to the kernel ID cache are reads. As such, the mutex that protects the kernel ID cache is only guarding reads, which adds an unnecessary overhead. Signed-off-by: Steffen Larsen --- sycl/source/detail/program_manager/program_manager.cpp | 6 ------ sycl/source/detail/program_manager/program_manager.hpp | 7 ------- 2 files changed, 13 deletions(-) diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index b916039bda9ee..6fa277f8e61a3 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -1038,7 +1038,6 @@ void ProgramManager::addImages(pi_device_binaries DeviceBinary) { // ... or create the set first if it hasn't been KernelSetId KSId = getNextKernelSetId(); { - std::lock_guard KernelIDsGuard(m_KernelIDsMutex); for (_pi_offload_entry EntriesIt = EntriesB; EntriesIt != EntriesE; ++EntriesIt) { auto Result = KSIdMap.insert(std::make_pair(EntriesIt->name, KSId)); @@ -1259,16 +1258,12 @@ static bool compatibleWithDevice(RTDeviceBinaryImage *BinImage, } kernel_id ProgramManager::getSYCLKernelID(const std::string &KernelName) { - std::lock_guard KernelIDsGuard(m_KernelIDsMutex); - auto KernelID = m_KernelIDs.find(KernelName); assert(KernelID != m_KernelIDs.end() && "Kernel ID missing"); return KernelID->second; } std::vector ProgramManager::getAllSYCLKernelIDs() { - std::lock_guard KernelIDsGuard(m_KernelIDsMutex); - std::vector AllKernelIDs; AllKernelIDs.reserve(m_KernelIDs.size()); for (std::pair KernelID : m_KernelIDs) { @@ -1329,7 +1324,6 @@ ProgramManager::getSYCLDeviceImagesWithCompatibleState( pi_device_binary DevBin = const_cast(&BinImage->getRawData()); { - std::lock_guard KernelIDsGuard(m_KernelIDsMutex); for (_pi_offload_entry EntriesIt = DevBin->EntriesBegin; EntriesIt != DevBin->EntriesEnd; ++EntriesIt) { auto KernelID = m_KernelIDs.find(EntriesIt->name); diff --git a/sycl/source/detail/program_manager/program_manager.hpp b/sycl/source/detail/program_manager/program_manager.hpp index 04c7202499721..26fab51c1b22f 100644 --- a/sycl/source/detail/program_manager/program_manager.hpp +++ b/sycl/source/detail/program_manager/program_manager.hpp @@ -275,15 +275,8 @@ class ProgramManager { /// Maps names of kernels to their unique kernel IDs. /// TODO: Use std::unordered_set with transparent hash and equality functions /// when C++20 is enabled for the runtime library. - /// Access must be guarded by the m_KernelIDsMutex mutex std::unordered_map m_KernelIDs; - /// Protects kernel ID cache. - /// NOTE: This may be acquired while \ref Sync::getGlobalLock() is held so to - /// avoid deadlocks care must be taken not to acquire - /// \ref Sync::getGlobalLock() while holding this mutex. - std::mutex m_KernelIDsMutex; - // Keeps track of pi_program to image correspondence. Needed for: // - knowing which specialization constants are used in the program and // injecting their current values before compiling the SPIR-V; the binary From 2f5beb5aeae28a1942d80419140d420fa0ab5302 Mon Sep 17 00:00:00 2001 From: Steffen Larsen Date: Fri, 10 Sep 2021 12:58:10 +0300 Subject: [PATCH 2/2] Added note about write-access limitation Signed-off-by: Steffen Larsen --- sycl/source/detail/program_manager/program_manager.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sycl/source/detail/program_manager/program_manager.hpp b/sycl/source/detail/program_manager/program_manager.hpp index 26fab51c1b22f..b86e1864df77b 100644 --- a/sycl/source/detail/program_manager/program_manager.hpp +++ b/sycl/source/detail/program_manager/program_manager.hpp @@ -275,6 +275,7 @@ class ProgramManager { /// Maps names of kernels to their unique kernel IDs. /// TODO: Use std::unordered_set with transparent hash and equality functions /// when C++20 is enabled for the runtime library. + /// Write access is only allowed during start-up (addImages). std::unordered_map m_KernelIDs; // Keeps track of pi_program to image correspondence. Needed for: