comments addressed

Author: mateuszpn

sycl/test-e2e/bindless_images/lit.local.cfg

@@ -1,5 +1,3 @@
 # Support of this feature is waiting for the upstream PR
 # https://github.com/llvm/llvm-project/pull/127737 to be merged with intel/llvm.
 config.unsupported_features += ['spirv-backend']
-# V2 adapter does not support bindless images yet
-config.unsupported_features += ['level_zero_v2_adapter']

unified-runtime/source/adapters/level_zero/image_common.cpp

@@ -16,6 +16,7 @@
 #else
 #include "context.hpp"
 #endif
+#include "helpers/memory_helpers.hpp"
 #include "image_common.hpp"
 #include "logger/ur_logger.hpp"
 #include "sampler.hpp"
@@ -267,9 +268,14 @@ ur_result_t ze2urImageFormat(const ze_image_format_t &ZeImageFormat,
 }
 
 /// Construct UR bindless image struct from ZE image handle and desc.
-ur_result_t createUrImgFromZeImage(ze_image_handle_t ZeImage,
+ur_result_t createUrImgFromZeImage(ze_context_handle_t hContext,
+                                   ze_device_handle_t hDevice,
                                    const ZeStruct<ze_image_desc_t> &ZeImageDesc,
                                    ur_exp_image_mem_native_handle_t *pImg) {
+  ze_image_handle_t ZeImage;
+  ZE2UR_CALL(zeImageCreate, (hContext, hDevice, &ZeImageDesc, &ZeImage));
+  ZE2UR_CALL(zeContextMakeImageResident, (hContext, hDevice, ZeImage));
+
   try {
     ur_bindless_mem_handle_t *urImg =
         new ur_bindless_mem_handle_t(ZeImage, ZeImageDesc);
@@ -951,12 +957,8 @@ ur_result_t urBindlessImagesImageAllocateExp(
   ZeImageBindlessDesc.flags = ZE_IMAGE_BINDLESS_EXP_FLAG_BINDLESS;
   ZeImageDesc.pNext = &ZeImageBindlessDesc;
 
-  ze_image_handle_t ZeImage;
-  ZE2UR_CALL(zeImageCreate, (hContext->getZeHandle(), hDevice->ZeDevice,
-                             &ZeImageDesc, &ZeImage));
-  ZE2UR_CALL(zeContextMakeImageResident,
-             (hContext->getZeHandle(), hDevice->ZeDevice, ZeImage));
-  UR_CALL(createUrImgFromZeImage(ZeImage, ZeImageDesc, phImageMem));
+  UR_CALL(createUrImgFromZeImage(hContext->getZeHandle(), hDevice->ZeDevice,
+                                 ZeImageDesc, phImageMem));
   return UR_RESULT_SUCCESS;
 }
 
@@ -990,9 +992,9 @@ ur_result_t urBindlessImagesUnsampledImageHandleDestroyExp(
   auto item = hDevice->ZeOffsetToImageHandleMap.find(hImage);
 
   if (item != hDevice->ZeOffsetToImageHandleMap.end()) {
-    ZE2UR_CALL(zeImageDestroy, (item->second));
     hDevice->ZeOffsetToImageHandleMap.erase(item);
     Lock.release();
+    ZE2UR_CALL(zeImageDestroy, (item->second));
   } else {
     Lock.release();
     return UR_RESULT_ERROR_INVALID_NULL_HANDLE;
@@ -1165,12 +1167,9 @@ ur_result_t urBindlessImagesMapExternalArrayExp(
   ZeImageBindlessDesc.flags = ZE_IMAGE_BINDLESS_EXP_FLAG_BINDLESS;
   ZeImageDesc.pNext = &ZeImageBindlessDesc;
 
-  ze_image_handle_t ZeImage;
-  ZE2UR_CALL(zeImageCreate, (hContext->getZeHandle(), hDevice->ZeDevice,
-                             &ZeImageDesc, &ZeImage));
-  ZE2UR_CALL(zeContextMakeImageResident,
-             (hContext->getZeHandle(), hDevice->ZeDevice, ZeImage));
-  UR_CALL(createUrImgFromZeImage(ZeImage, ZeImageDesc, phImageMem));
+  UR_CALL(createUrImgFromZeImage(hContext->getZeHandle(), hDevice->ZeDevice,
+                                 ZeImageDesc, phImageMem));
+
   externalMemoryData->urMemoryHandle =
       reinterpret_cast<ur_mem_handle_t>(*phImageMem);
   return UR_RESULT_SUCCESS;
@@ -1186,7 +1185,29 @@ ur_result_t urBindlessImagesReleaseExternalMemoryExp(
   struct ur_ze_external_memory_data *externalMemoryData =
       reinterpret_cast<ur_ze_external_memory_data *>(hExternalMem);
 
-  UR_CALL(ur::level_zero::urMemRelease(externalMemoryData->urMemoryHandle));
+  // externalMemoryData->urMemoryHandle can be ur_bindless_mem_handle_t or
+  // buffer allocated by zeMemAllocDevice()
+
+  ze_memory_allocation_properties_t allocProperties{
+      ZE_STRUCTURE_TYPE_MEMORY_ALLOCATION_PROPERTIES, nullptr,
+      ZE_MEMORY_TYPE_UNKNOWN, 0, 0};
+  ZE2UR_CALL(zeMemGetAllocProperties,
+             (hContext->getZeHandle(), externalMemoryData->urMemoryHandle,
+              &allocProperties, nullptr));
+
+  switch (allocProperties.type) {
+  case ZE_MEMORY_TYPE_DEVICE:
+    zeMemFree(hContext->getZeHandle(),
+              externalMemoryData->urMemoryHandle); // Free device memory
+    break;
+  case ZE_MEMORY_TYPE_UNKNOWN:
+  default:
+    auto *bindlessMem = reinterpret_cast<ur_bindless_mem_handle_t *>(
+        externalMemoryData->urMemoryHandle);
+    zeImageDestroy(bindlessMem->getZeImage());
+    delete bindlessMem;
+    break;
+  }
 
   switch (externalMemoryData->type) {
   case UR_ZE_EXTERNAL_OPAQUE_FD:

unified-runtime/source/adapters/level_zero/image_common.hpp

@@ -17,10 +17,11 @@
 #include "v2/common.hpp"
 
 struct ur_bindless_mem_handle_t {
-
+  // Constructor for bindless image handle
   ur_bindless_mem_handle_t(ze_image_handle_t zeImage,
                            const ZeStruct<ze_image_desc_t> &zeImageDesc)
       : zeImage(zeImage) {
+
     format = zeImageDesc.format;
     width = zeImageDesc.width;
     height = zeImageDesc.height;

unified-runtime/source/adapters/level_zero/memory.cpp

@@ -2357,6 +2357,8 @@ ur_result_t ur_mem_handle_t_::getZeHandle(char *&ZeHandle, access_mode_t mode,
   case ur_mem_handle_t_::buffer:
     return reinterpret_cast<_ur_buffer *>(this)->getBufferZeHandle(
         ZeHandle, mode, Device, phWaitEvents, numWaitEvents);
+  default:
+    die("ur_mem_handle_t_::getZeHandle: Unhandled memory type");
   }
   ur::unreachable();
 }
@@ -2371,6 +2373,8 @@ ur_result_t ur_mem_handle_t_::getZeHandlePtr(
   case ur_mem_handle_t_::buffer:
     return reinterpret_cast<_ur_buffer *>(this)->getBufferZeHandlePtr(
         ZeHandlePtr, mode, Device, phWaitEvents, numWaitEvents);
+  default:
+    die("ur_mem_handle_t_::getZeHandle: Unhandled memory type");
   }
   ur::unreachable();
 }

unified-runtime/source/adapters/level_zero/v2/image.cpp

@@ -30,18 +30,4 @@ urBindlessImagesImageFreeExp([[maybe_unused]] ur_context_handle_t hContext,
   return UR_RESULT_SUCCESS;
 }
 
-// ur_result_t urBindlessImagesMapExternalLinearMemoryExp(
-//     [[maybe_unused]] ur_context_handle_t hContext,
-//     [[maybe_unused]] ur_device_handle_t hDevice,
-//     [[maybe_unused]] uint64_t offset, [[maybe_unused]] uint64_t size,
-//     [[maybe_unused]] ur_exp_external_mem_handle_t hExternalMem,
-//     [[maybe_unused]] void **phRetMem) {
-//   // implemented in V1 in PR https://github.com/intel/llvm/pull/17458
-//   // TODO: implement in V2 after V1 is merged
-//   logger::error(
-//       logger::LegacyMessage("[UR][L0_v2] {} function not implemented!"),
-//       "{} function not implemented!", __FUNCTION__);
-//   return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
-// }
-
 } // namespace ur::level_zero

unified-runtime/source/adapters/level_zero/v2/memory.hpp

@@ -15,7 +15,9 @@
 #include <ur_api.h>
 
 #include "../device.hpp"
+#include "../image_common.hpp"
 #include "common.hpp"
+#include "helpers/memory_helpers.hpp"
 
 using usm_unique_ptr_t = std::unique_ptr<void, std::function<void(void *)>>;
 
@@ -214,7 +216,7 @@ struct ur_mem_image_t : _ur_object {
   ZeStruct<ze_image_desc_t> zeImageDesc;
 };
 
-struct ur_mem_handle_t_ {
+struct ur_mem_handle_t_ : _ur_object {
   template <typename T, typename... Args>
   static ur_mem_handle_t_ *create(Args &&...args) {
     return new ur_mem_handle_t_(std::in_place_type<T>,