pocl icon indicating copy to clipboard operation
pocl copied to clipboard

CMake code to find and link against OpenCL / ICD loader is problematic

Open JayFoxRox opened this issue 2 years ago • 5 comments

The current CMake setup is rather outdated and doesn't use the recommended way of integrating the OpenCL-ICD-Loader (which uses import targets now). Instead a couple of variables get created here:

https://github.com/pocl/pocl/blob/5a99e12d0bb78427ad948ac368589135507f1b59/CMakeLists.txt#L1022-L1023

However, OPENCL_LIBDIR is never actually used to control the linker, so if the OpenCL library is not part of the system library path, the build-script will fail to find it.

I think the build-system needs major redesign in this area.

To combat this, I'm currently using this patch:

diff --git a/bin/CMakeLists.txt b/bin/CMakeLists.txt
index 2fa37f44..f41983b1 100644
--- a/bin/CMakeLists.txt
+++ b/bin/CMakeLists.txt
@@ -28,7 +28,9 @@ set_opencl_header_includes()
 add_executable(poclcc poclcc.c)
 harden(poclcc)
 
-target_link_libraries(poclcc poclu ${OPENCL_LIBS})
+message("OPENCL_LIBS=${OPENCL_LIBS}")
+target_link_libraries(poclcc PRIVATE poclu ${OPENCL_LIBS})
+target_link_directories(poclcc PRIVATE "${OPENCL_LIBDIR}")
 
 install(TARGETS "poclcc" RUNTIME
         DESTINATION "${POCL_INSTALL_PUBLIC_BINDIR}" COMPONENT "poclcc")
diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
index 2e2ddc0a..817f368a 100644
--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -78,6 +78,8 @@ else()
   set(OPENCL_LIBS "${PTHREAD_LIBRARY};${POCL_LIBRARY_NAME};${POCL_TRANSITIVE_LIBS}")
 
 endif()
+message("OPENCL_LIBS=${OPENCL_LIBS} OPENCL_LIBDIR=${OPENCL_LIBDIR}")
+link_directories("${OPENCL_LIBDIR}")
 
 if(SANITIZER_OPTIONS)
   list(INSERT OPENCL_LIBS 0 ${SANITIZER_LIBS})

JayFoxRox avatar Aug 29 '23 19:08 JayFoxRox

Did you try the main branch or the release?

isuruf avatar Aug 29 '23 19:08 isuruf

I'm on the main branch. The current main branch revision also still contains the problematic code linked above.

JayFoxRox avatar Aug 29 '23 19:08 JayFoxRox

It does have the unused code, but I believe https://github.com/pocl/pocl/pull/1252 fixes the issue that you are running into.

isuruf avatar Aug 29 '23 19:08 isuruf

#1252 fixes a different problem: It was about not being able to find the OpenCL ICD Loader package (= Bad OCL_ICD_LIBRARIES OCL_ICD_LIBDIR).

In my case OpenCL ICD Loader is found, so both of these variables are valid, but they don't get used correctly.

These variables get used to set OPENCL_LIBRARIES (= OCL_ICD_LIBRARIES) and OPENCL_LIBDIR and (= OCL_ICD_LIBDIR) then. Example: OPENCL_LIBS=Threads::Threads;OpenCL OPENCL_LIBDIR=/opt/homebrew/Cellar/opencl-icd-loader/2023.04.17/lib; both valid.

However, from that point onwards, only OPENCL_LIBRARIES is used. OPENCL_LIBDIR is used for some things, but the linker never gets told about it.

This means the linker links against -lOpenCL (from OPENCL_LIBRARIES) but it never adds the path where that lib can be found (should be from OPENCL_LIBDIR). So essentially missing -L${OPENCL_LIBDIR}.

A redesign with import targets would make this error impossible, as the library name, library path (and other potentially important things like flags) are part of the same "object".

JayFoxRox avatar Aug 29 '23 19:08 JayFoxRox

A redesign with import targets would make this error impossible

Indeed, most of the CMake code is old and doesn't use targets. However any redesign must also keep in mind that PoCL should also support other ICD loaders (Khronos), and also builds without ICD.

franz avatar Aug 30 '23 11:08 franz