diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index cbd590bd686b5..e7c2329136751 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -756,35 +756,42 @@ ProgramManager::collectDependentDeviceImagesForVirtualFunctions( } } - while (!WorkList.empty()) { - std::string SetName = WorkList.front(); - WorkList.pop(); + if (!WorkList.empty()) { + // Guard read access to m_VFSet2BinImage: + // TODO: a better solution should be sought in the future, i.e. a different + // mutex than m_KernelIDsMutex, check lock check pattern, etc. + std::lock_guard KernelIDsGuard(m_KernelIDsMutex); - // There could be more than one device image that uses the same set - // of virtual functions, or provides virtual funtions from the same - // set. - for (RTDeviceBinaryImage *BinImage : m_VFSet2BinImage[SetName]) { - // Here we can encounter both uses-virtual-functions-set and - // virtual-functions-set properties, but their handling is the same: we - // just grab all sets they reference and add them for consideration if - // we haven't done so already. - for (const sycl_device_binary_property &VFProp : - BinImage->getVirtualFunctions()) { - std::string StrValue = DeviceBinaryProperty(VFProp).asCString(); - for (const auto &SetName : detail::split_string(StrValue, ',')) { - if (HandledSets.insert(SetName).second) - WorkList.push(SetName); + while (!WorkList.empty()) { + std::string SetName = WorkList.front(); + WorkList.pop(); + + // There could be more than one device image that uses the same set + // of virtual functions, or provides virtual funtions from the same + // set. + for (RTDeviceBinaryImage *BinImage : m_VFSet2BinImage.at(SetName)) { + // Here we can encounter both uses-virtual-functions-set and + // virtual-functions-set properties, but their handling is the same: we + // just grab all sets they reference and add them for consideration if + // we haven't done so already. + for (const sycl_device_binary_property &VFProp : + BinImage->getVirtualFunctions()) { + std::string StrValue = DeviceBinaryProperty(VFProp).asCString(); + for (const auto &SetName : detail::split_string(StrValue, ',')) { + if (HandledSets.insert(SetName).second) + WorkList.push(SetName); + } } - } - // TODO: Complete this part about handling of incompatible device images. - // If device image uses the same virtual function set, then we only - // link it if it is compatible. - // However, if device image provides virtual function set and it is - // incompatible, then we should link its "dummy" version to avoid link - // errors about unresolved external symbols. - if (doesDevSupportDeviceRequirements(Dev, *BinImage)) - DeviceImagesToLink.insert(BinImage); + // TODO: Complete this part about handling of incompatible device + // images. If device image uses the same virtual function set, then we + // only link it if it is compatible. However, if device image provides + // virtual function set and it is incompatible, then we should link its + // "dummy" version to avoid link errors about unresolved external + // symbols. + if (doesDevSupportDeviceRequirements(Dev, *BinImage)) + DeviceImagesToLink.insert(BinImage); + } } } @@ -2146,8 +2153,14 @@ void ProgramManager::addImages(sycl_device_binaries DeviceBinary) { } void ProgramManager::removeImages(sycl_device_binaries DeviceBinary) { + if (DeviceBinary->NumDeviceBinaries == 0) + return; + // Acquire lock to read and modify maps for kernel bundles + std::lock_guard KernelIDsGuard(m_KernelIDsMutex); + for (int I = 0; I < DeviceBinary->NumDeviceBinaries; I++) { sycl_device_binary RawImg = &(DeviceBinary->DeviceBinaries[I]); + auto DevImgIt = m_DeviceImages.find(RawImg); if (DevImgIt == m_DeviceImages.end()) continue; @@ -2161,9 +2174,6 @@ void ProgramManager::removeImages(sycl_device_binaries DeviceBinary) { // Drop the kernel argument mask map m_EliminatedKernelArgMasks.erase(Img); - // Acquire lock to modify maps for kernel bundles - std::lock_guard KernelIDsGuard(m_KernelIDsMutex); - // Unmap the unique kernel IDs for the offload entries for (sycl_offload_entry EntriesIt = EntriesB; EntriesIt != EntriesE; EntriesIt = EntriesIt->Increment()) { diff --git a/sycl/source/detail/program_manager/program_manager.hpp b/sycl/source/detail/program_manager/program_manager.hpp index 906692cdcac25..35843ea24def5 100644 --- a/sycl/source/detail/program_manager/program_manager.hpp +++ b/sycl/source/detail/program_manager/program_manager.hpp @@ -460,6 +460,7 @@ class ProgramManager { /// Keeps all device images we are refering to during program lifetime. Used /// for proper cleanup. + /// Access must be guarded by the m_KernelIDsMutex mutex. std::unordered_map m_DeviceImages; @@ -469,6 +470,7 @@ class ProgramManager { /// Caches list of device images that use or provide virtual functions from /// the same set. Used to simplify access. + /// Access must be guarded by the m_KernelIDsMutex mutex. std::unordered_map> m_VFSet2BinImage;