Page MenuHomePhabricator

jchlanda (Jakub Chlanda)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 23 2021, 6:39 AM (13 w, 3 d)

Recent Activity

Jul 30 2021

jchlanda abandoned D104848: [cmake] Handled utils/unittests before projects.

add_dependencies actually should work fine, regardless of the ordering. The problem is you're checking for the existence of the target.

Stop doing that.

Jul 30 2021, 6:57 AM · Restricted Project

Jul 29 2021

jchlanda added a comment to D104848: [cmake] Handled utils/unittests before projects.

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.

Jul 29 2021, 9:31 AM · Restricted Project
jchlanda added a comment to D104848: [cmake] Handled utils/unittests before projects.

SYCL already predicates building unittests on the value of LLVM_INCLUDE_TESTS (https://github.com/intel/llvm/blob/sycl/sycl/CMakeLists.txt#L212). 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.

Jul 29 2021, 1:53 AM · Restricted Project

Jul 28 2021

jchlanda added a comment to D104848: [cmake] Handled utils/unittests before projects.

What are you trying to do in a project that is motivating this?

I don’t have any real objection to this change, but the motivation concerns me. Generally we avoid things that require order-dependence, and setting up target dependencies does not.

Jul 28 2021, 8:09 AM · Restricted Project

Jul 20 2021

jchlanda added a comment to D104848: [cmake] Handled utils/unittests before projects.

ping

Jul 20 2021, 7:57 AM · Restricted Project
jchlanda updated subscribers of D104848: [cmake] Handled utils/unittests before projects.
Jul 20 2021, 7:57 AM · Restricted Project

Jun 24 2021

jchlanda requested review of D104848: [cmake] Handled utils/unittests before projects.
Jun 24 2021, 5:03 AM · Restricted Project