[llvm][CMake] Introduce LLVM_RUNTIME_<project>_BUILD in CMake

[mjklemm]

This PR adds CMake code to set LLVM_RUNTIME_<project>_BUILD similar to LLVM_TOOL_<project>_BUILD. This makes it easier to detect if a particular runtime build has been requested.

[mjklemm]

@petrhosek @delcypher Would have a bit of time to have a look at this PR? I would greatly appreciate your review.

[jhuber6]

So, is this adding a variable that's used to detect if a project has "any" runtime enabled for it? I know for detecting libc stuff it's kind of a mess because we need to check LLVM_ENABLE_PROJECTS and RUNTIMES_<triple>_LLVM_ENABLE_PROJECTS.

[petrhosek]

In LLVM, we have runtimes and bootstrapping builds.

The runtimes build uses https://github.com/llvm/llvm-project/tree/main/runtimes to build runtimes specified by LLVM_ENABLE_RUNTIMES using whatever host compiler you specify by CMAKE_{C,CXX}_COMPILER.

The bootstrapping build uses https://github.com/llvm/llvm-project/tree/main/llvm/runtimes to first build a toolchain with projects specified by LLVM_ENABLE_PROJECTS, and then uses that just built toolchain to build selected runtimes using the runtimes build.

This patch only sets LLVM_RUNTIME_<project>_BUILD in the bootstrapping build which is not the right place, it should be set in the runtimes build (that is https://github.com/llvm/llvm-project/blob/main/runtimes/CMakeLists.txt) to make sure that the variable is set whether we're using runtimes or bootstrapping build.

[mjklemm]

So, is this adding a variable that's used to detect if a project has "any" runtime enabled for it? I know for detecting libc stuff it's kind of a mess because we need to check LLVM_ENABLE_PROJECTS and RUNTIMES_<triple>_LLVM_ENABLE_PROJECTS.

I do hope that this will simply this. For the OpenMP tests, one would also have to explicitly check the presence of "openmp" in LLVM_ENABLE_PROJECTS. After this, this will hopefully be not needed anymore. If you could have a look and see if this PR helps there, I'd appreciate that.

[mjklemm]

Thanks for taking the time to review and the explanation about the mechanics!

This patch only sets LLVM_RUNTIME_<project>_BUILD in the bootstrapping build which is not the right place, it should be set in the runtimes build (that is https://github.com/llvm/llvm-project/blob/main/runtimes/CMakeLists.txt) to make sure that the variable is set whether we're using runtimes or bootstrapping build.

I have moved the code to runtimes/CMakeLists.txt now. Does that look better now?

[jhuber6]

Don't we need to check for each LLVM_RUNTIME_TARGETS if RUNTIMES_<triple>_LLVM_ENABLE_RUNTIMES is set?

[mjklemm]

Good question. I'm not qualified to answer it, though.

[petrhosek]

I have moved the code to runtimes/CMakeLists.txt now. Does that look better now?

It should be moved to runtimes/CMakeLists.txt, not llvm/runtimes/CMakeLists.txt.

[mjklemm]

It should be moved to runtimes/CMakeLists.txt, not llvm/runtimes/CMakeLists.txt.

Hm. When I do this, it does not seem that it is picked up and when adding a debugging message, CMake does not seem to go through runtimes/CMakeList.txt, but llvm/runtimes/CMakeLists.txt for a configuration like this:

cmake -DLLVM_ENABLE_PROJECTS="clang;flang;lld" -DLLVM_ENABLE_RUNTIMES="openmp" [...] ../llvm

So, if I correctly understand your explanation above, then I'm doing a bootstrapping build with the above command line. I have to use a command line like this to trigger the code in runtimes/CMakefile.txt:

cmake -DLLVM_ENABLE_RUNTIMES="openmp" [...] ../runtimes

So, a bootstrapping build simply configures the request to build a runtime via llvm/runtimes/CMakeLists.txt and then, when actually building, invokes CMake with a runtime build (using a command line like the 2nd one and C/C++ compiler set to Clang), which in turn triggers runtime/CMakeLists.txt? Is this thinking remotely correct?

Because then it would seem like code would need to be duplicated (or be moved to a CMake file that can be included in both llvm/runtimes and runtimes). Because only that would allow a bootstrapping build to detect that a particular has been requested to, say, enable a bunch of LIT tests. Or, alternatively, even have LLVM_RUNTIME_<project>_BUILD and LLVM_BOOTSTRAP_<project>_BUILD to distinguish the two variants of these builds to enable other parts of the CMake files to detect either one.

[mjklemm]

Or, alternatively, even have LLVM_RUNTIME_<project>_BUILD and LLVM_BOOTSTRAP_<project>_BUILD to distinguish the two variants of these builds to enable other parts of the CMake files to detect either one.

I have implemented thing that way now. @petrhosek Please have a look and let me know that's an acceptable compromise to be as flexible as possible in detecting how builds are being configured.

[petrhosek]

It should be moved to runtimes/CMakeLists.txt, not llvm/runtimes/CMakeLists.txt.

Hm. When I do this, it does not seem that it is picked up and when adding a debugging message, CMake does not seem to go through runtimes/CMakeList.txt, but llvm/runtimes/CMakeLists.txt for a configuration like this:

cmake -DLLVM_ENABLE_PROJECTS="clang;flang;lld" -DLLVM_ENABLE_RUNTIMES="openmp" [...] ../llvm

So, if I correctly understand your explanation above, then I'm doing a bootstrapping build with the above command line. I have to use a command line like this to trigger the code in runtimes/CMakefile.txt:

cmake -DLLVM_ENABLE_RUNTIMES="openmp" [...] ../runtimes

So, a bootstrapping build simply configures the request to build a runtime via llvm/runtimes/CMakeLists.txt and then, when actually building, invokes CMake with a runtime build (using a command line like the 2nd one and C/C++ compiler set to Clang), which in turn triggers runtime/CMakeLists.txt? Is this thinking remotely correct?

Yes, your understanding is correct.

Because then it would seem like code would need to be duplicated (or be moved to a CMake file that can be included in both llvm/runtimes and runtimes). Because only that would allow a bootstrapping build to detect that a particular has been requested to, say, enable a bunch of LIT tests. Or, alternatively, even have LLVM_RUNTIME_<project>_BUILD and LLVM_BOOTSTRAP_<project>_BUILD to distinguish the two variants of these builds to enable other parts of the CMake files to detect either one.

No, I don't think this is desirable. The reason is that the bootstrapping build also supports more advanced scenarios such as the following:

cmake -DLLVM_ENABLE_PROJECTS="clang;flang;lld" -DLLVM_RUNTIME_TARGETS="x86_64-linux-gnu;aarch64-linux-gnu" -DRUNTIMES_x86_64-linux-gnu_LLVM_ENABLE_RUNTIMES="libc" -DRUNTIMES_aarch64-linux-gnu_LLVM_ENABLE_RUNTIMES="openmp" [...] ../llvm

This would invoke the runtimes build once for requested target, so simply setting LLVM_BOOTSTRAP_openmp_BUILD=ON may be misleading; if your host platform is x86_64-linux-gnu, you still wouldn't want to run tests because although openmp was built, it was built for a different target.

Can you explain what the motivation behind introducing these variables is (and also ideally include it in the description of the PR)? I can try to think a better way we could address your needs but I need to understand the use case.

[mjklemm]

Can you explain what the motivation behind introducing these variables is (and also ideally include it in the description of the PR)? I can try to think a better way we could address your needs but I need to understand the use case.

Sure, one use case is to determine if, say, the OpenMP runtime was built and enable the tests for it. Right now flang/test/lit.site.cfg.py.in has this:

config.have_openmp_rtl = ("@LLVM_TOOL_OPENMP_BUILD@" == "TRUE")

However, there's no good way to detect and enable that test configuration if OpenMP is built via LLVM_ENABLE_RUNTIMES. There's a clumsy way doing it by looking at LLVM_ENABLE_RUNTIMES using Python string search, but does not seem like a good solution either, So, I was hoping for something along the lines of LLVM_TOOL_<project>_BUILD.

[ldionne]

I'm not sure about this approach.

It seems to me that you should instead have a parameter you pass to lit with --param and then have it set from the CI cache you use to configure your build. That would prevent having to infer whether a project is being built or not.

The reason why I'm lukewarm about this change is that it adds yet another procedural aspect to our CMake builds, and we've been working hard to get rid of that. We want to move towards more declarative (and more modern) CMake constructs where we define targets and avoid global state and logic, and this basically introduces another way to add such logic. So while I don't 100% understand your use case and the constraints you have to solve it, this specific approach seems like the wrong one to me at a high level.

[mjklemm]

Closing this PR to research for a better way.