ldionne added inline comments.
================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:124
set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
- set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
----------------
phosek wrote:
> ldionne wrote:
> > Note that I am removing these options here because I don't think they are
> > required -- since we specify `LIBCXXABI_ENABLE_SHARED=OFF`, there is no
> > shared libc++abi to link against, so we should already be linking against
> > `libc++abi.a` regardless of this change.
> This option was set to merge `libc++abi.a` into `libc++.a` so to achieve the
> same effect, presumably we would need to set
> `-DLIBCXX_CXX_ABI=libcxxabi-objects`?
I agree this is suspicious, but why is there no
`LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY` specified here then? I can add
`-DLIBCXX_CXX_ABI=libcxxabi-objects`, I just want to make sure we both
understand what's going on.
================
Comment at: libcxx/cmake/caches/MinGW.cmake:3
-set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "")
+set(LIBCXX_CXX_ABI libcxxabi-objects CACHE STRING "")
set(LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
----------------
mstorsjo wrote:
> Unfortunately, setting `LIBCXX_CXX_ABI=libcxxabi-objects` here in the cache
> file doesn't have any effect, because the `run-buildbot` script invokes cmake
> with a different parameter:
> ```
> function generate-cmake() {
> generate-cmake-base \
> -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
> -DLIBCXX_CXX_ABI=libcxxabi \
> "${@}"
> }
> ```
> A cmake parameter explicitly set on the command line always overrides what's
> set in a cache file.
I actually don't see why we need to specify the ABI library to use by libc++
manually there, so I'll remove it. It should be just the same for all the CI
bots we have anyways, but if we did introduce a CI configuration where we don't
want to use libc++abi as our ABI library from libc++, it would make sense not
to overwrite it. We are testing the libc++abi code base separately regardless
of that setting anyway.
================
Comment at: libcxxabi/CMakeLists.txt:254
# line or via a cache file), use its expected default value (enabled).
- if ((LIBCXX_ENABLE_SHARED OR NOT DEFINED LIBCXX_ENABLE_SHARED) AND
LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+ if ((LIBCXX_ENABLE_SHARED OR NOT DEFINED LIBCXX_ENABLE_SHARED) AND
LIBCXX_CXX_ABI STREQUAL libcxxabi-objects)
# Building libcxxabi statically, but intending for it to be statically
----------------
mstorsjo wrote:
> The fallback handling (checking `LIBCXX_ENABLE_STATIC_ABI_LIBRARY`, updating
> `LIBCXX_CXX_ABI` based on that) in libcxx doesn't have any effect here, since
> libcxxabi is processed strictly before libcxx, so here we only see the
> original values of the options.
>
> However, luckily enough, I've posted D125715 a couple days ago, which lets us
> get rid of this whole problematic piece of code altogether (together with
> fixing a couple other issues), now that we have object libraries in palce!
Awesome, thanks! This should go away when I rebase, then.
================
Comment at: llvm/docs/HowToBuildWindowsItaniumPrograms.rst:159
-* ``-DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON``
* ``-DLIBCXX_CXX_ABI=libcxxabi``
* ``-DLIBCXX_CXX_ABI_INCLUDE_PATHS=<libcxxabi src path>/include``
----------------
mstorsjo wrote:
> If the example instructions used to say `LIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON`
> before, you should update the `LIBCXX_CXX_ABI` setting here to
> `libcxxabi-objects` too.
Ugh, good catch, thanks. Not sure why I didn't do it that way in the first
place.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125683/new/
https://reviews.llvm.org/D125683
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
