[SYCL] Use image pointer as a unique identifier for cache (#7113)

Currently different paths for building programs use different cache
keys. Specifically, one path uses the kernel-set-id while another uses
the pointer to the binary image. This may cause device binaries built by
both paths may cause the programs to be rebuilt. Additionally, since the
kernel-set-id may cover multiple device binaries, it may make colliding
cache kernels.

To fix the problems this commit will change the key use an image
identifier rather than the kernel-set-id. We use the device binary
pointer as the unique identifier.

Signed-off-by: Larsen, Steffen <[email protected]>

Author: steffenlarsen

sycl/source/detail/kernel_program_cache.hpp

@@ -68,7 +68,7 @@ class KernelProgramCache {
   using PiProgramT = std::remove_pointer<RT::PiProgram>::type;
   using PiProgramPtrT = std::atomic<PiProgramT *>;
   using ProgramWithBuildStateT = BuildResult<PiProgramT>;
-  using ProgramCacheKeyT = std::pair<std::pair<SerializedObj, KernelSetId>,
+  using ProgramCacheKeyT = std::pair<std::pair<SerializedObj, std::uintptr_t>,
                                      std::pair<RT::PiDevice, std::string>>;
   using ProgramCacheT = std::map<ProgramCacheKeyT, ProgramWithBuildStateT>;
   using ContextPtr = context_impl *;

sycl/source/detail/program_manager/program_manager.cpp

@@ -640,9 +640,10 @@ RT::PiProgram ProgramManager::getBuiltPIProgram(
 
   const RT::PiDevice PiDevice = Dev->getHandleRef();
 
+  std::uintptr_t ImgId = reinterpret_cast<uintptr_t>(&Img);
   auto BuildResult = getOrBuild<PiProgramT, compile_program_error>(
       Cache,
-      std::make_pair(std::make_pair(std::move(SpecConsts), KSId),
+      std::make_pair(std::make_pair(std::move(SpecConsts), ImgId),
                      std::make_pair(PiDevice, CompileOpts + LinkOpts)),
       AcquireF, GetF, BuildF);
   // getOrBuild is not supposed to return nullptr
@@ -1993,11 +1994,12 @@ device_image_plain ProgramManager::build(const device_image_plain &DeviceImage,
     return BuiltProgram.release();
   };
 
+  std::uintptr_t ImgId = reinterpret_cast<uintptr_t>(ImgPtr);
   const RT::PiDevice PiDevice = getRawSyclObjImpl(Devs[0])->getHandleRef();
   // TODO: Throw SYCL2020 style exception
   auto BuildResult = getOrBuild<PiProgramT, compile_program_error>(
       Cache,
-      std::make_pair(std::make_pair(std::move(SpecConsts), (size_t)ImgPtr),
+      std::make_pair(std::make_pair(std::move(SpecConsts), ImgId),
                      std::make_pair(PiDevice, CompileOpts + LinkOpts)),
       AcquireF, GetF, BuildF);
   // getOrBuild is not supposed to return nullptr
@@ -2022,7 +2024,7 @@ device_image_plain ProgramManager::build(const device_image_plain &DeviceImage,
 
     getOrBuild<PiProgramT, compile_program_error>(
         Cache,
-        std::make_pair(std::make_pair(std::move(SpecConsts), (size_t)ImgPtr),
+        std::make_pair(std::make_pair(std::move(SpecConsts), ImgId),
                        std::make_pair(PiDeviceAdd, CompileOpts + LinkOpts)),
         AcquireF, GetF, CacheOtherDevices);
     // getOrBuild is not supposed to return nullptr

sycl/unittests/kernel-and-program/MultipleDevsCache.cpp

@@ -162,13 +162,17 @@ TEST_F(MultipleDeviceCacheTest, ProgramRetain) {
     detail::KernelProgramCache::KernelCacheT &KernelCache =
         CtxImpl->getKernelProgramCache().acquireKernelsPerProgramCache().get();
 
-    EXPECT_EQ(KernelCache.size(), (size_t)2) << "Expect 2 kernels in cache";
+    EXPECT_EQ(KernelCache.size(), (size_t)1)
+        << "Expect 1 program in kernel cache";
+    for (auto &KernelProgIt : KernelCache)
+      EXPECT_EQ(KernelProgIt.second.size(), (size_t)1)
+          << "Expect 1 kernel cache";
   }
-  // First kernel creating is called in handler::single_task().
+  // The kernel creating is called in handler::single_task().
   // kernel_bundle::get_kernel() creates a kernel and shares it with created
   // programs. Also the kernel is retained in kernel_bundle::get_kernel(). A
   // kernel is removed from cache if piKernelRelease was called for it, so it
   // will not be removed twice for the other programs. As a result we must
-  // expect 3 piKernelRelease calls.
-  EXPECT_EQ(KernelReleaseCounter, 3) << "Expect 3 piKernelRelease calls";
+  // expect 2 piKernelRelease calls.
+  EXPECT_EQ(KernelReleaseCounter, 2) << "Expect 2 piKernelRelease calls";
 }