phosek 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 "")
----------------
ldionne wrote:
> phosek wrote:
> > ldionne wrote:
> > > ldionne wrote:
> > > > phosek wrote:
> > > > > ldionne wrote:
> > > > > > 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.
> > > > > This is intentional. We are merging `libc++abi.a` into `libc++.a` but 
> > > > > we ship `libc++abi.so` and `libc++.so` as separate (and use the 
> > > > > generated linker script to pull in `libc++abi.so` when you pass 
> > > > > `-lc++` to linker). I'd be fine merging `libc++abi.so` into 
> > > > > `libc++.so` as well, but we'll need to figure out a transition plan 
> > > > > since there are several places in our build right now that expect 
> > > > > `libc++abi.so` to exist. We cannot land this change as is because 
> > > > > that would break the `-static-libstdc++` use case, since Clang driver 
> > > > > only passes `-lc++` to the linker and not `-lc++abi` and there's 
> > > > > nothing that would pull `libc++abi.a` in.
> > > > I see, so to summarize, basically you want to use `libcxxabi-objects` 
> > > > for the static `libc++.a`, but `libcxxabi` for the dynamic `libc++.so`. 
> > > > This change as currently laid out does not permit that to happen, since 
> > > > `libcxxabi-objects` implies that the objects are merged both in the 
> > > > static and in the shared library. I guess we could introduce a new 
> > > > `libcxxabi-objects-static` option, however that would be kind of 
> > > > strange. We can either do that, or wait for you to stop relying on 
> > > > `libc++abi.so` existing and switch to `libcxxabi-objects` wholesale for 
> > > > Fuchsia. WDYT?
> > > Gentle ping. It would be nice to land this in one form or another. If you 
> > > don't think Fuchsia can stop relying on `libc++abi.so` being there, I 
> > > could add yet another `libcxxabi-objects-static` -- I still feel like 
> > > that's better than the status quo.
> > We discussed this on our team and we don't have any issues switching 
> > towards combined `libc++.so`, we just need to figure out a transition plan. 
> > Let me test this change locally to see if landing this as is will break 
> > anything.
> Gentle ping -- did you get any time to investigate this?
I did and we are currently hitting a failure related to our libunwind usage. I 
have created D127528 which should address it. 


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

Reply via email to