[libc-commits] [PATCH] D137451: [CMake] Use LLVM_RUNTIME_TARGET in runtimes

Wed Sep 27 10:41:36 PDT 2023

phosek added a comment.

In D137451#4076376 <https://reviews.llvm.org/D137451#4076376>, @mstorsjo wrote:

> In D137451#3959515 <https://reviews.llvm.org/D137451#3959515>, @phosek wrote:
>
>> In D137451#3957333 <https://reviews.llvm.org/D137451#3957333>, @daltenty wrote:
>>
>>> This patch seem to do a bit more than it's description implies. It makes `LLVM_DEFAULT_TARGET_TRIPLE` no longer an valid option on the `runtimes` path:
>>>
>>>   CMake Warning:
>>>     Manually-specified variables were not used by the project:
>>>   
>>>       LLVM_DEFAULT_TARGET_TRIPLE
>>>
>>> Is this intended? Currently `LLVM_DEFAULT_TARGET_TRIPLE` is the only option we actually document for changing the target triple (https://llvm.org/docs/CMake.html), so now the support options seem kind of inconsistent (though I don't fully grok the intended distinction between  `LLVM_DEFAULT_TARGET_TRIPLE` and `LLVM_TARGET_TRIPLE`, which may be part of the problem).
>>
>> `LLVM_DEFAULT_TARGET_TRIPLE` is the default value that's used by tools like `clang` or `llc` when you don't explicitly specify `--target` or `--triple`. `LLVM_TARGET_TRIPLE` is the target triple we're building for. `LLVM_DEFAULT_TARGET_TRIPLE` is used as a the default value for `LLVM_TARGET_TRIPLE` but they're independent.
>>
>> I couldn't come up with a use case for `LLVM_DEFAULT_TARGET_TRIPLE` in the runtimes build which is why I dropped it, but I'm open to revisiting that if you have other suggestions. I can also update the documentation to make sure this difference is covered.
>
> I think the point that @daltenty raises, is that if you're currently building the runtimes (by manually invoking cmake on the `llvm-project/runtimes` directory), you might be setting `LLVM_DEFAULT_TARGET_TRIPLE` which previously worked for setting it, but after this patch no longer works. My branch for llvm-mingw to make it use the per-target directory does exactly that: https://github.com/mstorsjo/llvm-mingw/commit/2cf53146bb94e0cd56b36b4c71ef09753143389f
>
> I tried setting `LLVM_TARGET_TRIPLE` instead of `LLVM_DEFAULT_TARGET_TRIPLE` here, but that doesn't seem to work - with current git main, the only way of setting the triple for the purposes of the per-target runtime directories, is to set `LLVM_DEFAULT_TARGET_TRIPLE`. So in that respect, it's a public interface, and it'd at least be courteous to keep the old one working for some transition time while switching to a different way of setting it.
>
> Curiously, for compiler-rt, it seems to infer it from `CMAKE_C_COMPILER_TARGET`, while libcxx/libunwind/libcxxabi seem to require it to be set via `LLVM_DEFAULT_TARGET_TRIPLE`.

This should be addressed in the latest version. I plan on landing this change today unless there are any concerns as a prerequisite for D140925 <https://reviews.llvm.org/D140925>.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137451