HomePhabricator

[libc++abi] Simplify the logic for finding libc++ from libc++abi

Authored by ldionne on Jun 11 2020, 10:50 AM.

Description

[libc++abi] Simplify the logic for finding libc++ from libc++abi

Since we have the monorepo, libc++abi's build requires a sibling checkout
of the libc++ sources. Hence, the logic for finding libc++ can be greatly
simplified.

Details

Committed
ldionneJun 11 2020, 12:08 PM
Parents
rG43101d10dbd5: [OPENMP50]Codegen for scan directive in simd loops.
Branches
Unknown
Tags
Unknown

Event Timeline

sbc100 added a subscriber: sbc100.Jun 13 2020, 9:07 PM

This change seems to have broken the wasm waterfall builder: https://logs.chromium.org/logs/wasm/buildbucket/cr-buildbucket.appspot.com/8877568110673877104/+/steps/libcxxabi/0/stdout

We set LIBCXXABI_LIBCXX_INCLUDES to point to the installed version of libcxx headers and it looks like -DLIBCXXABI_LIBCXX_INCLUDES no longer has any effect.

Its important for use to use the installed version of the headers specifically because of the include/c++/v1/__config header. The __config header that lives in the source tree at libcxx/include/__config doesn't have any of the site config changes.

This change seems to have broken the wasm waterfall builder: https://logs.chromium.org/logs/wasm/buildbucket/cr-buildbucket.appspot.com/8877568110673877104/+/steps/libcxxabi/0/stdout

We set LIBCXXABI_LIBCXX_INCLUDES to point to the installed version of libcxx headers and it looks like -DLIBCXXABI_LIBCXX_INCLUDES no longer has any effect.

Its important for use to use the installed version of the headers specifically because of the include/c++/v1/__config header. The __config header that lives in the source tree at libcxx/include/__config doesn't have any of the site config changes.

Do you configure CMake with the LIBCXX_FOO options necessary to generate the correct __config_site header? The problem here stems from the fact that you're building libc++ and libc++abi separately -- would it be an option to build both in one go against each other?

I'd like to move away from being able to build libc++ and libc++abi against each other from two different CMake invocations, since that adds a lot of unnecessary complexity around setting up paths between both.

Sure we can try to re-configure our build system.

Both the wasm SDKs currently build libcxx and libcxxabi and compiler-rt as three different cmake invocations. I imagine anyone using the same technique will be broken by this change, right? Is this not a common pattern?

https://github.com/WebAssembly/wasi-sdk/blob/master/Makefile#L170

Is it enough to pass the LIBCXX options when configuring libcxxabi or do we have to also switch the combined build? What would combined build look like? Right now we run cmake passing in the libcxx directory and then run it again passing it the libcxxabi directory. Which directory should we pass it to build both? Is there and example you can point me too ?

Sure we can try to re-configure our build system.

Both the wasm SDKs currently build libcxx and libcxxabi and compiler-rt as three different cmake invocations. I imagine anyone using the same technique will be broken by this change, right? Is this not a common pattern?

Indeed, if they need to use custom libc++ headers.

https://github.com/WebAssembly/wasi-sdk/blob/master/Makefile#L170

Thanks for the link, that's really useful to see exactly what you're doing.

Is it enough to pass the LIBCXX options when configuring libcxxabi or do we have to also switch the combined build? What would combined build look like? Right now we run cmake passing in the libcxx directory and then run it again passing it the libcxxabi directory. Which directory should we pass it to build both? Is there and example you can point me too ?

So, ideally, you'd switch to a unified build. It would look like:

# Flags for libcxx and libcxxabi.
LIBCXX_LIBCXXABI_CMAKE_FLAGS = \
    -DUNIX:BOOL=ON \
    -DCXX_SUPPORTS_CXX11=ON \
    -DCMAKE_TOOLCHAIN_FILE=$(ROOT_DIR)/wasi-sdk.cmake \
    -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON \
    -DCMAKE_BUILD_TYPE=RelWithDebugInfo \
    -DCMAKE_CXX_COMPILER_WORKS=ON \
    -DCMAKE_C_COMPILER_WORKS=ON \
    -DWASI_SDK_PREFIX=$(PREFIX) \
    -DLLVM_COMPILER_CHECKED=ON \
    -DLLVM_CONFIG_PATH=$(ROOT_DIR)/build/llvm/bin/llvm-config \
    -DLIBCXX_ENABLE_THREADS:BOOL=OFF \
    -DLIBCXX_HAS_PTHREAD_API:BOOL=OFF \
    -DLIBCXX_HAS_EXTERNAL_THREAD_API:BOOL=OFF \
    -DLIBCXX_BUILD_EXTERNAL_THREAD_LIBRARY:BOOL=OFF \
    -DLIBCXX_HAS_WIN32_THREAD_API:BOOL=OFF \
    -DLIBCXX_ENABLE_SHARED:BOOL=OFF \
    -DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY:BOOL=OFF \
    -DLIBCXX_ENABLE_EXCEPTIONS:BOOL=OFF \
    -DLIBCXX_ENABLE_FILESYSTEM:BOOL=OFF \
    -DLIBCXX_CXX_ABI=libcxxabi \
    -DLIBCXX_CXX_ABI_INCLUDE_PATHS=$(LLVM_PROJ_DIR)/libcxxabi/include \
    -DLIBCXX_HAS_MUSL_LIBC:BOOL=ON \
    -DLIBCXX_ABI_VERSION=2 \
    -DLIBCXXABI_ENABLE_EXCEPTIONS:BOOL=OFF \
    -DLIBCXXABI_ENABLE_SHARED:BOOL=OFF \
    -DLIBCXXABI_SILENT_TERMINATE:BOOL=ON \
    -DLIBCXXABI_ENABLE_THREADS:BOOL=OFF \
    -DLIBCXXABI_HAS_PTHREAD_API:BOOL=OFF \
    -DLIBCXXABI_HAS_EXTERNAL_THREAD_API:BOOL=OFF \
    -DLIBCXXABI_BUILD_EXTERNAL_THREAD_LIBRARY:BOOL=OFF \
    -DLIBCXXABI_HAS_WIN32_THREAD_API:BOOL=OFF \
    $(if $(patsubst 8.%,,$(CLANG_VERSION)),-DLIBCXXABI_ENABLE_PIC:BOOL=OFF,) \
    --debug-trycompile

build/libcxx.BUILT: build/llvm.BUILT build/compiler-rt.BUILT build/wasi-libc.BUILT
    # Do the build.
    mkdir -p build/libcxx
    cd build/libcxx && cmake -G Ninja $(LIBCXX_LIBCXXABI_CMAKE_FLAGS) \
        -DCMAKE_C_FLAGS="$(DEBUG_PREFIX_MAP)" \
        -DCMAKE_CXX_FLAGS="$(DEBUG_PREFIX_MAP)" \
        -DLIBCXX_LIBDIR_SUFFIX=$(ESCAPE_SLASH)/wasm32-wasi \
        -DLIBCXXABI_LIBDIR_SUFFIX=$(ESCAPE_SLASH)/wasm32-wasi \
        -DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi" \
        $(LLVM_PROJ_DIR)/llvm
    ninja $(NINJA_FLAGS) -v -C build/libcxx
    # Do the install.
    ninja $(NINJA_FLAGS) -v -C build/libcxx install-cxx install-cxxabi
    touch build/libcxx.BUILT

Notice how we're now using <root>/llvm and LLVM_ENABLE_PROJECTS instead of "standalone builds". Now, the problem with this is that we're using <root>/llvm instead of just <root> (which doesn't have any CMakeLists.txt as of now), and we end up setting up all of LLVM, which is both kind of overkill and might not work for WASM. I do recognize this as a problem, and I already did before. However, I didn't realize that removing LIBCXXABI_LIBCXX_INCLUDES would have this effect, and I thought I had more time to fix the "build as part of LLVM" issue.

What I'll do is:

  1. Make a change that honours LIBCXXABI_LIBCXX_INCLUDES again to avoid breaking you and anyone that might be using the standalone build while needing a custom __config_site, and
  2. Start a discussion about how to handle building LLVM projects without having to build all of LLVM at once. I've been meaning to have that discussion for a long time but I never had time to follow up. I guess the time is now.

So for now, I guess no action is needed from you.

Commit e54828ad47d92666a9d17d0993bbd41930a66a88 should honour LIBCXXABI_LIBCXX_INCLUDES again. BTW, I think my intent was always to honour it at least temporarily for backwards compatibility purposes, but I seem to have messed it up in my commit. Anyway, now it should be fixed until we can hopefully get rid of it completely.

sbc100 added a subscriber: phosek.Jun 15 2020, 10:31 AM

Great! Thank you very much. I basically agree with your analysis and your direction forward.

It would indeed seem a little strange to be forced to configure the whole of llvm for cross compiling just to get these small runtime libraries built.

Also, have you looked at how llvm/runtimes works? I'm not being using it but I'm hoping to can help a lot in cross compile environments like ours one day. @phosek would probably be good person to be involved with figuring out the long term solution there.

[...]
Also, have you looked at how llvm/runtimes works? I'm not being using it but I'm hoping to can help a lot in cross compile environments like ours one day. @phosek would probably be good person to be involved with figuring out the long term solution there.

Yes, I have looked at how the _runtimes_ build work and I think it's really useful. However, I see the _runtimes_ build as a driver/wrapper over the actual libc++/libc++abi/libunwind builds. Those should still be usable as-is -- the _runtimes_ build only calls those "basic" builds with the just-built compiler, etc.

I believe the most basic way of building libc++/libc++abi/libunwind should be to just use

cmake <llvm-project-root> -DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi;libunwind" <options>

and the _runtimes_ build can then use that with a proper CMAKE_CXX_COMPILER and other options. @phosek do you agree with this?

@sunfish .. this is the change the is breaking wasi-sdk. Sounds like we have a path forward now.