[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux
E5ten added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:563 + crtend = Args.hasArg(options::OPT_shared) || IsPIE || IsStaticPIE ? + "crtend_shared" : "crtend"; + CmdArgs.push_back(ToolChain.getCompilerRTArgString( phosek wrote: > MaskRay wrote: > > phosek wrote: > > > MaskRay wrote: > > > > I believe `crtbegin.o` `crtend.o` should just work. It is not necessary > > > > to use `crtbegin_shared.o` `crtend_shared.o`. > > > This is related to your comments on D28791, specifically that we should > > > be using `crtbegin_shared.o` for `-shared` or `-pie` and `crtbegin.o` > > > otherwise, is that not the case? > > Yes. I think we can rename `crtbegin_shared.o` to `crtbegin.o` and use it > > for every configuration: `-no-pie` `-pie` `-shared` `-static` `-static > > -pie`. > We've checked the glibc implementation of `__cxa_finalize`. A nonzero > `__dso_handle` has to match the value passed to `__cxa_atexit` but a zero > `__dso_handle` matches every function registered. So it matters that DSO fini > calls use `&__dso_handle` to match their registrations for the `dlclose` > case, but it also matters that the main executable fini call use zero to run > all the dtors at exit time. It's not clear it really needs to be that way, > but it would affect how the dtors get run which might affect some use cases. > Hence, I don't think we can combine `crtbegin.o` and `crtbegin_shared.o`. I may be wrong but from what I can see crtend and crtend_shared are identical, so while the crtbegins must stay separate can't the 2 crtends be merged into one that gets used in all cases instead of having a duplicate object under a different name? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59264/new/ https://reviews.llvm.org/D59264 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D66068: cmake: Make building clang-shlib optional
E5ten added a comment. I am in favour of adding a user-facing option to disable generating this duplicate library for users that don't need it, like @jvesely says, there should be an option to disable linking a library that takes a long time to link and isn't necessary for a lot of users. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66068/new/ https://reviews.llvm.org/D66068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61909: Add Clang shared library with C++ exports
E5ten added a comment. I might be doing something wrong but this seems to have broken BUILD_SHARED_LIBS for me in that even with that enabled clang is built as a bunch of static libraries linked into a shared one like this patch is supposed to make it do, while I thought that BUILD_SHARED_LIBS was supposed to make it create a bunch of shared libraries as it did before with libclang and still does with libLLVM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61909/new/ https://reviews.llvm.org/D61909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61909: Add Clang shared library with C++ exports
E5ten added a comment. @beanz Wouldn't fixing this by adding OR BUILD_SHARED_LIBS to if(ARG_SHARED) in AddClang.cmake and to if (NOT LLVM_ENABLE_PIC) in clang-shlib/CMakeLists.txt to prevent making libclang_shared when BUILD_SHARED_LIBS is enabled make more sense? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61909/new/ https://reviews.llvm.org/D61909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61909: Add Clang shared library with C++ exports
E5ten added a comment. @beanz Well I took libclang_shared as effectively an equivalent to the libLLVM.so that's created with that dylib option, and when BUILD_SHARED_LIBS is enabled that library is not created, in fact the option to create that library conflicts with BUILD_SHARED_LIBS. Also when the libs are built shared and also as object libraries that are linked into libclang_shared would that not cause the same libraries to be linked into executables twice, once through the shared libraries and once through libclang_shared? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61909/new/ https://reviews.llvm.org/D61909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61909: Add Clang shared library with C++ exports
E5ten added a comment. @beanz But if libclang_shared is intended to be a shippable binary and BUILD_SHARED_LIBS is only intended to be an option used in developer builds, and libclang_shared while not causing conflicts (thanks for the info on how that works by the way) would be redundant in a local developer build one would see BUILD_SHARED_LIBS enabled in wouldn't it? And also to me it doesn't really make sense to enable the intended shippable binary in a build that is specifically enabling a developer option, and so is obviously not intended for shipment (I get that in the arch case it is intended for shipment but that case is them using an option they shouldn't not the option being one that should be used when the built products are intended for redistribution). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61909/new/ https://reviews.llvm.org/D61909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
