[PATCH] D104848: [cmake] Handled utils/unittests before projects

[Chris Bieneman via Phabricator via llvm-commits]

beanz added a comment.

In D104848#2912827 <https://reviews.llvm.org/D104848#2912827>, @jchlanda wrote:

> SYCL already predicates building unittests on the value of `LLVM_INCLUDE_TESTS` (https://github.com/intel/llvm/blob/sycl/sycl/CMakeLists.txt#L212).

This is actually not safe. You're creating a new cached value based on the value of another cached value. Both can be set and updated independently of each other. You should (at least) add a hard error if SYCL_INCLUDE_TESTS=On and LLVM_INCLUDE_TESTS=Off.

> The problem is that at the point of time when `check-sycl` is built `gtests` are not known to CMake (even though SYCL code did check for the existence of the source), so it is impossible to add them to the list of dependencies. Yes they will be built (if the `LLVM_INCLUDE_TESTS` is set on), but the dependency will not be set up correctly and depending on the order in which the targets are built `check-sycl` might fail to link. It boils down to inter-target dependencies in CMake.

I don't at all understand what you're talking about here. In CMake it is totally valid to list a dependency before the target is defined. Meaning:

  add_library(foo ...)
  target_link_libraries(foo PRIVATE bar)
  ...
  add_library(bar ...)

Foo will link bar, the target, not -lbar.

What you can't do without ordering issues is:

  add_library(foo ...)
  if(TARGET bar)
    target_link_libraries(foo PRIVATE bar)
  endif()
  ...
  add_library(bar ...)

SYCL should be able to do something like:

  if(LLVM_INCLUDE_TESTS)
    target_link_libraries(foo PRIVATE gtest)
  endif()

Why doesn't that work?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104848/new/

https://reviews.llvm.org/D104848