oneDAL
oneDAL copied to clipboard
Improvements in cmake configuration
Describe the bug
I've just started using the oneDAL cmake configuration that is generated from oneDALConfig.cmake.in
. It works well, thanks a lot for this helpful tool!
However I've found a number of small shortcomings. Attached is also a patch, but it was generated from the official Intel oneAPI binaries, so it may or may not apply 100% cleanly to the sources here.
General Issues
Sometimes it can happen that cmake may try to find a package repeatedly. This can happen for example because an upstream package is using find_dependency(DAL)
and the current package is also trying to find oneDAL: find_package(DAL)
. However it seems that oneDALConfig.cmake.in
is not fit for this. Here are some issues I found:
- The variable
DAL_LIBS
is not cleared at the beginning, so it will always become longer and longer. - The section
if (NOT TARGET oneDAL::${_dal_component})
does not cover all of the foreach()-loop. Therefore if a target already exists,set_target_properties()
will be called on the existing target, however the library path_dal_lib
is empty on the second run. Therefore all targets have noIMPORTED_LOCATION
.
Here a patch that resolves these issues: oneDALConfig.cmake.patch.gz
Windows Debug Build Issues
On Windows, the cmake package configuration does currently not respect the debug library names. The debug versions of libraries have a suffix d
at the end. But the cmake configuration will try to link the release versions (without the suffix). Here is a small patch with a suggested solution:
oneDALConfig.cmake-win32-debug-support.patch.gz
Naming Suggestions
Another small issue is the names of the configuration variables. Its great that the cmake configuration can be configured with the help of variables. Currently their names are TARGET_LINK
, USE_PARALLEL
, USE_DPCPP
, and USE_NEW_IFACES
. Some good improvements would be:
- to make these names specific to oneDAL. In MKL they use the prefix
MKL_
, maybe oneDAL could do the same? For large projects it can otherwise be a bit confusing when users override these settings from outside. I use now-DTARGET_LINK=static
in several projects which can confuse other users that my whole project would link static, not just static oneDAL. - The names are not very consistent with MKL. Maybe they could be made more consistent? MKL currently uses the following names and values:
# MKL_ARCH
# Values: ia32 intel64
# Default: intel64
# MKL_LINK
# Values: static, dynamic, sdl
# Default: dynamic
# Exceptions:- DPC++ doesn't support sdl
# MKL_THREADING
# Values: sequential,
# intel_thread (Intel OpenMP),
# gnu_thread (GNU OpenMP),
# pgi_thread (PGI OpenMP),
# tbb_thread
# Default: intel_thread
# Exceptions:- DPC++ defaults to tbb, PGI compiler on Windows defaults to pgi_thread
# MKL_INTERFACE (for MKL_ARCH=intel64 only)
# Values: lp64, ilp64
# GNU or INTEL interface will be selected based on Compiler.
# Default: ilp64
# MKL_MPI
# Values: intelmpi, mpich, openmpi, msmpi, mshpc
# Default: intelmpi
Just a kind reminder about this humble question. Let me know if I can do something to support this request?
@emmenlau - thanks a lot for analysis and patches - be free to comment on - https://github.com/oneapi-src/oneDAL/pull/2230