[SYCL] Keep the old format of files in persistent cache (#16428)

[PR#16056](#16056) fixed multi-device
support for persistent cache but also changed format of the files in
persistent cache - we used to write number of binaries before binary
size and binary data but stopped doing that because we always have a
single binary (for a single device) per file. Because of that new
runtime stopped working with persistent cache created by older runtimes.

This PR restores the old format of files in persistent cache.

For that we need to write number of binaries to the file (before binary
size and binary data) though it is always equal to 1 in current
implementation. Even in the old implementation we could only put a
single binary to the persistent cache in all scenarios, multi-device
case wasn't supported, didn't have an API to create a program from
multiple binaries. So we can assert that number of binaries is always 1
when reading from the cache.

Author: againull

sycl/source/detail/persistent_device_code_cache.cpp

@@ -367,11 +367,18 @@ std::string PersistentDeviceCodeCache::getDeviceIDString(const device &Device) {
 }
 
 /* Write built binary to persistent cache
- * Format: BinarySize, Binary
+ * Format: NumBinaries(=1), BinarySize, Binary
  */
 void PersistentDeviceCodeCache::writeBinaryDataToFile(
     const std::string &FileName, const std::vector<char> &Data) {
   std::ofstream FileStream{FileName, std::ios::binary};
+  // The reason why we need to write number of binaries (in current
+  // implementation always 1) is to keep compatibility with the old format of
+  // files in persistent cache, so that new runtime can use binaries from
+  // persistent cache generated by old compiler/runtime.
+  size_t NumBinaries = 1;
+  FileStream.write((char *)&NumBinaries, sizeof(NumBinaries));
+
   auto Size = Data.size();
   FileStream.write((char *)&Size, sizeof(Size));
   FileStream.write(Data.data(), Size);
@@ -380,11 +387,26 @@ void PersistentDeviceCodeCache::writeBinaryDataToFile(
 }
 
 /* Read built binary from persistent cache. Each persistent cache file contains
- * binary for a single device. Format: BinarySize, Binary
+ * binary for a single device.
+ * Format: NumBinaries(=1), BinarySize, Binary
  */
 std::vector<char>
 PersistentDeviceCodeCache::readBinaryDataFromFile(const std::string &FileName) {
   std::ifstream FileStream{FileName, std::ios::binary};
+  // We ignore this number, we always read single device binary from a file and
+  // we need this just to keep compatibility with the old format of files in
+  // persistent cache, so that new runtime can use binaries from persistent
+  // cache generated by old compiler/runtime.
+  size_t NumBinaries = 0;
+  FileStream.read((char *)&NumBinaries, sizeof(NumBinaries));
+  if (FileStream.fail()) {
+    trace("Failed to read number of binaries from " + FileName);
+    return {};
+  }
+  // Even in the old implementation we could only put a single binary to the
+  // persistent cache in all scenarios, multi-device case wasn't supported.
+  assert(NumBinaries == 1);
+
   size_t BinarySize = 0;
   FileStream.read((char *)&BinarySize, sizeof(BinarySize));
 

sycl/source/detail/persistent_device_code_cache.hpp

@@ -91,13 +91,19 @@ class PersistentDeviceCodeCache {
    */
 private:
   /* Write built binary to persistent cache
-   * Format: BinarySize, Binary
+   * Format: NumBinaries(=1), BinarySize, Binary
+   * The reason why we need to write a number of binaries (always 1 in current
+   * implementation) is to keep compatibility with the old format of files in
+   * the persistent cache, so that new runtime can use binaries from the
+   * persistent cache generated by an old compiler/runtime. NumBinaries can be
+   * removed at next ABI breaking window.
    */
   static void writeBinaryDataToFile(const std::string &FileName,
                                     const std::vector<char> &Data);
 
-  /* Read built binary to persistent cache
-   * Format: BinarySize, Binary
+  /* Read built binary from persistent cache
+   * Format: NumBinaries(=1), BinarySize, Binary
+   * See comment above regarding the reason why we need NumBinaries.
    */
   static std::vector<char> readBinaryDataFromFile(const std::string &FileName);
 

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

@@ -394,14 +394,14 @@ TEST_P(PersistentDeviceCodeCache, CorruptedCacheFiles) {
   // Binary file is corrupted
   detail::PersistentDeviceCodeCache::putItemToDisc({Dev}, {&Img}, {},
                                                    BuildOptions, NativeProg);
-  std::ofstream FileStream(ItemDir + "/0.bin",
-                           std::ofstream::out | std::ofstream::trunc);
-  /* Emulate binary built for 2 devices: first is OK, second is trancated
-   * from 23 bytes to 4
-   */
-  FileStream << 2 << 12 << "123456789012" << 23 << "1234";
-  FileStream.close();
-  EXPECT_FALSE(FileStream.fail()) << "Failed to create trancated binary file";
+  {
+    std::ofstream FileStream(ItemDir + "/0.bin",
+                             std::ofstream::out | std::ofstream::trunc);
+    // Emulate binary which is truncated from 23 bytes to 4.
+    FileStream << 1 << 23 << "1234";
+    FileStream.close();
+    EXPECT_FALSE(FileStream.fail()) << "Failed to create trancated binary file";
+  }
   Res = detail::PersistentDeviceCodeCache::getItemFromDisc({Dev}, {&Img}, {},
                                                            BuildOptions);
   EXPECT_EQ(Res.size(), static_cast<size_t>(0))
@@ -421,6 +421,23 @@ TEST_P(PersistentDeviceCodeCache, CorruptedCacheFiles) {
   EXPECT_EQ(Res.size(), static_cast<size_t>(0))
       << "Item with corrupted binary file was read";
   ASSERT_NO_ERROR(llvm::sys::fs::remove_directories(ItemDir));
+
+  // Unexpected 2 binaries in a single file.
+  detail::PersistentDeviceCodeCache::putItemToDisc({Dev}, {&Img}, {},
+                                                   BuildOptions, NativeProg);
+  {
+    std::ofstream FileStream(ItemDir + "/0.bin",
+                             std::ofstream::out | std::ofstream::trunc);
+    // Emulate binaries for 2 devices in a single file.
+    FileStream << 2 << 12 << "123456789012" << 4 << "1234";
+    FileStream.close();
+    EXPECT_FALSE(FileStream.fail())
+        << "Failed to create a file containing 2 binaries";
+  }
+  ASSERT_DEATH(detail::PersistentDeviceCodeCache::getItemFromDisc(
+                   {Dev}, {&Img}, {}, BuildOptions),
+               "NumBinaries == 1");
+  ASSERT_NO_ERROR(llvm::sys::fs::remove_directories(ItemDir));
 }
 
 /* Checks that lock file affects cache operations as expected: