[PATCH] D99621: [CMake][Compiler-rt] Make it possible to configure standalone compiler-rt without `LLVMConfig.cmake`.

[Petr Hosek via Phabricator via llvm-commits]

phosek added a comment.

In D99621#2667774 <https://reviews.llvm.org/D99621#2667774>, @delcypher wrote:

> In D99621#2667768 <https://reviews.llvm.org/D99621#2667768>, @mstorsjo wrote:
>
>> Thanks, this seems to work fine in my setups now!
>>
>> When building via llvm/runtimes (which I don't use so I don't have a test setup handy), I believe that sets `TARGET_TRIPLE` somehow. Does that end up trying to probe the triple from the compiler in that case? I guess it'd always be correct, but if we'd avoid the whole probing in that case too, that'd be great. Maybe it already does that - or the runtimes build does thing differently somehow? (I've just tried to read llvm/runtimes/CMakeLists.txt.) Or maybe we end up in a case where these routines aren't called at all in such a build?
>>
>> I might be able to try to set up such a build configuration in a couple days to try that out though...
>
> I'm not sure. Reading that code is a little confusing...
>
> So there are external project invocations that pass
>
> 1. `LLVM_DEFAULT_TARGET_TRIPLE`. That looks like it's only used in compiler-rt to help with finding GNU gold/ld. So that probably doesn't matter here.
> 2. `TARGET_TRIPLE` as an argument to the `llvm_ExternalProject_Add()` function
>
> The second one is a bit confusing.
>
> There's this code inside `llvm_ExternalProject_Add`
>
>   if(NOT ARG_TARGET_TRIPLE)
>     set(target_triple ${LLVM_DEFAULT_TARGET_TRIPLE})
>   else()
>     set(target_triple ${ARG_TARGET_TRIPLE})
>   endif()
>   
>   is_msvc_triple(is_msvc_target ${target_triple})
>
> but then `target_triple` doesn't get used again... which makes no sense. (@phosek @beanz is this a bug?).

This is a recent addition, but if I read the correctly, `is_msvc_triple` sets `is_msvc_target` based on the value of `target_triple`. I think the fact that the output variable is first is confusing.

> Then later on I do see
>
>   if(ARG_TARGET_TRIPLE)
>     list(APPEND compiler_args -DCMAKE_C_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
>     list(APPEND compiler_args -DCMAKE_CXX_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
>     list(APPEND compiler_args -DCMAKE_ASM_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
>   endif()
>
> So that does suggest Compiler-RT will have `CMAKE_C_COMPILER_TARGET` passed. The builtins and other runtime builds in `llvm-project/llvm/runtimes` look like they pass `-DCOMPILER_RT_DEFAULT_TARGET_ONLY=ON`.
>
> So that would suggest that both `CMAKE_C_COMPILER_TARGET` and `COMPILER_RT_DEFAULT_TARGET_ONLY` get set.
>
> **If I've read this correctly** that means this patch doesn't need to change because it already checks for `CMAKE_C_COMPILER_TARGET` and `COMPILER_RT_DEFAULT_TARGET_ONLY` being set and defers to using `CMAKE_C_COMPILER_TARGET` to set `TARGET_TRIPLE` in that case.
>
> I don't use the `llvm-project/llvm/runtimes` directory either so I don't know if it works on Apple platforms. I'd be a bit happier if someone who does use it checked that this patch didn't break anything.

Your understanding is correct. The only case where this isn't true and we rely on automatic detection even in the `llvm-project/llvm/runtimes` is on Apple platforms because the compiler-rt CMake setup works differently there due to universal binary support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99621