[llvm] [libsycl] Generic code cleanup (PR #224330)

[Alexey Bader via llvm-commits]

================
@@ -91,33 +99,34 @@ void ProgramAndKernelManager::registerFatBin(const void *BinaryStart,
   DeviceImageManagerVec Images;
   Images.reserve(BinOrErr->size());
 
-  std::lock_guard<std::mutex> Guard(MDataCollectionMutex);
   for (std::unique_ptr<llvm::object::OffloadBinary> &OB : *BinOrErr) {
     if (!checkDeviceImageValidity(*OB))
       throw sycl::exception(sycl::make_error_code(sycl::errc::runtime),
                             "Incompatible device image.");
 
-    llvm::StringRef Symbols = OB->getString("symbols");
-
     Images.push_back(std::make_unique<DeviceImageManager>(std::move(OB)));
-    DeviceImageManager &NewImageWrapper = *Images.back();
-
-    llvm::offloading::sycl::forEachSymbol(Symbols, [&](llvm::StringRef Name) {
-      auto It = MDeviceKernelInfoMap.find(std::string_view(Name));
-      if (It == MDeviceKernelInfoMap.end()) {
-        [[maybe_unused]] auto [Iterator, EmplaceSucceeded] =
-            MDeviceKernelInfoMap.emplace(
-                std::piecewise_construct,
-                std::forward_as_tuple(std::string_view(Name)),
-                std::forward_as_tuple(std::string_view(Name), NewImageWrapper));
-        assert(EmplaceSucceeded && "Kernel name found in multiple images");
-      }
-    });
   }
 
-  [[maybe_unused]] auto [It, Inserted] =
+  std::lock_guard<std::mutex> Guard(MDataCollectionMutex);
+  // The kernel info entries below hold references to the image managers, so the
+  // images have to be installed first: nothing may be recorded in
+  // MDeviceKernelInfoMap until their owner is in place and unregisterFatBin()
+  // can reach it.
+  auto [ImagesIt, Inserted] =
       MDeviceImageManagers.emplace(BinaryStart, std::move(Images));
   assert(Inserted && "Fat binary registered twice");
+  if (!Inserted)
+    return;
----------------
bader wrote:

I think it must be either `assert` or if-statement, but not both.
If double registration is not possible and signals about the broken SYCL runtime state - use assert.
If runtime is expected to handle double-registration - use if-statement.

For debugging purposes, it's better to use something different than assert.

https://github.com/llvm/llvm-project/pull/224330