[llvm] [LLVM] Add `LLVM_<proj>_RUNTIME_TARGETS` to set targets per-project (PR #81557)

Joseph Huber via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 20 11:51:22 PST 2024


jhuber6 wrote:

> I looked at all `"libc" IN_LISTS LLVM_ENABLE_RUNTIMES` cases but neither of those seemed like blockers and could be worked around by setting `LIBC_HDRGEN_ONLY=ON` and `LLVM_ENABLE_PROJECTS=libc` which IMHO is the correct solution rather than complicating the existing logic by iterating over all variables to check for `"libc" IN_LISTS RUNTIMES_<target>_LLVM_ENABLE_RUNTIMES` for every `<target>` in `LLVM_RUNTIME_TARGETS`.
>

Personally, I don't like splitting up the logic required to check if a runtime is enabled.

> I'm still not especially enthused about this feature, it adds additional complexity to the runtimes build and from what I can tell so far, you can achieve the same result with the existing implementation, and although that might require setting more options, I'm not sure it warrants the additional complexity. If there some specific things you believe aren't possible, it'd be great to provide an example so I can see if we could support those with the existing features.
>

Fundamentally, this comes down to the question of whether or not `runtime=targets` or `target=runtimes` is the more natural way to set these things. My perspective is that both have cases where they are the most convenient way to get the idea across. You can write both in terms of the other with enough effort and some fixes, but this seems to be coming down to the former being easier for my use-case and the latter being easier for your use-case. I personally would like to support both as they are the easiest solution in different cases.

> If we decide to go forward with change regardless, I'd like you to setup a builder that exercises it, ideally both presubmit and postsubmit. The problem we already have is that runtimes build has too many ways to spell the same thing which is making it frustratingly difficult to make changes. For example, in case of libc I have to verify that `LLVM_ENABLE_PROJECTS=libc`, `LLVM_ENABLE_RUNTIMES=libc` and `RUNTIMES_<target>_LLVM_ENABLE_RUNTIMES=libc` still works; with this change there's yet another variation and I don't have resources to try every one of them every time I make some change to the build and I'd like to avoid a situation where I make a change and then hear back some time later that I broke a downstream user.

We will have an AMDGPU and NVPTX builder that will use it at a minimum. I think there was some talk with @jplehr to have some AMD presubmit support, but I don't think that's set up yet. This patch also somewhat simplifies the complexity here by explicitly setting up a list of runtimes per target I believe.
 
> I'd also be curious to hear from @smeenai and @llvm-beanz as other reviewers of the runtimes build changes if they have other thoughts.

https://github.com/llvm/llvm-project/pull/81557


More information about the llvm-commits mailing list