[PATCH] D66068: cmake: Make building clang-shlib optional

2019-08-13 Thread Ethan Sommer via Phabricator via cfe-commits
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

2019-05-21 Thread Ethan Sommer via Phabricator via cfe-commits
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

2019-05-21 Thread Ethan Sommer via Phabricator via cfe-commits
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

2019-05-21 Thread Ethan Sommer via Phabricator via cfe-commits
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

2019-05-21 Thread Ethan Sommer via Phabricator via cfe-commits
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


[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

2019-03-13 Thread Ethan Sommer via Phabricator via cfe-commits
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