[SYCL] Correctly track dependencies when using check-sycl target

[jchlanda]

check-sycl should only depend on sycl-toolchain. Currently building those two targets from a fresh checkout will result in linker errors:

/usr/bin/ld: cannot find -lgtest_main
/usr/bin/ld: cannot find -lgtest
/usr/bin/ld: cannot find -lLLVMTestingSupport                                      
clang-13: error: linker command failed with exit code 1 (use -v to see invocation) 

Admittedly, this is somewhat an edge case, as the problem goes away when all targets are build prior to running check-sycl.

This patch ensures that gtest and gtest_main are known to cmake before it parses projects, so the dependencies can be correctly tracked.
Additional, it fixes a bug in handling of the libraries when creating SYCL executable, as well as adds llvm-dis to the list of SYCL test dependencies (needed by lit tests).

[bader]

check-sycl should only depend on sycl-toolchain. Currently building those two targets from a fresh checkout will result in linker errors:

/usr/bin/ld: cannot find -lgtest_main
/usr/bin/ld: cannot find -lgtest
/usr/bin/ld: cannot find -lLLVMTestingSupport                                      
clang-13: error: linker command failed with exit code 1 (use -v to see invocation) 

Admittedly, this is somewhat an edge case, as the problem goes away when all targets are build prior to running check-sycl.

This patch ensures that gtest and gtest_main are known to cmake before it parses projects, so the dependencies can be correctly tracked.

I'm not sure if 946cf57 is the right way to fix this problem. If so, I would expect someone from the community will encounter this edge case already and fit it. More likely it's a bug in SYCL project CMake files which do not set all dependencies properly (e.g. use Google Test libraries in link command w/o setting dependency on the project building them).
@vladimirlaz, could you check, please?

[vladimirlaz]

check-sycl should only depend on sycl-toolchain. Currently building those two targets from a fresh checkout will result in linker errors:

/usr/bin/ld: cannot find -lgtest_main
/usr/bin/ld: cannot find -lgtest
/usr/bin/ld: cannot find -lLLVMTestingSupport                                      
clang-13: error: linker command failed with exit code 1 (use -v to see invocation) 

Admittedly, this is somewhat an edge case, as the problem goes away when all targets are build prior to running check-sycl.
This patch ensures that gtest and gtest_main are known to cmake before it parses projects, so the dependencies can be correctly tracked.

I'm not sure if 946cf57 is the right way to fix this problem. If so, I would expect someone from the community will encounter this edge case already and fit it. More likely it's a bug in SYCL project CMake files which do not set all dependencies properly (e.g. use Google Test libraries in link command w/o setting dependency on the project building them).
@vladimirlaz, could you check, please?

As far as see we add gtest, gtest_main, LLVMSupport and LLVMTestingSupport libraries to the linkage of tests in SYCLUnitTests target (https://github.com/intel/llvm/blob/sycl/sycl/cmake/modules/AddSYCLUnitTest.cmake#L98).
But there is no target dependencies properly configured. So if google test package is installed in the system or corresponding targets are prebuilt. In our CI we build check-llvm which does google test and LLVM test infra build before we run check-sycl. I suggest to add corresponding target dependencies to functions defined in https://github.com/intel/llvm/blob/sycl/sycl/cmake/modules/AddSYCLUnitTest.cmake.

[bader]

But there is no target dependencies properly configured.

I don't think this is the case. We add those libraries as dependencies in add_sycl_executable macro - https://github.com/intel/llvm/blob/sycl/sycl/cmake/modules/AddSYCLExecutable.cmake#L38-L42. But dependencies are added only if there is corresponding CMake target already. That's why 946cf57 helps here.
@jchlanda, if you would like to proceed with 946cf57, I suggest we commit it to llorg directly.
I personally I think there should be no order in which cmake sees the targets and it's better to be handled in SYCL project cmake files.

[jchlanda]

Took 946cf57 to the mainline llvm (https://reviews.llvm.org/D104848) and cdde151 is superficial, closing.

[bader]

Thanks! We pull llorg commits once a week, so once https://reviews.llvm.org/D104848 is committed we can expect it to appear in this repository within a week.