CMake installation problems

[al42and]

Hi! Thank you for this project :)

Below are some observations I made about the current build system. Nothing is too critical, but I think it would be nice to iron such sings out eventually.

1. Trying to build a shared library (using -DBUILD_SHARED_LIBS=ON) fails with a "recompile with -fPIC" error.

Full repro:

$ docker run --pull=always --rm -it silkeh/clang:17 bash -c 'apt update && apt install -qy git libboost-all-dev && git clone https://github.com/celerity/SimSYCL.git && cd SimSYCL && cmake -S. -Bbuild/ -DBUILD_SHARED_LIBS=ON -DCMAKE_CXX_COMPILER=clang++-17 && cmake --build build/'
...
...
[ 14%] Linking CXX shared library libsimsycl.so
/usr/bin/ld: _deps/libenvpp-build/libenvpp.a(libenvpp_environment_unix.cpp.o): warning: relocation against `_ZSt19piecewise_construct' in read-only section `.text._ZNSt8__detail9_Map_baseINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS6_S6_ESaIS9_ENS_10_Select1stESt8equal_toIS6_ESt4hashIS6_ENS_18_Mod_range_hashingENS_20_Default_ranged_hashENS_20_Prime_rehash_policyENS_17_Hashtable_traitsILb1ELb0ELb1EEELb1EEixERS8_[_ZNSt8__detail9_Map_baseINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS6_S6_ESaIS9_ENS_10_Select1stESt8equal_toIS6_ESt4hashIS6_ENS_18_Mod_range_hashingENS_20_Default_ranged_hashENS_20_Prime_rehash_policyENS_17_Hashtable_traitsILb1ELb0ELb1EEELb1EEixERS8_]'
/usr/bin/ld: _deps/libenvpp-build/libenvpp.a(libenvpp_testing.cpp.o): relocation R_X86_64_PC32 against symbol `_ZN3env6detail21g_testing_environmentB5cxx11E' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
clang++-17: error: linker command failed with exit code 1 (use -v to see invocation)

Setting -DCMAKE_POSITION_INDEPENDENT_CODE=ON to force PIC globally helps.

2. The installed simsycl-config.cmake contains find_dependency for Boost, nlohmann_json, and libenvpp. However, all those libraries are PRIVATE, and, for a static library build, there should be no need to expose them to the application trying to link to libsimsycl.a. This makes linking to SimSYCL more complicated.

Just removing those find_dependency does not help, more dark CMake magic is needed?

3. There seems to be an unwritten tradition (followed by ComputeCpp and AdaptiveCpp, but not by DPC++) of having an add_sycl_to_target function to ease the integration of SYCL into the client code. How about adding something like this to the

function(add_sycl_to_target)
    cmake_parse_arguments(
        PARSE_ARGV 0 # No positional arguments
        ARGS # Prefix for the resulting variables
        "" # No options
        "TARGET" # One-value keyword
        "SOURCES" # Multi-value keyword
    )
    target_link_libraries(${ARGS_TARGET} PRIVATE SimSYCL::simsycl)
endfunction(add_sycl_to_target)

[PeterTh]

Thanks for the feedback! We'll have a look, I think we'd certainly want to fix 1 and 2.

As for 3, since DPC++ doesn't have it I feel like clients can no longer depend on this anyway, though I personally liked it -- it's also really simple in our case. I wouldn't be opposed to adding it, what do you think @fknorr ?

[fknorr]

Thanks!

1. It seems like we have to manually forward this info to the libenvpp build (and the other 3rdparty deps) - CMake docs for POSITION_INDEPENDENT_CODE mention that it will automatically be enabled for a shared library build, but if a shared SimSYCL links a static libenvpp that one will probably not be position independent automatically.
2. You probably meant to write shared library here, right? We need to expose those dependencies for a static build because user transitively depends on libenvpp.a even though that library will only used internally by libsimsycl.a. A shared simsycl.so could do without these extra installed libraries.
3. I'm a bit torn on this, because the API requires the user to list all sources even though SimSYCL does not actually use that information.

[al42and]

1. FWIW, adding if(BUILD_SHARED_LIBS) set_target_properties(libenvpp PROPERTIES POSITION_INDEPENDENT_CODE ON) endif() to libenvpp's CMakeLists.txt helps (e.g., using PATCH command of FetchContent), but I am not a CMake guru to say whether that's a proper solution.

2. Static build: "user transitively depends on libenvpp.a" Maybe I'm missing something, but I don't see any mentions of envpp in SimSYCL's headers, so as long as libenvpp is statically linked into libsimsycl.a, there should be no reason for a client application to care about libenvpp, right? Shared build: Agree.

3. "since DPC++ doesn't have it I feel like clients can no longer depend on this anyway" SimSYCL can contribute to putting peer pressure on DPC++ to adopt it :) "the API requires the user to list all sources even though SimSYCL does not actually use that information" True, that is moderately annoying. Overall, that is not a huge deal; users can trivially add this function to their code if they want to.

[fknorr]

3. is now implemented in #25.