Skip to content

Commit 299f506

Browse files
[SYCL] Remove incorrect call to piQueueRetain (#4988)
piextQueueCreateWithNativeHandle should not be followed by a piQueueRetain
1 parent f058fe0 commit 299f506

File tree

4 files changed

+65
-3
lines changed

4 files changed

+65
-3
lines changed

sycl/plugins/opencl/pi_opencl.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ pi_result piextQueueCreateWithNativeHandle(pi_native_handle nativeHandle,
389389
(void)ownNativeHandle;
390390
assert(piQueue != nullptr);
391391
*piQueue = reinterpret_cast<pi_queue>(nativeHandle);
392+
clRetainCommandQueue(cast<cl_command_queue>(nativeHandle));
392393
return PI_SUCCESS;
393394
}
394395

sycl/source/detail/queue_impl.hpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,6 @@ class queue_impl {
129129
sizeof(Device), &Device, nullptr);
130130
MDevice =
131131
DeviceImplPtr(new device_impl(Device, Context->getPlatformImpl()));
132-
133-
// TODO catch an exception and put it to list of asynchronous exceptions
134-
getPlugin().call<PiApiKind::piQueueRetain>(MQueues[0]);
135132
}
136133

137134
~queue_impl() {

sycl/unittests/pi/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ add_sycl_unittest(PiTests OBJECT
77
PiMock.cpp
88
PlatformTest.cpp
99
pi_arguments_handler.cpp
10+
piInteropRetain.cpp
1011
)
1112

1213
add_dependencies(PiTests sycl)

sycl/unittests/pi/piInteropRetain.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//==--------------------- piInteropRetain.cpp -- check proper retain calls -==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include <CL/opencl.h>
10+
#include <CL/sycl.hpp>
11+
#include <CL/sycl/backend/opencl.hpp>
12+
13+
#include <detail/queue_impl.hpp>
14+
#include <gtest/gtest.h>
15+
#include <helpers/PiMock.hpp>
16+
17+
namespace {
18+
using namespace cl::sycl;
19+
20+
static int QueueRetainCalled = 0;
21+
pi_result redefinedQueueRetain(pi_queue Queue) {
22+
++QueueRetainCalled;
23+
return PI_SUCCESS;
24+
}
25+
26+
bool preparePiMock(platform &Plt) {
27+
if (Plt.is_host()) {
28+
std::cout << "Not run on host - no PI events created in that case"
29+
<< std::endl;
30+
return false;
31+
}
32+
if (detail::getSyclObjImpl(Plt)->getPlugin().getBackend() !=
33+
backend::opencl) {
34+
std::cout << "Not run on non-OpenCL backend" << std::endl;
35+
return false;
36+
}
37+
return true;
38+
}
39+
40+
TEST(PiInteropTest, CheckRetain) {
41+
platform Plt{default_selector()};
42+
if (!preparePiMock(Plt))
43+
return;
44+
context Ctx{Plt.get_devices()[0]};
45+
46+
unittest::PiMock Mock{Plt};
47+
48+
// The queue construction should not call to piQueueRetain. Instead
49+
// piQueueCreate should return the "retained" queue.
50+
Mock.redefine<detail::PiApiKind::piQueueRetain>(redefinedQueueRetain);
51+
queue Q{Ctx, default_selector()};
52+
EXPECT_TRUE(QueueRetainCalled == 0);
53+
54+
cl_command_queue OCLQ = get_native<backend::opencl>(Q);
55+
EXPECT_TRUE(QueueRetainCalled == 1);
56+
57+
// The make_queue should not call to piQueueRetain. The
58+
// piextCreateQueueWithNative handle should do the "retain" if needed.
59+
queue Q1 = make_queue<backend::opencl>(OCLQ, Ctx);
60+
EXPECT_TRUE(QueueRetainCalled == 1);
61+
}
62+
63+
} // namespace

0 commit comments

Comments
 (0)