[PATCH] D68833: [CMake] Re-order runtimes in the order of dependencies

Fri Oct 11 11:19:32 PDT 2019

beanz added a comment.

In D68833#1706104 <https://reviews.llvm.org/D68833#1706104>, @ldionne wrote:

> CMake tracks dependencies between targets, but not between directories. If the CMakeLists.txt in some directory (e.g. `<root>/libcxx/`) needs a target defined in another directory (e.g. `<root>/libcxxabi/`), one has to make sure that `libcxxabi`'s `CMakeLists.txt` is included before `libcxx`'s `CMakeLists.txt`. This isn't new or vexing, IMO. If that is what this patch ensures (I don't know the runtimes build very well), I think this is good.

The problem is that libcxx depends on compiler-rt, and compiler-rt depends on libcxx. Which means we have circular dependencies. The solution is to migrate to CMake usage patterns that don't require strict ordering. Specifically the use of generator expressions in this (and many other situations) is warranted.

> I'd like to question that affirmation. What is it based on?

The fact that we have circular dependencies at the project level (not the target level). The `if (TARGET ...)` feature in CMake requires that targets are processed in specific orders and cannot deal with projects that have circular dependency relationships.

> IMO, the `HAVE_${runtime}` variables are the weird ones here. The normal LLVM monorepo build orders the directories correctly, and we don't run into that issue.

I'm not sure what you mean by this. What I *think* you are referring to is specifying runtime libraries in `LLVM_ENABLE_PROJECTS` which uses the llvm/projects subdirectory for building them. This build flow is also part of the monorepo and uses `LLVM_ENABLE_RUNTIMES`. Fundamentally `LLVM_ENABLE_PROJECTS` is the incorrect way to build runtime libraries because they are not built with the in-tree compiler. While this works for most development cases it is 100% wrong for building and shipping toolchains, and it is questionable practice to have developers building and testing things that aren't representative of what we ship.

> I don't see how `HAVE_${runtime}` can get around things like being able to query properties of e.g. `cxxabi_shared` inside the libcxx build before `cxxabi_shared` has been defined. I do support a push towards using generator expressions more, but I don't think generator expressions are a complete solution to this problem.

What do you think that generator expressions can't do that is relevant to this problem?

> I'd like to see this patch go in under some form so that we can remove the hacky workaround introduced in https://reviews.llvm.org/D68791.

You can also remove that hacky workaround using generator expressions. Replacing one hack with another isn't exactly a meaningful transformation of the codebase.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68833