On 1/30/15 1:17 PM, Saleem Abdulrasool wrote:> Although this has been discussed in the past, I think that given a few > conversations, it seems that it unfortunately needs to be brought up again. > > There seems to be some disagreement over the ideal location of the > unwinder (libunwind). Currently, libunwind resides in a subdirectory of > libc++abi. There seems to be some desire from multiple parties that it > be moved into compiler-rt or a separate repository. > > Currently, when using compiler-rt as the runtime library (on Linux) we > use libgcc_s and libgcc_eh as the builtins serve as the home for > __gcc_personality_v0. This error handling personality requires > unwinding support, which these libraries provide. This dependency can > be fulfilled with libunwind.I think this dependence should be satisfied by the target system's unwinder, whatever that is. If folks want to use this libunwind for their platform, that's fine... but we should probably continue to use libgcc_s and libgcc_eh on linux when that's the platform's unwinder.> > The concern that has been raised with adjusting this dependency has been > that libunwind is not guaranteed to be built and installed unless > libc++abi is present. The suggestion was that if libunwind were part of > compiler-rt or a separate repository, it would be easier to ensure that > the required library would be built and installed.+1 for putting it in a separate repo. Separate repos helps keep layering nonsense to a minimum. It would be nice if we had some libunwind-specific tests too. Currently we have none, other than the c++ abi tests. Nick, does Apple have any they're willing to upstream?> > Personally, I am of the opinion that a separate repository for it does > make some sense as painful as it sounds. There is a valid point that > the unwinder supports the compiler in some sense (for exception > handling). It seems that its not particularly as intrinsically tied to > the compiler as the builtins are. There is a proper specification that > it implements and interface it exposes, so it can be replaced with an > alternative implementation. At the same time, the unwinder is not > really tied to libc++abi either, though it too has a dependency on it.The fact that this ARM EHABI-induced layering violation is here shouldn't stop us from separating them.> Again, the use here can be replaced with an alternate implementation > (and AIUI, the build system already permits this). > > So, I am bringing up this question once more: what can we do about this > concern? Is moving it to a separate repository acceptable? Or perhaps > moving it to compiler-rt is palatable to more of the involved developers > (as much as I may prefer an alternate solution).Last time we brought this up, there was only partial consensus, and then someone arbitrarily declared total consensus (without compelling arguments in any particular direction) that we were going to move it to compiler_rt. Then the discussion fell on the floor because no-one had time to actually do the move. Please, let's not let that happen again this time. Cheers, Jon> > -- > Saleem Abdulrasool > compnerd (at) compnerd (dot) org-- Jon Roelofs jonathan at codesourcery.com CodeSourcery / Mentor Embedded
On Tue, Feb 3, 2015 at 7:07 PM, Renato Golin <renato.golin at linaro.org> wrote:> On 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> > wrote: > > Last time we brought this up, there was only partial consensus, and then > > someone arbitrarily declared total consensus (without compelling > arguments > > in any particular direction) that we were going to move it to > compiler_rt. > > Then the discussion fell on the floor because no-one had time to > actually do > > the move. Please, let's not let that happen again this time. > > So, do we have a consensus? > > AFAICS, the most agree solution (with optionals to be defined): > > 1. Move Unwinder to its own repository in the LLVM server > 2. Make the CMake connections from libc++abi and compiler-rt > 2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT > 2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT > 2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder > 3. Change clang to assume -lunwind when --rtlib=compiler-rt > 3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s > 3.2 OPTIONAL 5: Add option to change unwinder library by not adding > -lunwind/-lgcc_s, but whatever comes as argument > > 1, 2, and 3 must be changed. > > I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later. > > My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or > something like that. > > I personally don't think the front-end scanning existing libraries is > a good thing to do, but I'm not against the idea, if anyone feels > strongly about it. > > If all of us could agree to a common solution, and make sure all > interested parties are in, we should do the move before 3.7. >I know I'm in some minority, but whatever ends up happening please be aware that some platforms and targets aren't using the bundled unwind. Specifically this non-gnu one (aka HP's libunwind permissively licensed) www.nongnu.org/libunwind/download.html In my experience when it's used in the compiler the name (libgcc, libeh, libunwind) isn't always the same. (So searching for a particular lib name as the unwinder is fragile) ---------- Summary: pretty please allow a cmake option like UNWIND_LIB which can be null or something user defined. (I think this is covered in one of the cases above, but just want to be clear) Thanks for the minute -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150203/22f4c8c7/attachment.html>
On 2/3/15 5:07 AM, Renato Golin wrote:> On 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> wrote: >> Last time we brought this up, there was only partial consensus, and then >> someone arbitrarily declared total consensus (without compelling arguments >> in any particular direction) that we were going to move it to compiler_rt. >> Then the discussion fell on the floor because no-one had time to actually do >> the move. Please, let's not let that happen again this time. > > So, do we have a consensus?Wheeee Two Generals problem...> > AFAICS, the most agree solution (with optionals to be defined): > > 1. Move Unwinder to its own repository in the LLVM server1.1 Freeze the code 1.2 Copy unwinder to its own repo 1.3 Migrate all the buildbots that care about libc++abi's unwinder 1.4 Delete the old copy 1.5 Unfreeze the code> 2. Make the CMake connections from libc++abi and compiler-rt > 2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT > 2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT > 2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder > 3. Change clang to assume -lunwind when --rtlib=compiler-rt > 3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s > 3.2 OPTIONAL 5: Add option to change unwinder library by not adding > -lunwind/-lgcc_s, but whatever comes as argument > > 1, 2, and 3 must be changed. > > I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later.+1> > My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or > something like that. > > I personally don't think the front-end scanning existing libraries is > a good thing to do,Agreed.> but I'm not against the idea, if anyone feels > strongly about it. > > If all of us could agree to a common solution, and make sure all > interested parties are in, we should do the move before 3.7.+1> > Please, cast your votes. > > cheers, > --renato >-- Jon Roelofs jonathan at codesourcery.com CodeSourcery / Mentor Embedded
On Tue, Feb 3, 2015 at 4:07 AM, Renato Golin <renato.golin at linaro.org> wrote:> On 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> > wrote: > > Last time we brought this up, there was only partial consensus, and then > > someone arbitrarily declared total consensus (without compelling > arguments > > in any particular direction) that we were going to move it to > compiler_rt. > > Then the discussion fell on the floor because no-one had time to > actually do > > the move. Please, let's not let that happen again this time. > > So, do we have a consensus? >I think so.> AFAICS, the most agree solution (with optionals to be defined): > > 1. Move Unwinder to its own repository in the LLVM server > 2. Make the CMake connections from libc++abi and compiler-rt > 2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT > 2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT > 2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder > 3. Change clang to assume -lunwind when --rtlib=compiler-rt > 3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s > 3.2 OPTIONAL 5: Add option to change unwinder library by not adding > -lunwind/-lgcc_s, but whatever comes as argument > > 1, 2, and 3 must be changed. > > I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later. >It seems that there is some interest in making the unwinder a build time option, so that should be as good as 2.2 right? I am definitely fine with 3.1 (its no better or worse from the status quo). I would like say that although Im not a fan of options 2.1 and 2.2, I do think that 2.3 is a bad idea. There is far too much complexity involved, and worse yet, it needs a number of knobs to get right -- what if I have gcc 4.7, 4.8, 4.9 (each has its own libgcc_s), how do you select one? What if the layout differs between Linux distributions? There is just too much variability. Trying to reuse part of another compiler's resources is just too brittle IMO, and Id rather not introduce more of that behavior into clang.> My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or > something like that. >Thats fine; it is effectively what I was suggesting that we do if people feel strongly that we need to support clang_rt + gcc_s.> I personally don't think the front-end scanning existing libraries is > a good thing to do, but I'm not against the idea, if anyone feels > strongly about it. >Strongly agreed. This can easily change subtly and diverge in behavior from the linker.> If all of us could agree to a common solution, and make sure all > interested parties are in, we should do the move before 3.7. >I was hoping to do this much sooner (in the next few days) for the 3.7 release :-).> Please, cast your votes. > > cheers, > --renato >-- Saleem Abdulrasool compnerd (at) compnerd (dot) org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150203/3a546a83/attachment.html>
On Feb 3, 2015, at 4:07 AM, Renato Golin <renato.golin at linaro.org> wrote:> On 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> wrote: >> Last time we brought this up, there was only partial consensus, and then >> someone arbitrarily declared total consensus (without compelling arguments >> in any particular direction) that we were going to move it to compiler_rt. >> Then the discussion fell on the floor because no-one had time to actually do >> the move. Please, let's not let that happen again this time. > > So, do we have a consensus? > > AFAICS, the most agree solution (with optionals to be defined): > > 1. Move Unwinder to its own repository in the LLVM serverThis is fine with me. But I thought when I first submitted the source there was push back because we “already have too many repositories”.> 2. Make the CMake connections from libc++abi and compiler-rt > 2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT > 2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT > 2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder > 3. Change clang to assume -lunwind when --rtlib=compiler-rt > 3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s > 3.2 OPTIONAL 5: Add option to change unwinder library by not adding > -lunwind/-lgcc_s, but whatever comes as argumentI assume these are all just about ELF platforms. Darwin does not need any of this logic. -Nick> > 1, 2, and 3 must be changed. > > I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later. > > My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or > something like that. > > I personally don't think the front-end scanning existing libraries is > a good thing to do, but I'm not against the idea, if anyone feels > strongly about it. > > If all of us could agree to a common solution, and make sure all > interested parties are in, we should do the move before 3.7. > > Please, cast your votes. > > cheers, > --renato
+Iain On 2/3/15 5:07 AM, Renato Golin wrote:> On 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> wrote: >> Last time we brought this up, there was only partial consensus, and then >> someone arbitrarily declared total consensus (without compelling arguments >> in any particular direction) that we were going to move it to compiler_rt. >> Then the discussion fell on the floor because no-one had time to actually do >> the move. Please, let's not let that happen again this time. > > So, do we have a consensus? > > AFAICS, the most agree solution (with optionals to be defined): > > 1. Move Unwinder to its own repository in the LLVM server > 2. Make the CMake connections from libc++abi and compiler-rt > 2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT > 2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT > 2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder > 3. Change clang to assume -lunwind when --rtlib=compiler-rt > 3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s > 3.2 OPTIONAL 5: Add option to change unwinder library by not adding > -lunwind/-lgcc_s, but whatever comes as argument > > 1, 2, and 3 must be changed. > > I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later. > > My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or > something like that. > > I personally don't think the front-end scanning existing libraries is > a good thing to do, but I'm not against the idea, if anyone feels > strongly about it. > > If all of us could agree to a common solution, and make sure all > interested parties are in, we should do the move before 3.7. > > Please, cast your votes.Out-of-band +1 on this from Iain Sandoe.> > cheers, > --renato > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Jon Roelofs jonathan at codesourcery.com CodeSourcery / Mentor Embedded
On Tue, Feb 3, 2015 at 4:07 AM, Renato Golin <renato.golin at linaro.org> wrote:> On 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> > wrote: > > Last time we brought this up, there was only partial consensus, and then > > someone arbitrarily declared total consensus (without compelling > arguments > > in any particular direction) that we were going to move it to > compiler_rt. > > Then the discussion fell on the floor because no-one had time to > actually do > > the move. Please, let's not let that happen again this time. > > So, do we have a consensus? > > AFAICS, the most agree solution (with optionals to be defined): >1. Move Unwinder to its own repository in the LLVM server>I think that we have consensus here, and we've given people time to chime in. What exactly is the process for making this happen? I assume that aKor or someone else would need to create the repository on the server side so that it can be populated or is there someone else who should be contacted so that we can get this going? 2. Make the CMake connections from libc++abi and compiler-rt> 2.1 OPTIONAL 1: err if libunwinder is not there, clang errs if rtlib=RT > 2.2 OPTIONAL 2: warns if libunwind is not there, clang errs if rtlib=RT > 2.3 OPTIONAL 3: nothing, make clang smarter to pick existing unwinder > 3. Change clang to assume -lunwind when --rtlib=compiler-rt > 3.1 OPTIONAL 4: allow linker error if no -lunwind / -lgcc_s > 3.2 OPTIONAL 5: Add option to change unwinder library by not adding > -lunwind/-lgcc_s, but whatever comes as argument > > 1, 2, and 3 must be changed. > > I vote for adding { 2.2, 3.1 } for now, { 2.2, 3.1, 3.2 } for later. > > My idea for 3.2 is something like --unwinder=libgcc_s / libunwind, or > something like that. > > I personally don't think the front-end scanning existing libraries is > a good thing to do, but I'm not against the idea, if anyone feels > strongly about it. > > If all of us could agree to a common solution, and make sure all > interested parties are in, we should do the move before 3.7. > > Please, cast your votes. > > cheers, > --renato >-- Saleem Abdulrasool compnerd (at) compnerd (dot) org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150204/0140584d/attachment.html>