[SYCL][Docs] Allow discarded events case for ext_oneapi_get_last_event (#15638)

This commit changes the behavior for queue::ext_oneapi_get_last_event
when the queue either does not currently have a last event or the last
event was discarded. In these cases, the implementation will now insert
a marker event at the end of the queue which signifies the work related
to the last command in the queue.

Additionally, this allows us to relax the requirements for the extension
to now allow it to be used on queues where discarding of events is
enabled.

Signed-off-by: Larsen, Steffen <[email protected]>

Author: steffenlarsen

sycl/doc/extensions/experimental/sycl_ext_oneapi_in_order_queue_events.asciidoc

@@ -122,11 +122,6 @@ copy of the event that was passed to `ext_oneapi_set_external_event()`.
 
 Calls to this member function throw a `sycl::exception` with `errc::invalid` if
 the queue does not have the `property::queue::in_order` property.
-
-Calls to this member function throw a `sycl::exception` with `errc::invalid` if
-the queue has the `ext::oneapi::property::queue::discard_events` property from
-the
-link:../supported/sycl_ext_oneapi_discard_queue_events.asciidoc[sycl_ext_oneapi_discard_queue_events extension].
 
 a| 
 [source, c++]
@@ -154,10 +149,5 @@ undefined.
 
 Calls to this member function throw a `sycl::exception` with `errc::invalid` if
 the queue does not have the `property::queue::in_order` property.
-
-Calls to this member function throw a `sycl::exception` with `errc::invalid` if
-the queue has the `ext::oneapi::property::queue::discard_events` property from
-the
-link:../supported/sycl_ext_oneapi_discard_queue_events.asciidoc[sycl_ext_oneapi_discard_queue_events extension].
 |====
 --

sycl/source/detail/queue_impl.hpp

@@ -710,6 +710,18 @@ class queue_impl {
 
   const property_list &getPropList() const { return MPropList; }
 
+  /// Inserts a marker event at the end of the queue. Waiting for this marker
+  /// will wait for the completion of all work in the queue at the time of the
+  /// insertion, but will not act as a barrier unless the queue is in-order.
+  EventImplPtr insertMarkerEvent(const std::shared_ptr<queue_impl> &Self) {
+    auto ResEvent = std::make_shared<detail::event_impl>(Self);
+    ur_event_handle_t UREvent = nullptr;
+    getAdapter()->call<UrApiKind::urEnqueueEventsWait>(getHandleRef(), 0,
+                                                       nullptr, &UREvent);
+    ResEvent->setHandle(UREvent);
+    return ResEvent;
+  }
+
 protected:
   event discard_or_return(const event &Event);
 

sycl/source/queue.cpp

@@ -361,24 +361,22 @@ event queue::ext_oneapi_get_last_event() const {
     throw sycl::exception(
         make_error_code(errc::invalid),
         "ext_oneapi_get_last_event() can only be called on in-order queues.");
-  if (impl->MDiscardEvents)
-    throw sycl::exception(
-        make_error_code(errc::invalid),
-        "ext_oneapi_get_last_event() cannot be called on queues with the "
-        "ext::oneapi::property::queue::discard_events property.");
-  return impl->getLastEvent();
+
+  event LastEvent = impl->getLastEvent();
+  // If the last event was discarded or a NOP, we insert a marker to represent
+  // an event at end.
+  auto LastEventImpl = detail::getSyclObjImpl(LastEvent);
+  if (LastEventImpl->isDiscarded() || LastEventImpl->isNOP())
+    LastEvent =
+        detail::createSyclObjFromImpl<event>(impl->insertMarkerEvent(impl));
+  return LastEvent;
 }
 
 void queue::ext_oneapi_set_external_event(const event &external_event) {
   if (!is_in_order())
     throw sycl::exception(make_error_code(errc::invalid),
                           "ext_oneapi_set_external_event() can only be called "
                           "on in-order queues.");
-  if (impl->MDiscardEvents)
-    throw sycl::exception(
-        make_error_code(errc::invalid),
-        "ext_oneapi_set_external_event() cannot be called on queues with the "
-        "ext::oneapi::property::queue::discard_events property.");
   return impl->setExternalEvent(external_event);
 }
 

sycl/test-e2e/InOrderEventsExt/get_last_event.cpp

@@ -13,6 +13,7 @@
 //      call to ext_oneapi_set_external_event.
 
 #include <sycl/detail/core.hpp>
+#include <sycl/ext/oneapi/experimental/enqueue_functions.hpp>
 #include <sycl/properties/all_properties.hpp>
 #include <sycl/usm.hpp>
 
@@ -33,11 +34,18 @@ int main() {
 
   int Failed = 0;
 
-  Failed += Check(Q, "single_task", [&]() { return Q.single_task([]() {}); });
+  // Check that a valid event is returned on the empty queue.
+  Q.ext_oneapi_get_last_event().wait();
+
+  // Check that a valid event is returned after enqueuing work without events.
+  sycl::ext::oneapi::experimental::single_task(Q, []() {});
+  Q.ext_oneapi_get_last_event().wait();
 
+  // Check event equivalences - This is an implementation detail, but useful
+  // for checking behavior.
+  Failed += Check(Q, "single_task", [&]() { return Q.single_task([]() {}); });
   Failed += Check(Q, "parallel_for",
                   [&]() { return Q.parallel_for(32, [](sycl::id<1>) {}); });
-
   Failed += Check(Q, "host_task", [&]() {
     return Q.submit([&](sycl::handler &CGH) { CGH.host_task([]() {}); });
   });

sycl/unittests/Extensions/CMakeLists.txt

@@ -15,6 +15,7 @@ add_sycl_unittest(ExtensionsTests OBJECT
   ProfilingTag.cpp
   KernelProperties.cpp
   NoDeviceIPVersion.cpp
+  GetLastEvent.cpp
 )
 
 add_subdirectory(CommandGraph)

sycl/unittests/Extensions/GetLastEvent.cpp

@@ -0,0 +1,64 @@
+//==------------------------- GetLastEvent.cpp -----------------------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// Tests the behavior of queue::ext_oneapi_get_last_event.
+
+#include <detail/event_impl.hpp>
+#include <gtest/gtest.h>
+#include <helpers/TestKernel.hpp>
+#include <helpers/UrMock.hpp>
+#include <sycl/ext/oneapi/experimental/enqueue_functions.hpp>
+#include <sycl/properties/queue_properties.hpp>
+#include <sycl/queue.hpp>
+
+using namespace sycl;
+
+thread_local ur_event_handle_t MarkerEventLatest = nullptr;
+static ur_result_t redefinedEnqueueEventsWaitAfter(void *pParams) {
+  auto params = *static_cast<ur_enqueue_events_wait_params_t *>(pParams);
+  MarkerEventLatest = **(params.pphEvent);
+  return UR_RESULT_SUCCESS;
+}
+static ur_result_t redefinedEventRelease(void *) { return UR_RESULT_SUCCESS; }
+
+TEST(GetLastEventEmptyQueue, CheckEmptyQueueLastEvent) {
+  unittest::UrMock<> Mock;
+  platform Plt = sycl::platform();
+
+  MarkerEventLatest = nullptr;
+  mock::getCallbacks().set_after_callback("urEnqueueEventsWait",
+                                          &redefinedEnqueueEventsWaitAfter);
+  mock::getCallbacks().set_before_callback("urEventRelease",
+                                           &redefinedEventRelease);
+
+  queue Q{property::queue::in_order{}};
+  event E = Q.ext_oneapi_get_last_event();
+  ur_event_handle_t UREvent = detail::getSyclObjImpl(E)->getHandle();
+  ASSERT_NE(MarkerEventLatest, ur_event_handle_t{nullptr});
+  ASSERT_EQ(UREvent, MarkerEventLatest);
+}
+
+TEST(GetLastEventEmptyQueue, CheckEventlessWorkQueue) {
+  unittest::UrMock<> Mock;
+  platform Plt = sycl::platform();
+
+  MarkerEventLatest = nullptr;
+  mock::getCallbacks().set_after_callback("urEnqueueEventsWait",
+                                          &redefinedEnqueueEventsWaitAfter);
+  mock::getCallbacks().set_before_callback("urEventRelease",
+                                           &redefinedEventRelease);
+
+  queue Q{property::queue::in_order{}};
+
+  // The following single_task does not return an event, so it is expected that
+  // the last event query creates a new marker event.
+  sycl::ext::oneapi::experimental::single_task<TestKernel<>>(Q, []() {});
+  event E = Q.ext_oneapi_get_last_event();
+  ur_event_handle_t UREvent = detail::getSyclObjImpl(E)->getHandle();
+  ASSERT_NE(MarkerEventLatest, ur_event_handle_t{nullptr});
+  ASSERT_EQ(UREvent, MarkerEventLatest);
+}