Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use new UR handles for opencl instead of casting mechanism #12172

Draft
wants to merge 6 commits into
base: sycl
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions sycl/cmake/modules/FetchUnifiedRuntime.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,15 @@ if(SYCL_UR_USE_FETCH_CONTENT)
CACHE PATH "Path to external '${name}' adapter source dir" FORCE)
endfunction()

set(UNIFIED_RUNTIME_REPO "https://github.com/oneapi-src/unified-runtime.git")
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules/UnifiedRuntimeTag.cmake)
set(UNIFIED_RUNTIME_REPO "https://github.com/omarahmed1111/unified-runtime.git")
# commit df6da35d6e67f2383db28dd49ab08c5c0ef541d2
# Merge: 67590533 55bd5636
# Author: aarongreig <aaron.greig@codeplay.com>
# Date: Mon Oct 7 12:28:07 2024 +0100
# Merge pull request #2038 from GeorgeWeb/georgi/unsupported-max-coop-wgsize
# [UR][hip][opencl] Mark urKernelSuggestMaxCooperativeGroupCountExp as unsupported
# instead of returning misleading default value
set(UNIFIED_RUNTIME_TAG 6806e8b65c6cd878b6194118b88144d4f68ff7e8)

set(UMF_BUILD_EXAMPLES OFF CACHE INTERNAL "EXAMPLES")
# Due to the use of dependentloadflag and no installer for UMF and hwloc we need
Expand Down
2 changes: 1 addition & 1 deletion sycl/cmake/modules/UnifiedRuntimeTag.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
# Date: Wed Nov 6 16:45:49 2024 +0000
# Merge pull request #2276 from rafbiels/rafbiels/fix-hip-evbase
# Set the right HIP device before creating base event counter
set(UNIFIED_RUNTIME_TAG 2858a8a28d0b6524a3b2b0e25a597d1c8295ce9d)
set(UNIFIED_RUNTIME_TAG 6806e8b65c6cd878b6194118b88144d4f68ff7e8)
9 changes: 5 additions & 4 deletions sycl/include/sycl/backend.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ make_context(
const async_handler &Handler = {}) {
return detail::make_context(
detail::ur::cast<ur_native_handle_t>(BackendObject), Handler, Backend,
false /* KeepOwnership */);
true /* KeepOwnership */);
}

template <backend Backend>
Expand All @@ -348,8 +348,9 @@ std::enable_if_t<detail::InteropFeatureSupportMap<Backend>::MakeQueue == true,
make_queue(const typename backend_traits<Backend>::template input_type<queue>
&BackendObject,
const context &TargetContext, const async_handler Handler = {}) {
auto KeepOwnership =
Backend == backend::ext_oneapi_cuda || Backend == backend::ext_oneapi_hip;
auto KeepOwnership = Backend == backend::ext_oneapi_cuda ||
Backend == backend::ext_oneapi_hip ||
Backend == backend::opencl;
return detail::make_queue(detail::ur::cast<ur_native_handle_t>(BackendObject),
false, TargetContext, nullptr, KeepOwnership, {},
Handler, Backend);
Expand Down Expand Up @@ -424,7 +425,7 @@ make_kernel_bundle(const typename backend_traits<Backend>::template input_type<
std::shared_ptr<detail::kernel_bundle_impl> KBImpl =
detail::make_kernel_bundle(
detail::ur::cast<ur_native_handle_t>(BackendObject), TargetContext,
false, State, Backend);
true /* KeepOwnership */, State, Backend);
return detail::createSyclObjFromImpl<kernel_bundle<State>>(KBImpl);
}
} // namespace _V1
Expand Down
16 changes: 4 additions & 12 deletions sycl/source/backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ __SYCL_EXPORT queue make_queue(ur_native_handle_t NativeHandle,

__SYCL_EXPORT event make_event(ur_native_handle_t NativeHandle,
const context &Context, backend Backend) {
return make_event(NativeHandle, Context, false, Backend);
return make_event(NativeHandle, Context, true /* KeepOwnership */, Backend);
}

__SYCL_EXPORT event make_event(ur_native_handle_t NativeHandle,
Expand All @@ -179,9 +179,6 @@ __SYCL_EXPORT event make_event(ur_native_handle_t NativeHandle,
NativeHandle, ContextImpl->getHandleRef(), &Properties, &UrEvent);
event Event = detail::createSyclObjFromImpl<event>(
std::make_shared<event_impl>(UrEvent, Context));

if (Backend == backend::opencl)
Adapter->call<UrApiKind::urEventRetain>(UrEvent);
return Event;
}

Expand All @@ -204,9 +201,6 @@ make_kernel_bundle(ur_native_handle_t NativeHandle,
sycl::make_error_code(sycl::errc::invalid),
"urProgramCreateWithNativeHandle resulted in a null program handle.");

if (ContextImpl->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urProgramRetain>(UrProgram);

std::vector<ur_device_handle_t> ProgramDevices;
uint32_t NumDevices = 0;

Expand Down Expand Up @@ -310,7 +304,8 @@ std::shared_ptr<detail::kernel_bundle_impl>
make_kernel_bundle(ur_native_handle_t NativeHandle,
const context &TargetContext, bundle_state State,
backend Backend) {
return make_kernel_bundle(NativeHandle, TargetContext, false, State, Backend);
return make_kernel_bundle(NativeHandle, TargetContext,
true /* KeepOwnership*/, State, Backend);
}

kernel make_kernel(const context &TargetContext,
Expand Down Expand Up @@ -351,9 +346,6 @@ kernel make_kernel(const context &TargetContext,
NativeHandle, ContextImpl->getHandleRef(), UrProgram, &Properties,
&UrKernel);

if (Backend == backend::opencl)
Adapter->call<UrApiKind::urKernelRetain>(UrKernel);

// Construct the SYCL queue from UR queue.
return detail::createSyclObjFromImpl<kernel>(
std::make_shared<kernel_impl>(UrKernel, ContextImpl, KernelBundleImpl));
Expand All @@ -364,7 +356,7 @@ kernel make_kernel(ur_native_handle_t NativeHandle,
return make_kernel(
TargetContext,
get_empty_interop_kernel_bundle<bundle_state::executable>(TargetContext),
NativeHandle, false, Backend);
NativeHandle, true /* KeepOwnership */, Backend);
}

} // namespace detail
Expand Down
17 changes: 5 additions & 12 deletions sycl/source/detail/buffer_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,12 @@ void buffer_impl::destructorNotification(void *UserObj) {
void buffer_impl::addInteropObject(
std::vector<ur_native_handle_t> &Handles) const {
if (MOpenCLInterop) {
if (std::find(Handles.begin(), Handles.end(),
ur::cast<ur_native_handle_t>(MInteropMemObject)) ==
const AdapterPtr &Adapter = getAdapter();
ur_native_handle_t NativeHandle = 0;
Adapter->call<UrApiKind::urMemGetNativeHandle>(MInteropMemObject, nullptr,
&NativeHandle);
if (std::find(Handles.begin(), Handles.end(), NativeHandle) ==
Handles.end()) {
const AdapterPtr &Adapter = getAdapter();
Adapter->call<UrApiKind::urMemRetain>(
ur::cast<ur_mem_handle_t>(MInteropMemObject));
ur_native_handle_t NativeHandle = 0;
Adapter->call<UrApiKind::urMemGetNativeHandle>(MInteropMemObject, nullptr,
&NativeHandle);
Handles.push_back(NativeHandle);
}
}
Expand Down Expand Up @@ -86,10 +83,6 @@ buffer_impl::getNativeVector(backend BackendName) const {

auto Adapter = Platform->getAdapter();

if (Platform->getBackend() == backend::opencl) {
Adapter->call<UrApiKind::urMemRetain>(NativeMem);
}

ur_native_handle_t Handle = 0;
// When doing buffer interop we don't know what device the memory should be
// resident on, so pass nullptr for Device param. Buffer interop may not be
Expand Down
6 changes: 0 additions & 6 deletions sycl/source/detail/context_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,11 @@ context_impl::context_impl(ur_context_handle_t UrContext,
//
// TODO: Move this backend-specific retain of the context to SYCL-2020 style
// make_context<backend::opencl> interop, when that is created.
if (getBackend() == sycl::backend::opencl) {
getAdapter()->call<UrApiKind::urContextRetain>(MContext);
}
MKernelProgramCache.setContextPtr(this);
}

cl_context context_impl::get() const {
// TODO catch an exception and put it to list of asynchronous exceptions
getAdapter()->call<UrApiKind::urContextRetain>(MContext);
ur_native_handle_t nativeHandle = 0;
getAdapter()->call<UrApiKind::urContextGetNativeHandle>(MContext,
&nativeHandle);
Expand Down Expand Up @@ -297,8 +293,6 @@ context_impl::findMatchingDeviceImpl(ur_device_handle_t &DeviceUR) const {

ur_native_handle_t context_impl::getNative() const {
const auto &Adapter = getAdapter();
if (getBackend() == backend::opencl)
Adapter->call<UrApiKind::urContextRetain>(getHandleRef());
ur_native_handle_t Handle;
Adapter->call<UrApiKind::urContextGetNativeHandle>(getHandleRef(), &Handle);
return Handle;
Expand Down
3 changes: 0 additions & 3 deletions sycl/source/detail/device_image_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,6 @@ class device_image_impl {
assert(MProgram);
const auto &ContextImplPtr = detail::getSyclObjImpl(MContext);
const AdapterPtr &Adapter = ContextImplPtr->getAdapter();

if (ContextImplPtr->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urProgramRetain>(MProgram);
ur_native_handle_t NativeProgram = 0;
Adapter->call<UrApiKind::urProgramGetNativeHandle>(MProgram,
&NativeProgram);
Expand Down
3 changes: 0 additions & 3 deletions sycl/source/detail/device_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ bool device_impl::is_affinity_supported(

cl_device_id device_impl::get() const {
// TODO catch an exception and put it to list of asynchronous exceptions
getAdapter()->call<UrApiKind::urDeviceRetain>(MDevice);
return ur::cast<cl_device_id>(getNative());
}

Expand Down Expand Up @@ -339,8 +338,6 @@ std::vector<device> device_impl::create_sub_devices() const {

ur_native_handle_t device_impl::getNative() const {
auto Adapter = getAdapter();
if (getBackend() == backend::opencl)
Adapter->call<UrApiKind::urDeviceRetain>(getHandleRef());
ur_native_handle_t Handle;
Adapter->call<UrApiKind::urDeviceGetNativeHandle>(getHandleRef(), &Handle);
return Handle;
Expand Down
2 changes: 0 additions & 2 deletions sycl/source/detail/event_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,6 @@ ur_native_handle_t event_impl::getNative() {
this->setHandle(UREvent);
Handle = UREvent;
}
if (MContext->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urEventRetain>(Handle);
ur_native_handle_t OutHandle;
Adapter->call<UrApiKind::urEventGetNativeHandle>(Handle, &OutHandle);
return OutHandle;
Expand Down
8 changes: 0 additions & 8 deletions sycl/source/detail/kernel_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,6 @@ kernel_impl::kernel_impl(ur_kernel_handle_t Kernel, ContextImplPtr Context,
Context)),
MCreatedFromSource(true), MKernelBundleImpl(std::move(KernelBundleImpl)),
MIsInterop(true), MKernelArgMaskPtr{ArgMask} {
ur_context_handle_t UrContext = nullptr;
// Using the adapter from the passed ContextImpl
getAdapter()->call<UrApiKind::urKernelGetInfo>(
MKernel, UR_KERNEL_INFO_CONTEXT, sizeof(UrContext), &UrContext, nullptr);
if (Context->getHandleRef() != UrContext)
throw sycl::exception(
make_error_code(errc::invalid),
"Input context must be the same as the context of cl_kernel");

// Enable USM indirect access for interoperability kernels.
// Some UR Adapters (like OpenCL) require this call to enable USM
Expand Down
4 changes: 0 additions & 4 deletions sycl/source/detail/kernel_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ class kernel_impl {
///
/// \return a valid cl_kernel instance
cl_kernel get() const {
getAdapter()->call<UrApiKind::urKernelRetain>(MKernel);
ur_native_handle_t nativeHandle = 0;
getAdapter()->call<UrApiKind::urKernelGetNativeHandle>(MKernel,
&nativeHandle);
Expand Down Expand Up @@ -152,9 +151,6 @@ class kernel_impl {
ur_native_handle_t getNative() const {
const AdapterPtr &Adapter = MContext->getAdapter();

if (MContext->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urKernelRetain>(MKernel);

ur_native_handle_t NativeKernel = 0;
Adapter->call<UrApiKind::urKernelGetNativeHandle>(MKernel, &NativeKernel);

Expand Down
2 changes: 0 additions & 2 deletions sycl/source/detail/queue_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,6 @@ void queue_impl::destructorNotification() {

ur_native_handle_t queue_impl::getNative(int32_t &NativeHandleDesc) const {
const AdapterPtr &Adapter = getAdapter();
if (getContextImplPtr()->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urQueueRetain>(MQueues[0]);
ur_native_handle_t Handle{};
ur_queue_native_desc_t UrNativeDesc{UR_STRUCTURE_TYPE_QUEUE_NATIVE_DESC,
nullptr, nullptr};
Expand Down
1 change: 0 additions & 1 deletion sycl/source/detail/queue_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ class queue_impl {
/// \return an OpenCL interoperability queue handle.

cl_command_queue get() {
getAdapter()->call<UrApiKind::urQueueRetain>(MQueues[0]);
ur_native_handle_t nativeHandle = 0;
getAdapter()->call<UrApiKind::urQueueGetNativeHandle>(MQueues[0], nullptr,
&nativeHandle);
Expand Down
6 changes: 0 additions & 6 deletions sycl/source/detail/sycl_mem_obj_t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ SYCLMemObjT::SYCLMemObjT(ur_native_handle_t MemObject,
throw sycl::exception(
make_error_code(errc::invalid),
"Input context must be the same as the context of cl_mem");

if (MInteropContext->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urMemRetain>(MInteropMemObject);
}

ur_mem_type_t getImageType(int Dimensions) {
Expand Down Expand Up @@ -111,9 +108,6 @@ SYCLMemObjT::SYCLMemObjT(ur_native_handle_t MemObject,
throw sycl::exception(
make_error_code(errc::invalid),
"Input context must be the same as the context of cl_mem");

if (MInteropContext->getBackend() == backend::opencl)
Adapter->call<UrApiKind::urMemRetain>(MInteropMemObject);
}

void SYCLMemObjT::releaseMem(ContextImplPtr Context, void *MemAllocation) {
Expand Down
1 change: 0 additions & 1 deletion sycl/source/device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ device::device(cl_device_id DeviceId) {
auto Platform =
detail::platform_impl::getPlatformFromUrDevice(Device, Adapter);
impl = Platform->getOrMakeDeviceImpl(Device, Platform);
Adapter->call<detail::UrApiKind::urDeviceRetain>(impl->getHandleRef());
}

device::device(const device_selector &deviceSelector) {
Expand Down
15 changes: 7 additions & 8 deletions sycl/source/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,13 @@ inline namespace _V1 {

event::event() : impl(std::make_shared<detail::event_impl>(std::nullopt)) {}

event::event(cl_event ClEvent, const context &SyclContext)
: impl(std::make_shared<detail::event_impl>(
detail::ur::cast<ur_event_handle_t>(ClEvent), SyclContext)) {
// This is a special interop constructor for OpenCL, so the event must be
// retained.
// TODO(pi2ur): Don't just cast from cl_event above
impl->getAdapter()->call<detail::UrApiKind::urEventRetain>(
detail::ur::cast<ur_event_handle_t>(ClEvent));
event::event(cl_event ClEvent, const context &SyclContext) {
ur_event_handle_t hEvent = nullptr;
impl->getAdapter()->call<detail::UrApiKind::urEventCreateWithNativeHandle>(
detail::ur::cast<ur_native_handle_t>(ClEvent),
detail::getSyclObjImpl(SyclContext)->getHandleRef(), nullptr, &hEvent);

impl = std::make_shared<detail::event_impl>(hEvent, SyclContext);
}

bool event::operator==(const event &rhs) const { return rhs.impl == impl; }
Expand Down
17 changes: 9 additions & 8 deletions sycl/source/kernel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,17 @@ kernel::kernel(cl_kernel ClKernel, const context &SyclContext) {
ur_kernel_handle_t hKernel = nullptr;
ur_native_handle_t nativeHandle =
reinterpret_cast<ur_native_handle_t>(ClKernel);
Adapter->call<detail::UrApiKind::urKernelCreateWithNativeHandle>(
nativeHandle, detail::getSyclObjImpl(SyclContext)->getHandleRef(),
nullptr, nullptr, &hKernel);
ur_result_t Res =
Adapter->call_nocheck<detail::UrApiKind::urKernelCreateWithNativeHandle>(
nativeHandle, detail::getSyclObjImpl(SyclContext)->getHandleRef(),
nullptr, nullptr, &hKernel);
if (Res == UR_RESULT_ERROR_INVALID_CONTEXT) {
throw sycl::exception(
make_error_code(errc::invalid),
"Input context must be the same as the context of cl_kernel");
}
impl = std::make_shared<detail::kernel_impl>(
hKernel, detail::getSyclObjImpl(SyclContext), nullptr, nullptr);
// This is a special interop constructor for OpenCL, so the kernel must be
// retained.
if (get_backend() == backend::opencl) {
impl->getAdapter()->call<detail::UrApiKind::urKernelRetain>(hKernel);
}
}

cl_kernel kernel::get() const { return impl->get(); }
Expand Down
16 changes: 12 additions & 4 deletions sycl/source/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,18 @@ queue::queue(const context &SyclContext, const device &SyclDevice,
queue::queue(cl_command_queue clQueue, const context &SyclContext,
const async_handler &AsyncHandler) {
const property_list PropList{};
impl = std::make_shared<detail::queue_impl>(
// TODO(pi2ur): Don't cast straight from cl_command_queue
reinterpret_cast<ur_queue_handle_t>(clQueue),
detail::getSyclObjImpl(SyclContext), AsyncHandler, PropList);
ur_queue_handle_t hQueue;
auto Context = detail::getSyclObjImpl(SyclContext);
auto Adapter = sycl::detail::ur::getAdapter<backend::opencl>();

ur_queue_native_properties_t Properties[] = {
UR_STRUCTURE_TYPE_QUEUE_PROPERTIES, nullptr, 0};
Adapter->call<detail::UrApiKind::urQueueCreateWithNativeHandle>(
detail::ur::cast<ur_native_handle_t>(clQueue), Context->getHandleRef(),
nullptr, Properties, &hQueue);

impl = std::make_shared<detail::queue_impl>(hQueue, Context, AsyncHandler,
PropList);
}

cl_command_queue queue::get() const { return impl->get(); }
Expand Down
Loading
Loading