Linking to system TBB considered harmful (primarily on Ubuntu 18.04)

[MathiasMagnus]

I know that HIP-CPU may not share the support matrix of ROCm, however projects that rely on ROCm often do (and HIP-CPU targets these applications). I'm sure you're aware of the sad situation of the parallel STL and Ubuntu 18.04, the gist of the problem being that while installing GCC 8 and 9 sporting parallel STL is simple enough from the ppa:ubuntu-toolchain-r/test PPA, it requires a version of TBB newer than the one provided by the system.

In this case it is painfully clear that simply adding -ltbb to the linker command line will not fly. There are two issues with this:

• It doesn't compile.
• Making it compile robustly is like walking on broken glass. Adding a non-system lib as a dependency while potentially said lib being installed on the system is tricky. If not using proper CMake detection and relying on SYSTEM BEFORE flags, one must tread carefully.
  • For those living the life and surfing the waves of package managers (🙌) installing TBB comes by for eg. saying vcpkg install tbb, which in turn will install a statically built libtbb.a, which with the current build scripts will sit right next to -ltbb on the linker command line, simultaneously linking to an older system shared library and a cutting edge static version. What could go wrong?

Even if HIP-CPU does not intend on supporting this quirky situation, blindly adding -ltbb to the command line (beside being bad form) opens the door to consuming TBB twice, moreover with different versions. (Projects consuming HIP-CPU may want to depend on TBB on their own right as well.)

I would urge trying to detect libstdc++ instead off being on Linux (after all this situation is the consequence of an implementation detail) using CMake's try_run():

Try compiling a <srcfile>. Returns TRUE or FALSE for success or failure in <compileResultVar>. If the compile succeeded, runs the executable and returns its exit code in <runResultVar>.

or using check_cxx_symbol_exists, whichever is easiest. (I don't have a turnkey solution, but we should.) If STL is libc++, only then do

check_cxx_symbol_exists(SOME_LIBCXX_ONLY_SYMBOLNAME "iostream" USING_LIBCXX)
if(USING_LIBCXX)
  if(NOT TARGET TBB::tbb)
    find_package(TBB REQUIRED)
  endif()
  if(NOT TARGET Threads::Threads)
    find_package(Threads REQUIRED)
  endif()
endif(USING_LIBCXX)
target_link_libraries(${PROJECT_NAME}
  INTERFACE
    $<$<BOOL:${USING_LIBCXX}>:
      ${CMAKE_DL_LIBS}
      Threads::Threads
      TBB::tbb
    >
)

_(Note: AFAIK having to find and link to Threads::Threads (and perhaps ${CMAKE_DL_LIBS} too) is yet another quirkiness libc++, but don't call me out on that one. It isn't painful to have on non-Linux systems, it's minimal bloat to configuration time and console output on Windows.)_

[AlexVlx]

This is indeed a significant concern, and there was some measure of doubt before actually adding the convenience hip_cpu_rt target; in one incarnation the onus was fully on the user to provide the additional dependencies. In the end we have something that is not very robust, as you have observed. That being said, I'd be more than happy to take PRs improving the situation. Even fleshing this discussion out and thrashing the details of desirable behaviour in a broad sense would be nice.