[llvm-dev] Should the compiler-rt builtins be configured with CMAKE_TRY_COMPILE_TARGET_TYPE?

[Shoaib Meenai via llvm-dev]

https://reviews.llvm.org/D96404 landed recently to default Android targets to using compiler-rt instead of libgcc. Amusingly enough, it broke our builds of compiler-rt for Android in the runtimes build setup. What’s happening is that the configuration for the builtins for Android tests for all supported architectures [1], using check_symbol_exists to check for the architecture’s preprocessor macro [2]. check_symbol_exists tries to link a test executable, which fails because my just-built compiler doesn’t have compiler-rt yet (since it’s trying to build compiler-rt). Consequently, the builtins don’t think that any architectures are supported, and just don’t build anything as a result.

 

Other places in the builtins’ CMake logic use custom functions like try_compile_only to only test compilation and not linking, to avoid issues like this. My understanding is that the builtins will never be built as shared libraries. The build already instructs its custom test macros to always only check compilation [3]; now that we’re on a newer CMake version, would it be appropriate to always set CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY for the builtins as well, so that all CMake checks build static libraries (which has the same effect of only testing compilation and not linking)? We could also probably clean up a lot of the custom logic for only checking compilation afterwards.

 

[1] https://github.com/llvm/llvm-project/blob/c7712087cbb505d324e1149fa224f607c91a8c6a/compiler-rt/cmake/base-config-ix.cmake#L164-L167

[2] https://github.com/llvm/llvm-project/blob/c7712087cbb505d324e1149fa224f607c91a8c6a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake#L155-L170

[3] https://github.com/llvm/llvm-project/blob/c7712087cbb505d324e1149fa224f607c91a8c6a/compiler-rt/cmake/builtin-config-ix.cmake#L4-L5

 

[Martin Storsjö via llvm-dev]

Hi,

On Thu, 11 Mar 2021, Shoaib Meenai via llvm-dev wrote:

> What’s happening is that the configuration for the builtins for Android
> tests for all supported architectures [1], using check_symbol_exists to
> check for the architecture’s preprocessor macro [2]. check_symbol_exists
> tries to link a test executable, which fails because my just-built
> compiler doesn’t have compiler-rt yet (since it’s trying to build
> compiler-rt). Consequently, the builtins don’t think that any
> architectures are supported, and just don’t build anything as a result.
>
>  
>
> Other places in the builtins’ CMake logic use custom functions like
> try_compile_only to only test compilation and not linking, to avoid issues
> like this. My understanding is that the builtins will never be built as
> shared libraries. The build already instructs its custom test macros to
> always only check compilation [3]; now that we’re on a newer CMake version,
> would it be appropriate to always set CMAKE_TRY_COMPILE_TARGET_TYPE to
> STATIC_LIBRARY for the builtins as well, so that all CMake checks build
> static libraries (which has the same effect of only testing compilation and
> not linking)? We could also probably clean up a lot of the custom logic for
> only checking compilation afterwards.

These kinds of chicken-and-egg problems are all over building the
runtimes, and when building the initial copy of runtime libraries, there's
some amount of trickery needed to pass such tests.

In most of the cases where I build runtimes, I've had to add
-DCMAKE_C_COMPILER_WORKS=TRUE -DCMAKE_CXX_COMPILER_WORKS=TRUE, but iirc
one of such cases for compiler-rt could be removed if we'd set
CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY. (For
libunwind/libcxxabi/libcxx, things were more complicated though, I think.)

I actually sent a patch to this effect a couple months ago, have a look at
[1]. That one was only aimed at standalone builds of
compiler-rt/lib/builtins, but maybe it could be generalized to some other
location as well. For non-standalone builds, I guess the option would have
to be cleared after processing the current project?

// Martin

[1] https://reviews.llvm.org/D91334

[Petr Hosek via llvm-dev]

I was actually wondering if we could go further and set this not just for builtins but also for the runtimes build. In the runtimes build, we link shared libraries like libunwind, libc++abi, libc++ and some of the sanitizers, but it's not clear to me if this would cause any issues. We have a few check_library_exists calls but most of them are checking libraries like libc, libm, libdl, librt? Even C libraries that don't provide these as separate libraries like musl typically provide linker scripts for backwards compatibility. Would any of the currently supported platforms break if we started linking those libraries unconditionally? Alternatively, we could also consider replacing the use of -nodefaultlibs with -nostdlib++ and letting the compiler driver handle this, but GCC as far as I'm aware doesn't yet support that flag so we would need a fallback (we could set CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY and -nostdlib++ only for Clang, and keeping the existing behavior for GCC, but it might increase the complexity).

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Martin Storsjö via llvm-dev]

On Wed, 10 Mar 2021, Petr Hosek wrote:

> I was actually wondering if we could go further and set this not just for
> builtins but also for the runtimes build. In the runtimes build, we link
> shared libraries like libunwind, libc++abi, libc++ and some of the
> sanitizers, but it's not clear to me if this would cause any issues. We have
> a few check_library_exists calls but most of them are checking libraries
> like libc, libm, libdl, librt? Even C libraries that don't provide these as
> separate libraries like musl typically provide linker scripts for backwards
> compatibility. Would any of the currently supported platforms break if we
> started linking those libraries unconditionally? Alternatively, we could
> also consider replacing the use of -nodefaultlibs with -nostdlib++ and
> letting the compiler driver handle this, but GCC as far as I'm aware doesn't
> yet support that flag so we would need a fallback (we could set
> CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY and -nostdlib++ only for
> Clang, and keeping the existing behavior for GCC, but it might increase the
> complexity).

I remember testing setting CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY
for libunwind/libcxxabi/libcxx a couple months ago so that I could lose
the hardcoded CMAKE_C_COMPILER_WORKS=TRUE and
CMAKE_CXX_COMPILER_WORKS=TRUE, but it didn't work flawlessly. I don't
remember the exact details, but I think it was in the form of lots of
tests succeeding while they shouldn't.

As for libc/libm/librt/libdl, windows has none of those.

Overall regarding -nostdlib++, that one generally is much neater than
-nodefaultlibs, but e.g. if running in a setup with -unwindlib=libunwind,
that still tries to link in libunwind, which may be missing at that point.

// Martin

[Cág via llvm-dev]

Shoaib Meenai wrote:
> https://reviews.llvm.org/D96404 landed recently to default Android
> targets to using compiler-rt instead of libgcc. Amusingly enough, it
> broke our builds of compiler-rt for Android in the runtimes build
> setup. What’s happening is that the configuration for the builtins
> for Android tests for all supported architectures [1], using
> check_symbol_exists to check for the architecture’s preprocessor
> macro [2]. check_symbol_exists tries to link a test executable,
> which fails because my just-built compiler doesn’t have compiler-rt
> yet (since it’s trying to build compiler-rt). Consequently,
> the builtins don’t think that any architectures are supported, and
> just don’t build anything as a result.

Hi all,

I've raised the topic back in November[0], and this what still works
for me (thanks to the suggestions by Martin and Chris):
1. Get the sources.
2. Build clang, llvm, lld.
3. Install libc headers to a sysroot.
4. Build compiler-rt builtins and crt with the freshly-built clang.
One need to set C_COMPILER_WORKS to skip the checks.
5. Build libc.a/libc.so

Then the script builds libc++, libc++abi, libunwind, and it results in a
working C/C++ cross-compiler. The libc is musl, but I think this works
with any libc.

compiler-rt C_COMPILER_WORKS checks are unnecessary because it doesn't
need itself (or any runtimes) and libc to be built.

[0]: https://lists.llvm.org/pipermail/llvm-dev/2020-November/146450.html

--
caóc

[Petr Hosek via llvm-dev]

I've been thinking about this a bit more and doing some experiments. I'm no longer sure if setting CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY is the best solution. While it could simplify checks in projects that only ever produce static libraries like builtins or crt, using it more broadly might be a problem because it breaks checks like check_function_exists and check_library_exists.

The idea I'm considering would be to instead roll out our own checks. The way checks like check_function_exists and check_library_exists work is that CMake generates a little snippet which it then tries to compile. For example when checking if dlopen in libdl exists, CMake would generate:

char dlopen(void);

int main(int ac, char* av[]) {
  dlopen();
}

It'd then try to compile this file with -ldl and see if the command succeeds or fails. The problem with this snippet is that it implicitly uses libc and other libraries like crt or builtins, which in the case of the runtimes build, are libraries we're going to build introducing a cycle. The alternative I came up with would be to generate a custom snippet that looks like this:

char dlopen(void);

void _start() {
  dlopen();
}

When invoking the compiler, we would additionally pass -nostdlib. That way, we would still check if libdl is available without any implicit dependencies. The only downside to this approach I can see is that we would need to maintain our own set of checks, but since we could rely on CMake's try_compile to do most of the heavy lifting, I don't think it'd be too much of a maintenance overhead.

If we switched to these custom checks across all runtimes, I hope that we could eliminate the need to bootstrap builtins before we build the rest of runtimes, and instead we could use a single CMake build for all runtimes and rely on regular dependencies for things like builtins.

Does this make sense? Have I missed anything?

[Martin Storsjö via llvm-dev]

Hi,

On Mon, 15 Mar 2021, Petr Hosek wrote:

> I've been thinking about this a bit more and doing some experiments. I'm
> no longer sure if setting CMAKE_TRY_COMPILE_TARGET_TYPE to STATIC_LIBRARY is
> the best solution.

Yes, it's a bit problematic in some cases too... But for passing the
initial CMake compiler sanity check, it's either that, or passing
CMAKE_CXX_COMPILER_WORKS.

> While it could simplify checks in projects that only ever produce static
> libraries like builtins or crt, using it more broadly might be a problem
> because it breaks checks like check_function_exists and
> check_library_exists.

Yeah it can't be used straight as-is in other projects

> The idea I'm considering would be to instead roll out our own checks. The
> way checks like check_function_exists and check_library_exists work is that
> CMake generates a little snippet which it then tries to compile. For example
> when checking if dlopen in libdl exists, CMake would generate:
>
> char dlopen(void);
> int main(int ac, char* av[]) {
>   dlopen();
> }
>
> It'd then try to compile this file with -ldl and see if the command succeeds
> or fails. The problem with this snippet is that it implicitly uses libc and
> other libraries like crt or builtins, which in the case of the runtimes
> build, are libraries we're going to build introducing a cycle. The
> alternative I came up with would be to generate a custom snippet that looks
> like this:
>
> char dlopen(void);
> void _start() {
>   dlopen();
> }
>
> When invoking the compiler, we would additionally pass -nostdlib. That way,
> we would still check if libdl is available without any implicit
> dependencies. The only downside to this approach I can see is that we would
> need to maintain our own set of checks, but since we could rely on CMake's
> try_compile to do most of the heavy lifting, I don't think it'd be too much
> of a maintenance overhead.

That sounds workable to me (at least on first thought, without trying it
out), but it'd require a bit of platform specifics, as the entry point
function name differs a bit between platforms (on Windows, it's
"mainCRTStartup", not "_start").

// Martin