[SYCL] Don't return last event in ext_oneapi_submit_barrier (#20235)

This is a cherry-pick of #20159 (and #20241)

**Problem**

As an optimization in `ext_oneapi_submit_barrier`, for in-order queue,
when waitlist is empty, we return the last event, if it is available.
However, after #18277, we no longer
store last event and instead submit a marker to get the last event. And
this caused performance regression in some of our internal benchmarks.

**Solution**
Don't return last event and instead submit a barrier with empty
waitlist.


Patch-by: Udit Kumar Agarwal <[email protected]>

Author: AlexeySachkov

sycl/source/queue.cpp

@@ -327,24 +327,6 @@ void queue::wait_and_throw_proxy(const detail::code_location &CodeLoc) {
   impl->wait_and_throw(CodeLoc);
 }
 
-static event
-getBarrierEventForInorderQueueHelper(detail::queue_impl &QueueImpl) {
-  // This function should not be called when a queue is recording to a graph,
-  // as a graph can record from multiple queues and we cannot guarantee the
-  // last node added by an in-order queue will be the last node added to the
-  // graph.
-  assert(!QueueImpl.hasCommandGraph() &&
-         "Should not be called in on graph recording.");
-
-  sycl::detail::optional<event> LastEvent = QueueImpl.getLastEvent();
-  if (LastEvent)
-    return *LastEvent;
-
-  // If there was no last event, we create an empty one.
-  return detail::createSyclObjFromImpl<event>(
-      detail::event_impl::create_default_event());
-}
-
 /// Prevents any commands submitted afterward to this queue from executing
 /// until all commands previously submitted to this queue have entered the
 /// complete state.
@@ -367,18 +349,17 @@ event queue::ext_oneapi_submit_barrier(const detail::code_location &CodeLoc) {
 /// group is being enqueued on.
 event queue::ext_oneapi_submit_barrier(const std::vector<event> &WaitList,
                                        const detail::code_location &CodeLoc) {
+
+  // If waitlist contains only empty, default constructed events, ignore
+  // them.
   bool AllEventsEmptyOrNop = std::all_of(
       begin(WaitList), end(WaitList), [&](const event &Event) -> bool {
         detail::event_impl &EventImpl = *detail::getSyclObjImpl(Event);
         return (EventImpl.isDefaultConstructed() || EventImpl.isNOP()) &&
                !EventImpl.hasCommandGraph();
       });
-  if (is_in_order() && !impl->hasCommandGraph() && !impl->MIsProfilingEnabled &&
-      AllEventsEmptyOrNop) {
-    return getBarrierEventForInorderQueueHelper(*impl);
-  }
 
-  if (WaitList.empty())
+  if (WaitList.empty() || AllEventsEmptyOrNop)
     return submit([=](handler &CGH) { CGH.ext_oneapi_barrier(); }, CodeLoc);
   else
     return submit([=](handler &CGH) { CGH.ext_oneapi_barrier(WaitList); },

sycl/test-e2e/InorderQueue/in_order_ext_oneapi_submit_barrier.cpp

@@ -53,28 +53,13 @@ int main() {
     std::cout << "Test 2" << std::endl;
     *Res = 0;
 
-    auto Event1 = Q.submit(
-        [&](sycl::handler &CGH) { CGH.host_task([&] { *Res += 1; }); });
-    auto BarrierEvent1 = Q.ext_oneapi_submit_barrier();
-    assert(checkBarrierEvent(Q.get_backend(), Event1, BarrierEvent1,
-                             false /* host tasks used */));
-    auto Event2 = Q.submit([&](sycl::handler &CGH) { CGH.fill(Res, 10, 1); });
+    Q.submit([&](sycl::handler &CGH) { CGH.host_task([&] { *Res += 1; }); });
+    Q.ext_oneapi_submit_barrier();
+    Q.submit([&](sycl::handler &CGH) { CGH.fill(Res, 10, 1); });
 
     Q.wait();
     assert(*Res == 10);
   }
-
-  {
-    // Test cast 3 - empty queue.
-    std::cout << "Test 3" << std::endl;
-    sycl::queue EmptyQ({sycl::property::queue::in_order{}});
-    auto BarrierEvent = EmptyQ.ext_oneapi_submit_barrier();
-    assert(
-        BarrierEvent.get_info<sycl::info::event::command_execution_status>() ==
-        sycl::info::event_command_status::complete);
-    BarrierEvent.wait();
-  }
-
   {
     // Test cast 4 - graph.
     sycl::queue GQueue{sycl::property::queue::in_order{}};

sycl/test-e2e/Regression/ext_oneapi_barrier_opt.cpp

@@ -12,6 +12,7 @@ add_sycl_unittest(ExtensionsTests OBJECT
   CompositeDevice.cpp
   OneAPIProd.cpp
   EnqueueFunctionsEvents.cpp
+  ExtOneapiBarrierOpt.cpp
   ProfilingTag.cpp
   KernelProperties.cpp
   NoDeviceIPVersion.cpp

sycl/unittests/Extensions/CMakeLists.txt

@@ -0,0 +1,60 @@
+//==------------------- ExtOneapiBarrierOpt.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
+//
+//===----------------------------------------------------------------------===//
+
+#include <gtest/gtest.h>
+#include <helpers/ScopedEnvVar.hpp>
+#include <helpers/UrMock.hpp>
+#include <sycl/sycl.hpp>
+
+using namespace sycl;
+
+inline thread_local uint32_t NumEventsInWaitList;
+
+static ur_result_t redefinedEnqueueEventsWaitWithBarrierExt(void *pParams) {
+  auto params =
+      *static_cast<ur_enqueue_events_wait_with_barrier_ext_params_t *>(pParams);
+  NumEventsInWaitList = *(params.pnumEventsInWaitList);
+  return UR_RESULT_SUCCESS;
+}
+
+class ExtOneapiBarrierOptTest : public ::testing::Test {
+public:
+  ExtOneapiBarrierOptTest() : Mock{} {}
+
+protected:
+  void SetUp() override { NumEventsInWaitList = 0; }
+
+protected:
+  sycl::unittest::UrMock<> Mock;
+};
+
+// Check that ext_oneapi_submit_barrier works fine in the scenarios
+// when provided waitlist consists of only empty events.
+// Tets for https://github.com/intel/llvm/pull/12951
+TEST_F(ExtOneapiBarrierOptTest, EmptyEventTest) {
+  sycl::queue q1{{sycl::property::queue::in_order()}};
+
+  mock::getCallbacks().set_after_callback(
+      "urEnqueueEventsWaitWithBarrierExt",
+      &redefinedEnqueueEventsWaitWithBarrierExt);
+
+  NumEventsInWaitList = 100;
+  q1.ext_oneapi_submit_barrier();
+  ASSERT_EQ(0u, NumEventsInWaitList);
+
+  // ext_oneapi_submit_barrier should ignore empty, default constructed events.
+  sycl::event E1{};
+  NumEventsInWaitList = 100;
+  q1.ext_oneapi_submit_barrier({E1});
+  ASSERT_EQ(0u, NumEventsInWaitList);
+
+  sycl::event E2{};
+  NumEventsInWaitList = 100;
+  q1.ext_oneapi_submit_barrier({E1, E2});
+  ASSERT_EQ(0u, NumEventsInWaitList);
+}