From e658d6f2789f74736616f3427830630ce201db8f Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Tue, 21 Apr 2020 13:20:03 +0300 Subject: [PATCH 1/3] [SYCL] Make queue's non-USM event ownership temporary Signed-off-by: Sergey Semenov --- sycl/source/detail/queue_impl.cpp | 21 ++++++++++++++++----- sycl/source/detail/queue_impl.hpp | 10 +++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/sycl/source/detail/queue_impl.cpp b/sycl/source/detail/queue_impl.cpp index ccaab363c64b5..da07df6b949db 100644 --- a/sycl/source/detail/queue_impl.cpp +++ b/sycl/source/detail/queue_impl.cpp @@ -49,7 +49,7 @@ event queue_impl::memset(shared_ptr_class Impl, void *Ptr, return event(); event ResEvent{pi::cast(Event), Context}; - addEvent(ResEvent); + addUSMEvent(ResEvent); return ResEvent; } @@ -63,7 +63,7 @@ event queue_impl::memcpy(shared_ptr_class Impl, void *Dest, return event(); event ResEvent{pi::cast(Event), Context}; - addEvent(ResEvent); + addUSMEvent(ResEvent); return ResEvent; } @@ -81,13 +81,19 @@ event queue_impl::mem_advise(const void *Ptr, size_t Length, Advice, &Event); event ResEvent{pi::cast(Event), Context}; - addEvent(ResEvent); + addUSMEvent(ResEvent); return ResEvent; } void queue_impl::addEvent(event Event) { + std::weak_ptr EventWeakPtr{getSyclObjImpl(Event)}; std::lock_guard Guard(MMutex); - MEvents.push_back(std::move(Event)); + MEvents.push_back(std::move(EventWeakPtr)); +} + +void queue_impl::addUSMEvent(event Event) { + std::lock_guard Guard(MMutex); + MUSMEvents.push_back(std::move(Event)); } void *queue_impl::instrumentationProlog(const detail::code_location &CodeLoc, @@ -175,8 +181,13 @@ void queue_impl::wait(const detail::code_location &CodeLoc) { #endif std::lock_guard Guard(MMutex); - for (auto &Event : MEvents) + for (std::weak_ptr &EventImplWeakPtr : MEvents) { + if (std::shared_ptr EventImplPtr = EventImplWeakPtr.lock()) + EventImplPtr->wait(EventImplPtr); + } + for (event &Event : MUSMEvents) { Event.wait(); + } MEvents.clear(); #ifdef XPTI_ENABLE_INSTRUMENTATION diff --git a/sycl/source/detail/queue_impl.hpp b/sycl/source/detail/queue_impl.hpp index e48d59694af27..937223f114c5f 100644 --- a/sycl/source/detail/queue_impl.hpp +++ b/sycl/source/detail/queue_impl.hpp @@ -382,12 +382,20 @@ class queue_impl { /// \param Event is the event to be stored void addEvent(event Event); + /// Stores a USM operation event that should be associated with the queue + /// + /// \param Event is the event to be stored + void addUSMEvent(event Event); + /// Protects all the fields that can be changed by class' methods. mutex_class MMutex; DeviceImplPtr MDevice; const ContextImplPtr MContext; - vector_class MEvents; + vector_class> MEvents; + // USM operations are not added to the scheduler command graph, + // queue is the only owner on the runtime side. + vector_class MUSMEvents; exception_list MExceptions; const async_handler MAsyncHandler; const property_list MPropList; From 74576a9185bbaf98de1391fd256e45133c37c0cd Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Wed, 22 Apr 2020 13:33:49 +0300 Subject: [PATCH 2/3] Add a test Signed-off-by: Sergey Semenov --- sycl/test/basic_tests/event_release.cpp | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 sycl/test/basic_tests/event_release.cpp diff --git a/sycl/test/basic_tests/event_release.cpp b/sycl/test/basic_tests/event_release.cpp new file mode 100644 index 0000000000000..0ca83f5bfca5a --- /dev/null +++ b/sycl/test/basic_tests/event_release.cpp @@ -0,0 +1,36 @@ +// RUN: %clangxx -fsycl %s -o %t.out +// RUN: env SYCL_PI_TRACE=1 %CPU_RUN_PLACEHOLDER %t.out 2>&1 %CPU_CHECK_PLACEHOLDER +#include +#include + +// The test checks that pi_events are released without queue destruction +// or call to queue::wait, when the corresponding commands are cleaned up. + +using namespace cl::sycl; + +class Foo; + +int main() { + int Val = 0; + int Gold = 42; + + // Check event releasing without queue destruction + queue *QPtr = new queue{}; + + { + buffer Buf{&Val, range<1>(1)}; + QPtr->submit([&](handler &Cgh) { + auto Acc = Buf.get_access(Cgh); + Cgh.single_task([=]() { + Acc[0] = Gold; + }); + }); + } + + // Buffer destruction triggers execution graph cleanup, check that both + // events (one for launching the kernel and one for memory transfer to host) + // are released. + // CHECK: piEventRelease + // CHECK: piEventRelease + assert(Val == Gold); +} From c32d78f7dd3c45bbd5e32a8cbe5de72652d230b9 Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Wed, 22 Apr 2020 14:47:23 +0300 Subject: [PATCH 3/3] Adjust the test Signed-off-by: Sergey Semenov --- sycl/test/basic_tests/event_release.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sycl/test/basic_tests/event_release.cpp b/sycl/test/basic_tests/event_release.cpp index 0ca83f5bfca5a..37e0d4258b28b 100644 --- a/sycl/test/basic_tests/event_release.cpp +++ b/sycl/test/basic_tests/event_release.cpp @@ -2,6 +2,7 @@ // RUN: env SYCL_PI_TRACE=1 %CPU_RUN_PLACEHOLDER %t.out 2>&1 %CPU_CHECK_PLACEHOLDER #include #include +#include // The test checks that pi_events are released without queue destruction // or call to queue::wait, when the corresponding commands are cleaned up. @@ -14,12 +15,11 @@ int main() { int Val = 0; int Gold = 42; - // Check event releasing without queue destruction - queue *QPtr = new queue{}; + queue Q; { buffer Buf{&Val, range<1>(1)}; - QPtr->submit([&](handler &Cgh) { + Q.submit([&](handler &Cgh) { auto Acc = Buf.get_access(Cgh); Cgh.single_task([=]() { Acc[0] = Gold; @@ -33,4 +33,6 @@ int main() { // CHECK: piEventRelease // CHECK: piEventRelease assert(Val == Gold); + // CHECK: End of main scope + std::cout << "End of main scope" << std::endl; }