From b6faeec0e3d2b64734476a1b72751834b59e03f6 Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Fri, 21 Oct 2022 12:54:26 -0700 Subject: [PATCH 1/3] [SYCL] Native event for default-ctored sycl::event has to be in COMPLETE state Per SYCL 2020 for event(): > Constructs an event that is immediately ready. The event has no > dependencies and no associated commands. Waiting on this event will > return immediately and querying its status will return > info::event_command_status::complete. piEventCreate on the other hand creates an event that isn't completed. As such an extra call to piEventSetStatus is needed making that API not-deprecated. There is a more general problem that isn't addressed here: auto e = q.submit(... h.host_task(...) ..) This event would be a host one and we assert that no get_native could be called on it (see existing sycl::detail::getImplBackend). If we will ever want to support such scenario we'd need to implement some tracking of host/backed events in the SYCL RT and keep updating the latter whenever the host one changes the state. --- sycl/plugins/level_zero/pi_level_zero.cpp | 9 ++++++--- sycl/source/detail/backend_impl.hpp | 2 ++ sycl/source/detail/event_impl.cpp | 6 ++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 98a90d9292534..271f10a0c876b 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -5964,9 +5964,12 @@ pi_result piEventSetCallback(pi_event Event, pi_int32 CommandExecCallbackType, } pi_result piEventSetStatus(pi_event Event, pi_int32 ExecutionStatus) { - (void)Event; - (void)ExecutionStatus; - die("piEventSetStatus: deprecated, to be removed"); + if (ExecutionStatus == PI_EVENT_COMPLETE) + zeEventHostSignal(Event->ZeEvent); + else + // We don't expect this path ever to be executed when called from SYCL RT. + die("piEventSetStatus: with anything but PI_EVENT_COMPLETE is " + "unsupported!"); return PI_SUCCESS; } diff --git a/sycl/source/detail/backend_impl.hpp b/sycl/source/detail/backend_impl.hpp index fb3ab07737dda..b5969589c3af7 100644 --- a/sycl/source/detail/backend_impl.hpp +++ b/sycl/source/detail/backend_impl.hpp @@ -15,6 +15,8 @@ __SYCL_INLINE_VER_NAMESPACE(_V1) { namespace detail { template backend getImplBackend(const T &Impl) { + // If that would ever become possible, event_impl::getNative needs to be + // updated too. assert(!Impl->is_host() && "Cannot get the backend for host."); return Impl->getPlugin().getBackend(); } diff --git a/sycl/source/detail/event_impl.cpp b/sycl/source/detail/event_impl.cpp index 7d88e52baf27d..68f4a1971109c 100644 --- a/sycl/source/detail/event_impl.cpp +++ b/sycl/source/detail/event_impl.cpp @@ -375,6 +375,12 @@ pi_native_handle event_impl::getNative() { MIsInitialized = true; auto TempContext = MContext.get()->getHandleRef(); Plugin.call(TempContext, &MEvent); + // See an assert in sycl::detail::getImplBackend. + assert(!MHostEvent && "Can't get native event from a host event!"); + assert(!isDiscarded() && "Can't ask getNative of a discarded event!"); + // Wouldn't be true if MHostEvent could be true. + assert(MState == HES_Complete && "Expected to have a completed event!"); + Plugin.call(MEvent, PI_EVENT_COMPLETE); } if (Plugin.getBackend() == backend::opencl) Plugin.call(getHandleRef()); From f7518c43b4bc8c0f4d32a880cd8b1cd189606854 Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Fri, 21 Oct 2022 14:57:16 -0700 Subject: [PATCH 2/3] Modify piEventCreate semantics to create signaled/completed event --- sycl/plugins/level_zero/pi_level_zero.cpp | 14 +++++++------- sycl/plugins/opencl/pi_opencl.cpp | 9 +++++++-- sycl/source/detail/backend_impl.hpp | 2 -- sycl/source/detail/event_impl.cpp | 6 ------ 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp index 271f10a0c876b..6b6fc56208a67 100644 --- a/sycl/plugins/level_zero/pi_level_zero.cpp +++ b/sycl/plugins/level_zero/pi_level_zero.cpp @@ -5650,7 +5650,10 @@ static pi_result EventCreate(pi_context Context, pi_queue Queue, pi_result piEventCreate(pi_context Context, pi_event *RetEvent) { pi_result Result = EventCreate(Context, nullptr, true, RetEvent); (*RetEvent)->RefCountExternal++; - return Result; + if (Result != PI_SUCCESS) + return Result; + ZE_CALL(zeEventHostSignal, ((*RetEvent)->ZeEvent)); + return PI_SUCCESS; } pi_result piEventGetInfo(pi_event Event, pi_event_info ParamName, @@ -5964,12 +5967,9 @@ pi_result piEventSetCallback(pi_event Event, pi_int32 CommandExecCallbackType, } pi_result piEventSetStatus(pi_event Event, pi_int32 ExecutionStatus) { - if (ExecutionStatus == PI_EVENT_COMPLETE) - zeEventHostSignal(Event->ZeEvent); - else - // We don't expect this path ever to be executed when called from SYCL RT. - die("piEventSetStatus: with anything but PI_EVENT_COMPLETE is " - "unsupported!"); + (void)Event; + (void)ExecutionStatus; + die("piEventSetStatus: deprecated, to be removed"); return PI_SUCCESS; } diff --git a/sycl/plugins/opencl/pi_opencl.cpp b/sycl/plugins/opencl/pi_opencl.cpp index 07fd25da52f35..f26bc5516c8c4 100644 --- a/sycl/plugins/opencl/pi_opencl.cpp +++ b/sycl/plugins/opencl/pi_opencl.cpp @@ -973,8 +973,13 @@ pi_result piKernelGetSubGroupInfo(pi_kernel kernel, pi_device device, pi_result piEventCreate(pi_context context, pi_event *ret_event) { pi_result ret_err = PI_ERROR_INVALID_OPERATION; - *ret_event = cast( - clCreateUserEvent(cast(context), cast(&ret_err))); + auto *cl_err = cast(&ret_err); + + cl_event e = clCreateUserEvent(cast(context), cl_err); + *ret_event = cast(e); + if (*cl_err != CL_SUCCESS) + return ret_err; + *cl_err = clSetUserEventStatus(e, CL_COMPLETE); return ret_err; } diff --git a/sycl/source/detail/backend_impl.hpp b/sycl/source/detail/backend_impl.hpp index b5969589c3af7..fb3ab07737dda 100644 --- a/sycl/source/detail/backend_impl.hpp +++ b/sycl/source/detail/backend_impl.hpp @@ -15,8 +15,6 @@ __SYCL_INLINE_VER_NAMESPACE(_V1) { namespace detail { template backend getImplBackend(const T &Impl) { - // If that would ever become possible, event_impl::getNative needs to be - // updated too. assert(!Impl->is_host() && "Cannot get the backend for host."); return Impl->getPlugin().getBackend(); } diff --git a/sycl/source/detail/event_impl.cpp b/sycl/source/detail/event_impl.cpp index 68f4a1971109c..7d88e52baf27d 100644 --- a/sycl/source/detail/event_impl.cpp +++ b/sycl/source/detail/event_impl.cpp @@ -375,12 +375,6 @@ pi_native_handle event_impl::getNative() { MIsInitialized = true; auto TempContext = MContext.get()->getHandleRef(); Plugin.call(TempContext, &MEvent); - // See an assert in sycl::detail::getImplBackend. - assert(!MHostEvent && "Can't get native event from a host event!"); - assert(!isDiscarded() && "Can't ask getNative of a discarded event!"); - // Wouldn't be true if MHostEvent could be true. - assert(MState == HES_Complete && "Expected to have a completed event!"); - Plugin.call(MEvent, PI_EVENT_COMPLETE); } if (Plugin.getBackend() == backend::opencl) Plugin.call(getHandleRef()); From 018fb4bb45f5b55a020ad5071ff2e65e6254203e Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Tue, 25 Oct 2022 09:53:12 -0700 Subject: [PATCH 3/3] docstring/version bump --- sycl/include/sycl/detail/pi.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sycl/include/sycl/detail/pi.h b/sycl/include/sycl/detail/pi.h index d79025bf7d306..26b6695c21b96 100644 --- a/sycl/include/sycl/detail/pi.h +++ b/sycl/include/sycl/detail/pi.h @@ -52,9 +52,10 @@ // 10.13 Added new PI_EXT_ONEAPI_QUEUE_DISCARD_EVENTS queue property. // 10.14 Add PI_EXT_INTEL_DEVICE_INFO_FREE_MEMORY as an extension for // piDeviceGetInfo. +// 11.15 piEventCreate creates even in the signalled state now. -#define _PI_H_VERSION_MAJOR 10 -#define _PI_H_VERSION_MINOR 14 +#define _PI_H_VERSION_MAJOR 11 +#define _PI_H_VERSION_MINOR 15 #define _PI_STRING_HELPER(a) #a #define _PI_CONCAT(a, b) _PI_STRING_HELPER(a.b) @@ -1397,6 +1398,11 @@ piextKernelGetNativeHandle(pi_kernel kernel, pi_native_handle *nativeHandle); // // Events // + +/// Create PI event object in a signalled/completed state. +/// +/// \param context is the PI context of the event. +/// \param ret_event is the PI even created. __SYCL_EXPORT pi_result piEventCreate(pi_context context, pi_event *ret_event); __SYCL_EXPORT pi_result piEventGetInfo(pi_event event, pi_event_info param_name,