Skip to content

Commit 41b9947

Browse files
RossBruntonRossBrunton
committed
Revert "[UR] Replace calls to UR in native handle functions to proper OpenCL functions (#17016)"
This reverts commit e1cf106. In testing, it turns out a number of people link against `libOpenCL.so.1` rather than `libOpenCL.so`, which is considered a seperate library by the linker. Reverting this change for now while we consider the best option.
1 parent 37df391 commit 41b9947

25 files changed

+42
-225
lines changed

‎sycl/cmake/modules/AddSYCLUnitTest.cmake‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ macro(add_sycl_unittest test_dirname link_variant)
77
set(LLVM_REQUIRES_EH ON)
88
set(LLVM_REQUIRES_RTTI ON)
99

10-
get_target_property(SYCL_BINARY_DIR sycl-toolchain BINARY_DIR)
11-
1210
string(TOLOWER "${CMAKE_BUILD_TYPE}" build_type_lower)
1311
if (MSVC AND build_type_lower MATCHES "debug")
1412
set(sycl_obj_target "sycld_object")
@@ -61,7 +59,7 @@ macro(add_sycl_unittest test_dirname link_variant)
6159
SYCL_CONFIG_FILE_NAME=null.cfg
6260
SYCL_DEVICELIB_NO_FALLBACK=1
6361
SYCL_CACHE_DIR="${CMAKE_BINARY_DIR}/sycl_cache"
64-
"LD_LIBRARY_PATH=${SYCL_BINARY_DIR}/unittests/lib:${CMAKE_BINARY_DIR}/lib:$ENV{LD_LIBRARY_PATH}"
62+
"LD_LIBRARY_PATH=${CMAKE_BINARY_DIR}/lib:$ENV{LD_LIBRARY_PATH}"
6563
${CMAKE_CURRENT_BINARY_DIR}/${test_dirname}
6664
DEPENDS
6765
${test_dirname}
@@ -70,28 +68,15 @@ macro(add_sycl_unittest test_dirname link_variant)
7068

7169
add_dependencies(check-sycl-unittests check-sycl-${test_dirname})
7270

73-
if(WIN32)
74-
# Windows doesn't support LD_LIBRARY_PATH, so instead we copy the mock OpenCL binary next to the test and ensure
75-
# that the test itself links to OpenCL (rather than through ur_adapter_opencl.dll)
76-
set(mock_ocl ${CMAKE_CURRENT_BINARY_DIR}/OpenCL.dll)
77-
add_custom_command(TARGET ${test_dirname} POST_BUILD
78-
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:mockOpenCL> ${mock_ocl}
79-
DEPENDS mockOpenCL
80-
BYPRODUCTS ${mock_ocl}
81-
COMMAND_EXPAND_LISTS
82-
)
83-
endif()
84-
8571
target_link_libraries(${test_dirname}
8672
PRIVATE
87-
mockOpenCL
8873
LLVMTestingSupport
8974
OpenCL-Headers
9075
unified-runtime::mock
9176
${SYCL_LINK_LIBS}
9277
)
9378

94-
add_dependencies(${test_dirname} ur_adapter_mock mockOpenCL)
79+
add_dependencies(${test_dirname} ur_adapter_mock)
9580

9681
if(SYCL_ENABLE_EXTENSION_JIT)
9782
target_link_libraries(${test_dirname} PRIVATE sycl-jit)

‎sycl/include/sycl/detail/os_util.hpp‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -106,24 +106,6 @@ void fileTreeWalk(const std::string Path,
106106
std::function<void(const std::string)> Func,
107107
bool ignoreErrors = false);
108108

109-
void *dynLookup(const char *WinName, const char *LinName, const char *FunName);
110-
111-
// Look up a function name that was dynamically linked
112-
// This is used by the runtime where it needs to manipulate native handles (e.g.
113-
// retaining OpenCL handles). On Windows, the symbol name is looked up in
114-
// `WinName`. In Linux, it uses `LinName`.
115-
//
116-
// The library must already have been loaded (perhaps by UR), otherwise this
117-
// function throws a SYCL runtime exception.
118-
template <typename fn>
119-
fn *dynLookupFunction(const char *WinName, const char *LinName,
120-
const char *FunName) {
121-
return reinterpret_cast<fn *>(dynLookup(WinName, LinName, FunName));
122-
}
123-
#define __SYCL_OCL_CALL(FN, ...) \
124-
(sycl::_V1::detail::dynLookupFunction<decltype(FN)>( \
125-
"OpenCL", "libOpenCL.so", #FN)(__VA_ARGS__))
126-
127109
} // namespace detail
128110
} // namespace _V1
129111
} // namespace sycl

‎sycl/source/backend.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ __SYCL_EXPORT event make_event(ur_native_handle_t NativeHandle,
181181
std::make_shared<event_impl>(UrEvent, Context));
182182

183183
if (Backend == backend::opencl)
184-
__SYCL_OCL_CALL(clRetainEvent, ur::cast<cl_event>(NativeHandle));
184+
Adapter->call<UrApiKind::urEventRetain>(UrEvent);
185185
return Event;
186186
}
187187

@@ -205,7 +205,7 @@ make_kernel_bundle(ur_native_handle_t NativeHandle,
205205
"urProgramCreateWithNativeHandle resulted in a null program handle.");
206206

207207
if (ContextImpl->getBackend() == backend::opencl)
208-
__SYCL_OCL_CALL(clRetainProgram, ur::cast<cl_program>(NativeHandle));
208+
Adapter->call<UrApiKind::urProgramRetain>(UrProgram);
209209

210210
std::vector<ur_device_handle_t> ProgramDevices;
211211
uint32_t NumDevices = 0;
@@ -352,7 +352,7 @@ kernel make_kernel(const context &TargetContext,
352352
&UrKernel);
353353

354354
if (Backend == backend::opencl)
355-
__SYCL_OCL_CALL(clRetainKernel, ur::cast<cl_kernel>(NativeHandle));
355+
Adapter->call<UrApiKind::urKernelRetain>(UrKernel);
356356

357357
// Construct the SYCL queue from UR queue.
358358
return detail::createSyclObjFromImpl<kernel>(

‎sycl/source/detail/buffer_impl.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ buffer_impl::getNativeVector(backend BackendName) const {
8787
auto Adapter = Platform->getAdapter();
8888

8989
if (Platform->getBackend() == backend::opencl) {
90-
__SYCL_OCL_CALL(clRetainMemObject, ur::cast<cl_mem>(NativeMem));
90+
Adapter->call<UrApiKind::urMemRetain>(NativeMem);
9191
}
9292

9393
ur_native_handle_t Handle = 0;

‎sycl/source/detail/context_impl.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,11 +303,10 @@ context_impl::findMatchingDeviceImpl(ur_device_handle_t &DeviceUR) const {
303303

304304
ur_native_handle_t context_impl::getNative() const {
305305
const auto &Adapter = getAdapter();
306+
if (getBackend() == backend::opencl)
307+
Adapter->call<UrApiKind::urContextRetain>(getHandleRef());
306308
ur_native_handle_t Handle;
307309
Adapter->call<UrApiKind::urContextGetNativeHandle>(getHandleRef(), &Handle);
308-
if (getBackend() == backend::opencl) {
309-
__SYCL_OCL_CALL(clRetainContext, ur::cast<cl_context>(Handle));
310-
}
311310
return Handle;
312311
}
313312

‎sycl/source/detail/device_image_impl.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,11 @@ class device_image_impl {
300300
const auto &ContextImplPtr = detail::getSyclObjImpl(MContext);
301301
const AdapterPtr &Adapter = ContextImplPtr->getAdapter();
302302

303+
if (ContextImplPtr->getBackend() == backend::opencl)
304+
Adapter->call<UrApiKind::urProgramRetain>(MProgram);
303305
ur_native_handle_t NativeProgram = 0;
304306
Adapter->call<UrApiKind::urProgramGetNativeHandle>(MProgram,
305307
&NativeProgram);
306-
if (ContextImplPtr->getBackend() == backend::opencl)
307-
__SYCL_OCL_CALL(clRetainProgram, ur::cast<cl_program>(NativeProgram));
308308

309309
return NativeProgram;
310310
}

‎sycl/source/detail/device_impl.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ bool device_impl::is_affinity_supported(
9999

100100
cl_device_id device_impl::get() const {
101101
// TODO catch an exception and put it to list of asynchronous exceptions
102-
__SYCL_OCL_CALL(clRetainDevice, ur::cast<cl_device_id>(getNative()));
102+
getAdapter()->call<UrApiKind::urDeviceRetain>(MDevice);
103103
return ur::cast<cl_device_id>(getNative());
104104
}
105105

@@ -346,11 +346,10 @@ std::vector<device> device_impl::create_sub_devices() const {
346346

347347
ur_native_handle_t device_impl::getNative() const {
348348
auto Adapter = getAdapter();
349+
if (getBackend() == backend::opencl)
350+
Adapter->call<UrApiKind::urDeviceRetain>(getHandleRef());
349351
ur_native_handle_t Handle;
350352
Adapter->call<UrApiKind::urDeviceGetNativeHandle>(getHandleRef(), &Handle);
351-
if (getBackend() == backend::opencl) {
352-
__SYCL_OCL_CALL(clRetainDevice, ur::cast<cl_device_id>(Handle));
353-
}
354353
return Handle;
355354
}
356355

‎sycl/source/detail/event_impl.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,10 +511,10 @@ ur_native_handle_t event_impl::getNative() {
511511
this->setHandle(UREvent);
512512
Handle = UREvent;
513513
}
514+
if (MContext->getBackend() == backend::opencl)
515+
Adapter->call<UrApiKind::urEventRetain>(Handle);
514516
ur_native_handle_t OutHandle;
515517
Adapter->call<UrApiKind::urEventGetNativeHandle>(Handle, &OutHandle);
516-
if (MContext->getBackend() == backend::opencl)
517-
__SYCL_OCL_CALL(clRetainEvent, ur::cast<cl_event>(OutHandle));
518518
return OutHandle;
519519
}
520520

‎sycl/source/detail/kernel_impl.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ class kernel_impl {
7575
///
7676
/// \return a valid cl_kernel instance
7777
cl_kernel get() const {
78+
getAdapter()->call<UrApiKind::urKernelRetain>(MKernel);
7879
ur_native_handle_t nativeHandle = 0;
7980
getAdapter()->call<UrApiKind::urKernelGetNativeHandle>(MKernel,
8081
&nativeHandle);
81-
__SYCL_OCL_CALL(clRetainKernel, ur::cast<cl_kernel>(nativeHandle));
8282
return ur::cast<cl_kernel>(nativeHandle);
8383
}
8484

@@ -212,12 +212,12 @@ class kernel_impl {
212212
ur_native_handle_t getNative() const {
213213
const AdapterPtr &Adapter = MContext->getAdapter();
214214

215+
if (MContext->getBackend() == backend::opencl)
216+
Adapter->call<UrApiKind::urKernelRetain>(MKernel);
217+
215218
ur_native_handle_t NativeKernel = 0;
216219
Adapter->call<UrApiKind::urKernelGetNativeHandle>(MKernel, &NativeKernel);
217220

218-
if (MContext->getBackend() == backend::opencl)
219-
__SYCL_OCL_CALL(clRetainKernel, ur::cast<cl_kernel>(NativeKernel));
220-
221221
return NativeKernel;
222222
}
223223

‎sycl/source/detail/os_util.cpp‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -291,39 +291,6 @@ size_t getDirectorySize(const std::string &Path, bool ignoreErrors) {
291291
return DirSizeVar;
292292
}
293293

294-
// Look up a function name that was dynamically linked
295-
// This is used by the runtime where it needs to manipulate native handles (e.g.
296-
// retaining OpenCL handles). On Windows, the symbol name is looked up in
297-
// `WinName`. In Linux, it uses `LinName`.
298-
//
299-
// The library must already have been loaded (perhaps by UR), otherwise this
300-
// function throws a SYCL runtime exception.
301-
void *dynLookup([[maybe_unused]] const char *WinName,
302-
[[maybe_unused]] const char *LinName, const char *FunName) {
303-
#ifdef __SYCL_RT_OS_WINDOWS
304-
auto handle = GetModuleHandleA(WinName);
305-
if (!handle) {
306-
throw sycl::exception(make_error_code(errc::runtime),
307-
std::string(WinName) + " library is not loaded");
308-
}
309-
auto *retVal = GetProcAddress(handle, FunName);
310-
#else
311-
auto handle = dlopen(LinName, RTLD_LAZY | RTLD_NOLOAD);
312-
if (!handle) {
313-
throw sycl::exception(make_error_code(errc::runtime),
314-
std::string(LinName) + " library is not loaded");
315-
}
316-
auto *retVal = dlsym(handle, FunName);
317-
dlclose(handle);
318-
#endif
319-
if (!retVal) {
320-
throw sycl::exception(make_error_code(errc::runtime),
321-
"Symbol " + std::string(FunName) +
322-
" could not be found");
323-
}
324-
return reinterpret_cast<void *>(retVal);
325-
}
326-
327294
} // namespace detail
328295
} // namespace _V1
329296
} // namespace sycl

0 commit comments

Comments
 (0)