Make implicit local arg member const

Author: sergey-semenov

sycl/source/detail/device_kernel_info.cpp

@@ -12,8 +12,10 @@ namespace sycl {
 inline namespace _V1 {
 namespace detail {
 
-DeviceKernelInfo::DeviceKernelInfo(const CompileTimeKernelInfoTy &Info)
-    : CompileTimeKernelInfoTy(Info) {}
+DeviceKernelInfo::DeviceKernelInfo(std::string_view Name,
+                                   std::optional<int> ImplicitLocalArgPos)
+    : CompileTimeKernelInfoTy{Name}, MImplicitLocalArgPos{ImplicitLocalArgPos} {
+}
 
 template <typename OtherTy>
 inline constexpr bool operator==(const CompileTimeKernelInfoTy &LHS,
@@ -42,11 +44,6 @@ void DeviceKernelInfo::setCompileTimeInfoIfNeeded(
   assert(isCompileTimeInfoSet());
   assert(Info == *this);
 }
-
-void DeviceKernelInfo::setImplicitLocalArgPos(int Pos) {
-  assert(!MImplicitLocalArgPos.has_value() || MImplicitLocalArgPos == Pos);
-  MImplicitLocalArgPos = Pos;
-}
 } // namespace detail
 } // namespace _V1
 } // namespace sycl

sycl/source/detail/device_kernel_info.hpp

@@ -89,9 +89,9 @@ struct FastKernelSubcacheT {
 // into this structure and get rid of the other KernelName -> * maps.
 class DeviceKernelInfo : public CompileTimeKernelInfoTy {
 public:
-  DeviceKernelInfo(const CompileTimeKernelInfoTy &Info);
+  DeviceKernelInfo(std::string_view Name,
+                   std::optional<int> ImplicitLocalArgPos = {});
 
-  void init(std::string_view KernelName);
   void setCompileTimeInfoIfNeeded(const CompileTimeKernelInfoTy &Info);
 
   FastKernelSubcacheT &getKernelSubcache() { return MFastKernelSubcache; }
@@ -100,13 +100,11 @@ class DeviceKernelInfo : public CompileTimeKernelInfoTy {
     return MImplicitLocalArgPos;
   }
 
-  void setImplicitLocalArgPos(int Pos);
-
 private:
   bool isCompileTimeInfoSet() const { return KernelSize != 0; }
 
   FastKernelSubcacheT MFastKernelSubcache;
-  std::optional<int> MImplicitLocalArgPos;
+  const std::optional<int> MImplicitLocalArgPos;
 };
 
 } // namespace detail

sycl/source/detail/kernel_impl.cpp

@@ -16,11 +16,6 @@ namespace sycl {
 inline namespace _V1 {
 namespace detail {
 
-static CompileTimeKernelInfoTy
-createCompileTimeKernelInfo(std::string_view KernelName = {}) {
-  return CompileTimeKernelInfoTy{KernelName};
-}
-
 kernel_impl::kernel_impl(Managed<ur_kernel_handle_t> &&Kernel,
                          context_impl &Context,
                          kernel_bundle_impl *KernelBundleImpl,
@@ -32,7 +27,7 @@ kernel_impl::kernel_impl(Managed<ur_kernel_handle_t> &&Kernel,
       MKernelBundleImpl(KernelBundleImpl ? KernelBundleImpl->shared_from_this()
                                          : nullptr),
       MIsInterop(true), MKernelArgMaskPtr{ArgMask}, MOwnsDeviceKernelInfo(true),
-      MDeviceKernelInfo(createCompileTimeKernelInfo(getName())) {
+      MDeviceKernelInfo(getName()) {
   ur_context_handle_t UrContext = nullptr;
   // Using the adapter from the passed ContextImpl
   getAdapter().call<UrApiKind::urKernelGetInfo>(
@@ -60,9 +55,8 @@ kernel_impl::kernel_impl(Managed<ur_kernel_handle_t> &&Kernel,
       MIsInterop(MDeviceImageImpl->getOriginMask() & ImageOriginInterop),
       MKernelArgMaskPtr{ArgMask}, MCacheMutex{CacheMutex},
       MOwnsDeviceKernelInfo(checkOwnsDeviceKernelInfo()),
-      MDeviceKernelInfo(MOwnsDeviceKernelInfo
-                            ? createCompileTimeKernelInfo(getName())
-                            : createCompileTimeKernelInfo()) {
+      MDeviceKernelInfo(MOwnsDeviceKernelInfo ? getName()
+                                              : std::string_view()) {
 
   // Enable USM indirect access for interop and non-sycl-jit source kernels.
   // sycl-jit kernels will enable this if needed through the regular kernel

sycl/source/detail/program_manager/program_manager.cpp

@@ -1788,18 +1788,6 @@ Managed<ur_program_handle_t> ProgramManager::build(
   return LinkedProg;
 }
 
-void ProgramManager::cacheKernelImplicitLocalArg(
-    const RTDeviceBinaryImage &Img) {
-  const RTDeviceBinaryImage::PropertyRange &ImplicitLocalArgRange =
-      Img.getImplicitLocalArg();
-  if (ImplicitLocalArgRange.isAvailable())
-    for (auto Prop : ImplicitLocalArgRange) {
-      auto It = m_DeviceKernelInfoMap.find(Prop->Name);
-      assert(It != m_DeviceKernelInfoMap.end());
-      It->second.setImplicitLocalArgPos(DeviceBinaryProperty(Prop).asUint32());
-    }
-}
-
 DeviceKernelInfo &
 ProgramManager::getDeviceKernelInfo(const CompileTimeKernelInfoTy &Info) {
   std::lock_guard<std::mutex> Guard(m_DeviceKernelInfoMapMutex);
@@ -2000,6 +1988,16 @@ void ProgramManager::addImage(sycl_device_binary RawImg,
       m_BinImg2KernelIDs[Img.get()];
   KernelIDs.reset(new std::vector<kernel_id>);
 
+  std::unordered_map<std::string_view, int> ImplicitLocalArgPositions;
+  const RTDeviceBinaryImage::PropertyRange &ImplicitLocalArgRange =
+      Img->getImplicitLocalArg();
+  if (ImplicitLocalArgRange.isAvailable())
+    for (auto Prop : ImplicitLocalArgRange) {
+      auto Result = ImplicitLocalArgPositions.try_emplace(
+          Prop->Name, DeviceBinaryProperty(Prop).asUint32());
+      assert(Result.second && "Duplicate implicit arg property");
+    }
+
   for (sycl_offload_entry EntriesIt = EntriesB; EntriesIt != EntriesE;
        EntriesIt = EntriesIt->Increment()) {
 
@@ -2024,10 +2022,14 @@ void ProgramManager::addImage(sycl_device_binary RawImg,
     m_KernelIDs2BinImage.insert(std::make_pair(It->second, Img.get()));
     KernelIDs->push_back(It->second);
 
-    CompileTimeKernelInfoTy DefaultCompileTimeInfo{std::string_view(name)};
-    m_DeviceKernelInfoMap.try_emplace(std::string_view(name),
-                                      DefaultCompileTimeInfo);
-
+    std::optional<int> ImplicitLocalArgPos;
+    auto ImplicitLocalArgPosIt = ImplicitLocalArgPositions.find(name);
+    if (ImplicitLocalArgPosIt != ImplicitLocalArgPositions.end())
+      ImplicitLocalArgPos = ImplicitLocalArgPosIt->second;
+    auto Result =
+        m_DeviceKernelInfoMap.try_emplace(name, name, ImplicitLocalArgPos);
+    assert(ImplicitLocalArgPos == Result->first.getImplicitLocalArgPos() &&
+           "Conflicting values of implicit local arg positions");
     // Keep track of image to kernel name reference count for cleanup.
     m_KernelNameRefCount[name]++;
   }
@@ -2048,8 +2050,6 @@ void ProgramManager::addImage(sycl_device_binary RawImg,
     }
   }
 
-  cacheKernelImplicitLocalArg(*Img);
-
   // Sort kernel ids for faster search
   std::sort(KernelIDs->begin(), KernelIDs->end(), LessByHash<kernel_id>{});
 

sycl/source/detail/program_manager/program_manager.hpp

@@ -393,8 +393,6 @@ class ProgramManager {
 
   SanitizerType kernelUsesSanitizer() const { return m_SanitizerFoundInImage; }
 
-  void cacheKernelImplicitLocalArg(const RTDeviceBinaryImage &Img);
-
   DeviceKernelInfo &getDeviceKernelInfo(const CompileTimeKernelInfoTy &Info);
   DeviceKernelInfo &getDeviceKernelInfo(std::string_view KernelName);
   DeviceKernelInfo *tryGetDeviceKernelInfo(std::string_view KernelName);