diff --git a/unified-runtime/source/adapters/level_zero/v2/memory.cpp b/unified-runtime/source/adapters/level_zero/v2/memory.cpp index b0601ba956af2..4cd09f9d80c3b 100644 --- a/unified-runtime/source/adapters/level_zero/v2/memory.cpp +++ b/unified-runtime/source/adapters/level_zero/v2/memory.cpp @@ -320,7 +320,8 @@ ur_discrete_buffer_handle_t::ur_discrete_buffer_handle_t( } ur_discrete_buffer_handle_t::~ur_discrete_buffer_handle_t() { - if (!activeAllocationDevice || !writeBackPtr) + if (!activeAllocationDevice || !activeAllocationDevice->Id.has_value() || + !writeBackPtr) return; auto srcPtr = getActiveDeviceAlloc(); diff --git a/unified-runtime/source/adapters/opencl/command_buffer.cpp b/unified-runtime/source/adapters/opencl/command_buffer.cpp index 834a252c14078..6b5d4c205072d 100644 --- a/unified-runtime/source/adapters/opencl/command_buffer.cpp +++ b/unified-runtime/source/adapters/opencl/command_buffer.cpp @@ -27,17 +27,10 @@ ur_exp_command_buffer_handle_t_::~ur_exp_command_buffer_handle_t_() { clReleaseEvent(LastSubmission); } - cl_context CLContext = hContext->CLContext; - cl_ext::clReleaseCommandBufferKHR_fn clReleaseCommandBufferKHR = nullptr; - cl_int Res = - cl_ext::getExtFuncFromContext( - CLContext, - cast(ur::cl::getAdapter())->fnCache.clReleaseCommandBufferKHRCache, - cl_ext::ReleaseCommandBufferName, &clReleaseCommandBufferKHR); - assert(Res == CL_SUCCESS); - (void)Res; - - clReleaseCommandBufferKHR(CLCommandBuffer); + if (CLCommandBuffer) { + [[maybe_unused]] cl_int Res = CLReleaseCommandBufferKHR(CLCommandBuffer); + assert(Res == CL_SUCCESS); + } } ur_result_t @@ -54,6 +47,12 @@ urCommandBufferCreateExp(ur_context_handle_t hContext, CLContext, cast(ur::cl::getAdapter())->fnCache.clCreateCommandBufferKHRCache, cl_ext::CreateCommandBufferName, &clCreateCommandBufferKHR)); + cl_ext::clReleaseCommandBufferKHR_fn clReleaseCommandBufferKHR = nullptr; + UR_RETURN_ON_FAILURE( + cl_ext::getExtFuncFromContext( + CLContext, + cast(ur::cl::getAdapter())->fnCache.clReleaseCommandBufferKHRCache, + cl_ext::ReleaseCommandBufferName, &clReleaseCommandBufferKHR)); const bool IsUpdatable = pCommandBufferDesc->isUpdatable; @@ -102,6 +101,8 @@ urCommandBufferCreateExp(ur_context_handle_t hContext, auto URCommandBuffer = std::make_unique( QueuePtr, cast(hContext), cast(hDevice), CLCommandBuffer, IsUpdatable, IsInOrder); + URCommandBuffer->CLCreateCommandBufferKHR = clCreateCommandBufferKHR; + URCommandBuffer->CLReleaseCommandBufferKHR = clReleaseCommandBufferKHR; *phCommandBuffer = cast(URCommandBuffer.release()); } catch (std::bad_alloc &) { return UR_RESULT_ERROR_OUT_OF_RESOURCES; diff --git a/unified-runtime/source/adapters/opencl/command_buffer.hpp b/unified-runtime/source/adapters/opencl/command_buffer.hpp index 31badc7e899da..080c2fe7da2fb 100644 --- a/unified-runtime/source/adapters/opencl/command_buffer.hpp +++ b/unified-runtime/source/adapters/opencl/command_buffer.hpp @@ -45,6 +45,10 @@ struct ur_exp_command_buffer_handle_t_ : handle_base { ur_device_handle_t_ *hDevice; /// OpenCL command-buffer object. cl_command_buffer_khr CLCommandBuffer; + /// OpenCL function used to create the command-buffer object. + cl_ext::clCreateCommandBufferKHR_fn CLCreateCommandBufferKHR; + /// OpenCL function used to release the command-buffer object. + cl_ext::clReleaseCommandBufferKHR_fn CLReleaseCommandBufferKHR; /// Set to true if the kernel commands in the command-buffer can be updated, /// false otherwise bool IsUpdatable; @@ -67,6 +71,7 @@ struct ur_exp_command_buffer_handle_t_ : handle_base { bool IsUpdatable, bool IsInOrder) : handle_base(), hInternalQueue(hQueue), hContext(hContext), hDevice(hDevice), CLCommandBuffer(CLCommandBuffer), + CLCreateCommandBufferKHR(nullptr), CLReleaseCommandBufferKHR(nullptr), IsUpdatable(IsUpdatable), IsInOrder(IsInOrder), IsFinalized(false), LastSubmission(nullptr) {} diff --git a/unified-runtime/source/adapters/opencl/context.hpp b/unified-runtime/source/adapters/opencl/context.hpp index 0428bc944edb5..278f73045ec7d 100644 --- a/unified-runtime/source/adapters/opencl/context.hpp +++ b/unified-runtime/source/adapters/opencl/context.hpp @@ -45,7 +45,11 @@ struct ur_context_handle_t_ : handle_base { // clear the ext function pointer cache. This isn't foolproof sadly but it // should drastically reduce the chances of the pathological case described // in the comments in common.hpp. - cast(ur::cl::getAdapter())->fnCache.clearCache(CLContext); + try { + cast(ur::cl::getAdapter())->fnCache.clearCache(CLContext); + } catch (...) { + assert(false && "Failed to clear OpenCL extension function cache"); + } for (uint32_t i = 0; i < DeviceCount; i++) { ur::opencl::urDeviceRelease(cast(Devices[i])); diff --git a/unified-runtime/source/adapters/opencl/device.hpp b/unified-runtime/source/adapters/opencl/device.hpp index 1cfd76e8822e9..c96587e21c9cd 100644 --- a/unified-runtime/source/adapters/opencl/device.hpp +++ b/unified-runtime/source/adapters/opencl/device.hpp @@ -43,10 +43,10 @@ struct ur_device_handle_t_ : handle_base { ~ur_device_handle_t_() { if (ParentDevice) { - // This does not need protected by a lock; this destructor can only run - // exactly once. However, to prevent issues with the OpenCL handle being - // reused, CLDevice must still be alive here. - Platform->SubDevices.erase(CLDevice); + { + std::lock_guard lock{Platform->SubDevicesLock}; + Platform->SubDevices.erase(CLDevice); + } [[maybe_unused]] auto Res = clReleaseDevice(CLDevice); assert(Res == CL_SUCCESS); } diff --git a/unified-runtime/source/adapters/opencl/enqueue.cpp b/unified-runtime/source/adapters/opencl/enqueue.cpp index 4c5b34143f15e..b40944c633c12 100644 --- a/unified-runtime/source/adapters/opencl/enqueue.cpp +++ b/unified-runtime/source/adapters/opencl/enqueue.cpp @@ -546,25 +546,8 @@ ur_result_t urEnqueueKernelLaunchWithArgsExp( _launchPropList->pNext); } - // Only look up USM function pointer if we have POINTER args - clSetKernelArgMemPointerINTEL_fn SetKernelArgMemPointerPtr = nullptr; - bool hasPointerArgs = false; - for (uint32_t i = 0; i < numArgs; i++) { - if (pArgs[i].type == UR_EXP_KERNEL_ARG_TYPE_POINTER) { - hasPointerArgs = true; - break; - } - } auto Queue = cast(hQueue); auto Kernel = cast(hKernel); - if (hasPointerArgs) { - UR_RETURN_ON_FAILURE( - cl_ext::getExtFuncFromContext( - Queue->Context->CLContext, - cast(ur::cl::getAdapter()) - ->fnCache.clSetKernelArgMemPointerINTELCache, - cl_ext::SetKernelArgMemPointerName, &SetKernelArgMemPointerPtr)); - } for (uint32_t i = 0; i < numArgs; i++) { switch (pArgs[i].type) { @@ -588,7 +571,10 @@ ur_result_t urEnqueueKernelLaunchWithArgsExp( break; } case UR_EXP_KERNEL_ARG_TYPE_POINTER: - CL_RETURN_ON_FAILURE(SetKernelArgMemPointerPtr( + if (!Kernel->clSetKernelArgMemPointerINTEL) { + return UR_RESULT_ERROR_UNSUPPORTED_FEATURE; + } + CL_RETURN_ON_FAILURE(Kernel->clSetKernelArgMemPointerINTEL( Kernel->CLKernel, static_cast(pArgs[i].index), pArgs[i].value.pointer)); break; diff --git a/unified-runtime/source/adapters/opencl/event.hpp b/unified-runtime/source/adapters/opencl/event.hpp index 831f1104c527b..0b0736166692d 100644 --- a/unified-runtime/source/adapters/opencl/event.hpp +++ b/unified-runtime/source/adapters/opencl/event.hpp @@ -72,8 +72,8 @@ inline ur_result_t createUREvent(cl_event Event, ur_context_handle_t Context, try { auto UREvent = std::make_unique(Event, cast(Context), UrQueue); + UR_RETURN_ON_FAILURE(UrQueue->storeLastEvent(cast(UREvent.get()))); *ReturnedEvent = cast(UREvent.release()); - UR_RETURN_ON_FAILURE(UrQueue->storeLastEvent(*ReturnedEvent)); } catch (std::bad_alloc &) { return UR_RESULT_ERROR_OUT_OF_RESOURCES; } catch (...) { diff --git a/unified-runtime/source/adapters/opencl/platform.cpp b/unified-runtime/source/adapters/opencl/platform.cpp index 443b5dfb8c089..3fe44a55fff32 100644 --- a/unified-runtime/source/adapters/opencl/platform.cpp +++ b/unified-runtime/source/adapters/opencl/platform.cpp @@ -127,12 +127,9 @@ ur_result_t urPlatformGet(ur_adapter_handle_t, uint32_t NumEntries, } auto Adapter = cast(AdapterHandle); - if (Adapter->NumPlatforms == 0) { + { std::lock_guard guard{adapterPopulationMutex}; - // It's possible for urPlatformGet, if ran on multiple threads, to enter - // this branch simultaneously. This check ensures that only one sees that - // Adapter->NumPlatforms is zero. if (Adapter->NumPlatforms == 0) { uint32_t NumPlatforms = 0; cl_int Res = clGetPlatformIDs(0, nullptr, &NumPlatforms); diff --git a/unified-runtime/source/adapters/opencl/usm.cpp b/unified-runtime/source/adapters/opencl/usm.cpp index 540ffa502cc88..46ae6418b5a6f 100644 --- a/unified-runtime/source/adapters/opencl/usm.cpp +++ b/unified-runtime/source/adapters/opencl/usm.cpp @@ -341,14 +341,14 @@ ur_result_t urEnqueueUSMFill(ur_queue_handle_t hQueue, void *ptr, numEventsInWaitList, CLWaitEvents.data(), &CopyEvent)); + std::unique_ptr UREvent; if (phEvent) { // Since we're releasing this in the callback above we need to retain it // here to keep the user copy alive. CL_RETURN_ON_FAILURE(clRetainEvent(CopyEvent)); try { - auto UREvent = std::make_unique( - CopyEvent, Queue->Context, Queue); - *phEvent = cast(UREvent.release()); + UREvent = std::make_unique(CopyEvent, Queue->Context, + Queue); } catch (std::bad_alloc &) { return UR_RESULT_ERROR_OUT_OF_RESOURCES; } catch (...) { @@ -372,6 +372,10 @@ ur_result_t urEnqueueUSMFill(ur_queue_handle_t hQueue, void *ptr, CL_RETURN_ON_FAILURE(ClErr); } + if (phEvent) { + *phEvent = cast(UREvent.release()); + } + return UR_RESULT_SUCCESS; } @@ -506,7 +510,12 @@ ur_result_t urEnqueueUSMMemcpy(ur_queue_handle_t hQueue, bool blocking, } // We are going to release this event in our callback so we need to // retain if the user wants a copy. - CL_RETURN_ON_FAILURE(clRetainEvent(FinalCopyEvent)); + cl_int RetainResult = clRetainEvent(FinalCopyEvent); + if (RetainResult != CL_SUCCESS) { + ur::opencl::urEventRelease(*phEvent); + *phEvent = nullptr; + CL_RETURN_ON_FAILURE(RetainResult); + } } // This self destructs taking the event and allocation with it. @@ -521,6 +530,10 @@ ur_result_t urEnqueueUSMMemcpy(ur_queue_handle_t hQueue, bool blocking, if (CLErr != CL_SUCCESS) { // We can attempt to recover gracefully by attempting to wait for the // copy to finish and deleting the info struct here. + if (phEvent) { + ur::opencl::urEventRelease(*phEvent); + *phEvent = nullptr; + } clWaitForEvents(1, &HostCopyEvent); delete DeleterInfo; clReleaseEvent(HostCopyEvent); diff --git a/unified-runtime/source/loader/layers/sanitizer/sanitizer_common/sanitizer_allocator.cpp b/unified-runtime/source/loader/layers/sanitizer/sanitizer_common/sanitizer_allocator.cpp index 86cba5781ef05..075558dab320d 100644 --- a/unified-runtime/source/loader/layers/sanitizer/sanitizer_common/sanitizer_allocator.cpp +++ b/unified-runtime/source/loader/layers/sanitizer/sanitizer_common/sanitizer_allocator.cpp @@ -45,6 +45,11 @@ ur_result_t SafeAllocate(ur_context_handle_t Context, ur_device_handle_t Device, uptr Size, const ur_usm_desc_t *Properties, ur_usm_pool_handle_t Pool, AllocType Type, void **Allocated) { + if (!Device && + (Type == AllocType::DEVICE_USM || Type == AllocType::MEM_BUFFER || + Type == AllocType::SHARED_USM)) { + return UR_RESULT_ERROR_INVALID_NULL_HANDLE; + } DeviceType DevieType = Device ? GetDeviceType(Context, Device) : DeviceType::UNKNOWN; switch (Type) { diff --git a/unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.cpp b/unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.cpp index cf4a8dfc9ab64..2e46dfee3ee18 100644 --- a/unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.cpp +++ b/unified-runtime/source/loader/layers/sanitizer/tsan/tsan_interceptor.cpp @@ -94,7 +94,17 @@ ur_result_t DeviceInfo::allocShadowMemory() { &ShadowContext)); Shadow = GetShadowMemory(ShadowContext, Handle, Type); assert(Shadow && "Failed to get shadow memory"); - UR_CALL(Shadow->Setup()); + auto Result = Shadow->Setup(); + // Release the shadow context if the operation failed or if the device type is + // CPU. Note that the singleton CPU shadow does not store per-device contexts. + if (Result != UR_RESULT_SUCCESS || Type == DeviceType::CPU) { + [[maybe_unused]] auto ReleaseResult = + getContext()->urDdiTable.Context.pfnRelease(ShadowContext); + assert(ReleaseResult == UR_RESULT_SUCCESS); + } + if (Result != UR_RESULT_SUCCESS) { + return Result; + } UR_LOG_L(getContext()->logger, INFO, "ShadowMemory(Global): {} - {}", (void *)Shadow->ShadowBegin, (void *)Shadow->ShadowEnd); return UR_RESULT_SUCCESS;