From 3690165af6eed1534cc5b334f4ad085bf2feae5c Mon Sep 17 00:00:00 2001 From: Sergey V Maslov Date: Thu, 18 Nov 2021 12:40:44 -0800 Subject: [PATCH 1/5] [SYCL] piextQueueCreateWithNativeHandle should not be followed by a piQueueRetain Signed-off-by: Sergey V Maslov --- sycl/plugins/opencl/pi_opencl.cpp | 1 + sycl/source/detail/queue_impl.hpp | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/sycl/plugins/opencl/pi_opencl.cpp b/sycl/plugins/opencl/pi_opencl.cpp index e259ac5e942cf..3e52769eb0f80 100644 --- a/sycl/plugins/opencl/pi_opencl.cpp +++ b/sycl/plugins/opencl/pi_opencl.cpp @@ -389,6 +389,7 @@ pi_result piextQueueCreateWithNativeHandle(pi_native_handle nativeHandle, (void)ownNativeHandle; assert(piQueue != nullptr); *piQueue = reinterpret_cast(nativeHandle); + clRetainCommandQueue(*piQueue); return PI_SUCCESS; } diff --git a/sycl/source/detail/queue_impl.hpp b/sycl/source/detail/queue_impl.hpp index 1d4a524e12e8f..e9a201e71a990 100644 --- a/sycl/source/detail/queue_impl.hpp +++ b/sycl/source/detail/queue_impl.hpp @@ -129,9 +129,6 @@ class queue_impl { sizeof(Device), &Device, nullptr); MDevice = DeviceImplPtr(new device_impl(Device, Context->getPlatformImpl())); - - // TODO catch an exception and put it to list of asynchronous exceptions - getPlugin().call(MQueues[0]); } ~queue_impl() { From 8055c91246a3ae5188f697a0a9f7c7acbcf12b89 Mon Sep 17 00:00:00 2001 From: Sergey V Maslov Date: Thu, 18 Nov 2021 15:25:00 -0800 Subject: [PATCH 2/5] fix build Signed-off-by: Sergey V Maslov --- sycl/plugins/opencl/pi_opencl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/plugins/opencl/pi_opencl.cpp b/sycl/plugins/opencl/pi_opencl.cpp index 3e52769eb0f80..57d2e15b96317 100644 --- a/sycl/plugins/opencl/pi_opencl.cpp +++ b/sycl/plugins/opencl/pi_opencl.cpp @@ -389,7 +389,7 @@ pi_result piextQueueCreateWithNativeHandle(pi_native_handle nativeHandle, (void)ownNativeHandle; assert(piQueue != nullptr); *piQueue = reinterpret_cast(nativeHandle); - clRetainCommandQueue(*piQueue); + clRetainCommandQueue(cast(nativeHandle)); return PI_SUCCESS; } From 8f0b9b61db35a1917dcfe1820478ae5f1439a8e3 Mon Sep 17 00:00:00 2001 From: Sergey V Maslov Date: Mon, 22 Nov 2021 17:12:34 -0800 Subject: [PATCH 3/5] [SYCL] Add unittest Signed-off-by: Sergey V Maslov --- sycl/unittests/pi/CMakeLists.txt | 1 + sycl/unittests/pi/piInteropRetain.cpp | 65 +++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100755 sycl/unittests/pi/piInteropRetain.cpp diff --git a/sycl/unittests/pi/CMakeLists.txt b/sycl/unittests/pi/CMakeLists.txt index 53c69f5cae2fa..0d174ac94dbeb 100644 --- a/sycl/unittests/pi/CMakeLists.txt +++ b/sycl/unittests/pi/CMakeLists.txt @@ -7,6 +7,7 @@ add_sycl_unittest(PiTests OBJECT PiMock.cpp PlatformTest.cpp pi_arguments_handler.cpp + piInteropRetain.cpp ) add_dependencies(PiTests sycl) diff --git a/sycl/unittests/pi/piInteropRetain.cpp b/sycl/unittests/pi/piInteropRetain.cpp new file mode 100755 index 0000000000000..f1636e925a6c4 --- /dev/null +++ b/sycl/unittests/pi/piInteropRetain.cpp @@ -0,0 +1,65 @@ +//==--------------------- Wait.cpp --- queue unit tests --------------------==// +// +// 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 +#include +#include + +#include +#include +#include + +namespace { +using namespace cl::sycl; + +static bool QueueRetainCalled; +pi_result unexpectedQueueRetain(pi_queue Queue) { return PI_INVALID_QUEUE; } +pi_result expectedQueueRetain(pi_queue Queue) { QueueRetainCalled = true; return PI_SUCCESS; } + +bool preparePiMock(platform &Plt) { + if (Plt.is_host()) { + std::cout << "Not run on host - no PI events created in that case" + << std::endl; + return false; + } + if (detail::getSyclObjImpl(Plt)->getPlugin().getBackend() != + backend::opencl) { + std::cout << "Not run on non-OpenCL backend" + << std::endl; + return false; + } + return true; +} + +TEST(PiInteropTest, CheckRetain) { + platform Plt{default_selector()}; + if (!preparePiMock(Plt)) + return; + context Ctx{Plt.get_devices()[0]}; + + unittest::PiMock Mock{Plt}; + + // The queue construction should not call to piQueueRetain. Instead piQueueCreate should return the "retained" queue. + QueueRetainCalled = false; + Mock.redefine(unexpectedQueueRetain); + queue Q{Ctx, default_selector()}; + EXPECT_FALSE(QueueRetainCalled); + + QueueRetainCalled = false; + Mock.redefine(expectedQueueRetain); + cl_command_queue OCLQ = get_native(Q); + EXPECT_TRUE(QueueRetainCalled); + + // The make_queue should not call to piQueueRetain. The piextCreateQueueWithNative handle should do the "retain" if needed. + QueueRetainCalled = false; + Mock.redefine(unexpectedQueueRetain); + queue Q1 = make_queue(OCLQ, Ctx); + EXPECT_FALSE(QueueRetainCalled); +} + +} // namespace From b3397bf7a25911dff70bff2b79a7ad7a4ddee737 Mon Sep 17 00:00:00 2001 From: Sergey V Maslov Date: Mon, 22 Nov 2021 17:31:07 -0800 Subject: [PATCH 4/5] clang-format Signed-off-by: Sergey V Maslov --- sycl/unittests/pi/piInteropRetain.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) mode change 100755 => 100644 sycl/unittests/pi/piInteropRetain.cpp diff --git a/sycl/unittests/pi/piInteropRetain.cpp b/sycl/unittests/pi/piInteropRetain.cpp old mode 100755 new mode 100644 index f1636e925a6c4..bc7961568434c --- a/sycl/unittests/pi/piInteropRetain.cpp +++ b/sycl/unittests/pi/piInteropRetain.cpp @@ -10,16 +10,19 @@ #include #include +#include #include #include -#include namespace { using namespace cl::sycl; static bool QueueRetainCalled; pi_result unexpectedQueueRetain(pi_queue Queue) { return PI_INVALID_QUEUE; } -pi_result expectedQueueRetain(pi_queue Queue) { QueueRetainCalled = true; return PI_SUCCESS; } +pi_result expectedQueueRetain(pi_queue Queue) { + QueueRetainCalled = true; + return PI_SUCCESS; +} bool preparePiMock(platform &Plt) { if (Plt.is_host()) { @@ -29,8 +32,7 @@ bool preparePiMock(platform &Plt) { } if (detail::getSyclObjImpl(Plt)->getPlugin().getBackend() != backend::opencl) { - std::cout << "Not run on non-OpenCL backend" - << std::endl; + std::cout << "Not run on non-OpenCL backend" << std::endl; return false; } return true; @@ -44,7 +46,8 @@ TEST(PiInteropTest, CheckRetain) { unittest::PiMock Mock{Plt}; - // The queue construction should not call to piQueueRetain. Instead piQueueCreate should return the "retained" queue. + // The queue construction should not call to piQueueRetain. Instead + // piQueueCreate should return the "retained" queue. QueueRetainCalled = false; Mock.redefine(unexpectedQueueRetain); queue Q{Ctx, default_selector()}; @@ -55,7 +58,8 @@ TEST(PiInteropTest, CheckRetain) { cl_command_queue OCLQ = get_native(Q); EXPECT_TRUE(QueueRetainCalled); - // The make_queue should not call to piQueueRetain. The piextCreateQueueWithNative handle should do the "retain" if needed. + // The make_queue should not call to piQueueRetain. The + // piextCreateQueueWithNative handle should do the "retain" if needed. QueueRetainCalled = false; Mock.redefine(unexpectedQueueRetain); queue Q1 = make_queue(OCLQ, Ctx); From e95ced8156b0f24567fe986bf278fcf339c1dc16 Mon Sep 17 00:00:00 2001 From: Sergey V Maslov Date: Tue, 23 Nov 2021 16:03:08 -0800 Subject: [PATCH 5/5] address review comments Signed-off-by: Sergey V Maslov --- sycl/unittests/pi/piInteropRetain.cpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/sycl/unittests/pi/piInteropRetain.cpp b/sycl/unittests/pi/piInteropRetain.cpp index bc7961568434c..d8a56f0f3ad3a 100644 --- a/sycl/unittests/pi/piInteropRetain.cpp +++ b/sycl/unittests/pi/piInteropRetain.cpp @@ -1,4 +1,4 @@ -//==--------------------- Wait.cpp --- queue unit tests --------------------==// +//==--------------------- piInteropRetain.cpp -- check proper retain calls -==// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -17,10 +17,9 @@ namespace { using namespace cl::sycl; -static bool QueueRetainCalled; -pi_result unexpectedQueueRetain(pi_queue Queue) { return PI_INVALID_QUEUE; } -pi_result expectedQueueRetain(pi_queue Queue) { - QueueRetainCalled = true; +static int QueueRetainCalled = 0; +pi_result redefinedQueueRetain(pi_queue Queue) { + ++QueueRetainCalled; return PI_SUCCESS; } @@ -48,22 +47,17 @@ TEST(PiInteropTest, CheckRetain) { // The queue construction should not call to piQueueRetain. Instead // piQueueCreate should return the "retained" queue. - QueueRetainCalled = false; - Mock.redefine(unexpectedQueueRetain); + Mock.redefine(redefinedQueueRetain); queue Q{Ctx, default_selector()}; - EXPECT_FALSE(QueueRetainCalled); + EXPECT_TRUE(QueueRetainCalled == 0); - QueueRetainCalled = false; - Mock.redefine(expectedQueueRetain); cl_command_queue OCLQ = get_native(Q); - EXPECT_TRUE(QueueRetainCalled); + EXPECT_TRUE(QueueRetainCalled == 1); // The make_queue should not call to piQueueRetain. The // piextCreateQueueWithNative handle should do the "retain" if needed. - QueueRetainCalled = false; - Mock.redefine(unexpectedQueueRetain); queue Q1 = make_queue(OCLQ, Ctx); - EXPECT_FALSE(QueueRetainCalled); + EXPECT_TRUE(QueueRetainCalled == 1); } } // namespace