Skip to content

Commit a285b9d

Browse files
[SYCL] Fix unreleased events returned by queue USM API (#1979)
The events were created with the interoperability constructor that performed an unneeeded retain. Signed-off-by: Sergey Semenov <[email protected]>
1 parent e582224 commit a285b9d

File tree

4 files changed

+37
-14
lines changed

4 files changed

+37
-14
lines changed

sycl/source/detail/queue_impl.cpp

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <CL/sycl/detail/memory_manager.hpp>
1111
#include <CL/sycl/detail/pi.hpp>
1212
#include <CL/sycl/device.hpp>
13+
#include <detail/event_impl.hpp>
1314
#include <detail/queue_impl.hpp>
1415

1516
#include <cstring>
@@ -39,48 +40,59 @@ template <> device queue_impl::get_info<info::queue::device>() const {
3940
return get_device();
4041
}
4142

43+
static event prepareUSMEvent(shared_ptr_class<detail::queue_impl> QueueImpl,
44+
RT::PiEvent NativeEvent) {
45+
auto EventImpl = std::make_shared<detail::event_impl>(QueueImpl);
46+
EventImpl->getHandleRef() = NativeEvent;
47+
EventImpl->setContextImpl(detail::getSyclObjImpl(QueueImpl->get_context()));
48+
return detail::createSyclObjFromImpl<event>(EventImpl);
49+
}
50+
4251
event queue_impl::memset(shared_ptr_class<detail::queue_impl> Impl, void *Ptr,
4352
int Value, size_t Count) {
4453
context Context = get_context();
45-
RT::PiEvent Event = nullptr;
46-
MemoryManager::fill_usm(Ptr, Impl, Count, Value, /*DepEvents*/ {}, Event);
54+
RT::PiEvent NativeEvent = nullptr;
55+
MemoryManager::fill_usm(Ptr, Impl, Count, Value, /*DepEvents*/ {},
56+
NativeEvent);
4757

4858
if (Context.is_host())
4959
return event();
5060

51-
event ResEvent{pi::cast<cl_event>(Event), Context};
61+
event ResEvent = prepareUSMEvent(Impl, NativeEvent);
5262
addUSMEvent(ResEvent);
5363
return ResEvent;
5464
}
5565

5666
event queue_impl::memcpy(shared_ptr_class<detail::queue_impl> Impl, void *Dest,
5767
const void *Src, size_t Count) {
5868
context Context = get_context();
59-
RT::PiEvent Event = nullptr;
60-
MemoryManager::copy_usm(Src, Impl, Count, Dest, /*DepEvents*/ {}, Event);
69+
RT::PiEvent NativeEvent = nullptr;
70+
MemoryManager::copy_usm(Src, Impl, Count, Dest, /*DepEvents*/ {},
71+
NativeEvent);
6172

6273
if (Context.is_host())
6374
return event();
6475

65-
event ResEvent{pi::cast<cl_event>(Event), Context};
76+
event ResEvent = prepareUSMEvent(Impl, NativeEvent);
6677
addUSMEvent(ResEvent);
6778
return ResEvent;
6879
}
6980

70-
event queue_impl::mem_advise(const void *Ptr, size_t Length,
81+
event queue_impl::mem_advise(shared_ptr_class<detail::queue_impl> Impl,
82+
const void *Ptr, size_t Length,
7183
pi_mem_advice Advice) {
7284
context Context = get_context();
7385
if (Context.is_host()) {
7486
return event();
7587
}
7688

7789
// non-Host device
78-
RT::PiEvent Event = nullptr;
90+
RT::PiEvent NativeEvent = nullptr;
7991
const detail::plugin &Plugin = getPlugin();
8092
Plugin.call<PiApiKind::piextUSMEnqueueMemAdvise>(getHandleRef(), Ptr, Length,
81-
Advice, &Event);
93+
Advice, &NativeEvent);
8294

83-
event ResEvent{pi::cast<cl_event>(Event), Context};
95+
event ResEvent = prepareUSMEvent(Impl, NativeEvent);
8496
addUSMEvent(ResEvent);
8597
return ResEvent;
8698
}

sycl/source/detail/queue_impl.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,12 @@ class queue_impl {
344344
/// Provides additional information to the underlying runtime about how
345345
/// different allocations are used.
346346
///
347+
/// \param Impl is a shared_ptr to this queue.
347348
/// \param Ptr is a USM pointer to the allocation.
348349
/// \param Length is a number of bytes in the allocation.
349350
/// \param Advice is a device-defined advice for the specified allocation.
350-
event mem_advise(const void *Ptr, size_t Length, pi_mem_advice Advice);
351+
event mem_advise(shared_ptr_class<queue_impl> Impl, const void *Ptr,
352+
size_t Length, pi_mem_advice Advice);
351353

352354
/// Puts exception to the list of asynchronous ecxeptions.
353355
///

sycl/source/queue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ event queue::memcpy(void *dest, const void *src, size_t count) {
100100
}
101101

102102
event queue::mem_advise(const void *ptr, size_t length, pi_mem_advice advice) {
103-
return impl->mem_advise(ptr, length, advice);
103+
return impl->mem_advise(impl, ptr, length, advice);
104104
}
105105

106106
event queue::submit_impl(function_class<void(handler &)> CGH,

sycl/unittests/queue/wait.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ struct TestCtx {
1818

1919
context &Ctx;
2020
int NEventsWaitedFor = 0;
21+
int EventReferenceCount = 0;
2122
};
2223

2324
std::unique_ptr<TestCtx> TestContext;
@@ -29,6 +30,7 @@ pi_result redefinedUSMEnqueueMemset(pi_queue queue, void *ptr, pi_int32 value,
2930
pi_event *event) {
3031
// Provide a dummy non-nullptr value
3132
*event = reinterpret_cast<pi_event>(1);
33+
TestContext->EventReferenceCount = 1;
3234
return PI_SUCCESS;
3335
}
3436

@@ -50,9 +52,15 @@ pi_result redefinedEventGetInfo(pi_event event, pi_event_info param_name,
5052
return PI_SUCCESS;
5153
}
5254

53-
pi_result redefinedEventRetain(pi_event event) { return PI_SUCCESS; }
55+
pi_result redefinedEventRetain(pi_event event) {
56+
++TestContext->EventReferenceCount;
57+
return PI_SUCCESS;
58+
}
5459

55-
pi_result redefinedEventRelease(pi_event event) { return PI_SUCCESS; }
60+
pi_result redefinedEventRelease(pi_event event) {
61+
--TestContext->EventReferenceCount;
62+
return PI_SUCCESS;
63+
}
5664

5765
// Check that the USM events are cleared from the queue upon call to wait(),
5866
// so that they are not waited for multiple times.
@@ -87,6 +95,7 @@ TEST(QueueWaitTest, USMEventClear) {
8795
Q.memset(HostAlloc, 42, 1);
8896
Q.wait();
8997
ASSERT_EQ(TestContext->NEventsWaitedFor, 1);
98+
ASSERT_EQ(TestContext->EventReferenceCount, 0);
9099
Q.wait();
91100
ASSERT_EQ(TestContext->NEventsWaitedFor, 1);
92101
}

0 commit comments

Comments
 (0)