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. 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. 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. 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). -- Saleem Abdulrasool compnerd (at) compnerd (dot) org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150130/3c3894a2/attachment.html>
On 30 January 2015 at 20:17, Saleem Abdulrasool <compnerd at compnerd.org> wrote:> 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.Unwinding is not just used for EH, but also debugging, profiling, sanitizers (when in slow-mode) and possibly other code inspection tools. While still not strictly for *compiler* support, it is for many other toolchain components. The counter argument, that libc++ has other low-level code to deal with exception handling so it would make sense to bundle the unwinder together, is also compelling. But in my view, less so.> 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).For my purposes, bundling it in compiler-rt would be the easiest solution. It'd also make Clang simpler regarding --rtlib=compiler-rt. I'm also assuming that libc++ should be able to work with libgcc_s and other unwinding libraries, and, even if our unwinder is local to it, it seems very standard in the interface it exports. However, if libc++ folks don't want to depend on compiler-rt or libgcc_s for their own unwinding, I don't see a problem in having it in a separate repository under the LLVM server. One less, one more, won't make a difference for developers and build systems. What I don't agree is to leave it as it is, so that even to compile C code in debug/profile mode, I need to either include libgcc_s or libc++abi. cheers, --renato
I thought the ARM EHABI added a twist to this because it created some upward dependency from the unwinder to libc++abi. Other than that, I don’t have any strong feeling where it lives. -Nick On Jan 30, 2015, at 12:33 PM, Renato Golin <renato.golin at linaro.org> wrote:> On 30 January 2015 at 20:17, Saleem Abdulrasool <compnerd at compnerd.org> wrote: >> 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. > > Unwinding is not just used for EH, but also debugging, profiling, > sanitizers (when in slow-mode) and possibly other code inspection > tools. While still not strictly for *compiler* support, it is for many > other toolchain components. > > The counter argument, that libc++ has other low-level code to deal > with exception handling so it would make sense to bundle the unwinder > together, is also compelling. But in my view, less so. > > >> 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). > > For my purposes, bundling it in compiler-rt would be the easiest > solution. It'd also make Clang simpler regarding --rtlib=compiler-rt. > I'm also assuming that libc++ should be able to work with libgcc_s and > other unwinding libraries, and, even if our unwinder is local to it, > it seems very standard in the interface it exports. > > However, if libc++ folks don't want to depend on compiler-rt or > libgcc_s for their own unwinding, I don't see a problem in having it > in a separate repository under the LLVM server. One less, one more, > won't make a difference for developers and build systems. > > What I don't agree is to leave it as it is, so that even to compile C > code in debug/profile mode, I need to either include libgcc_s or > libc++abi. > > cheers, > --renato
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 Fri, Jan 30, 2015 at 12:43 PM, Jonathan Roelofs < jonathan at codesourcery.com> wrote:> > > 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.I think that controlling that via a flag is best if we want to continue using libgcc_s/libgcc_eh instead of libuwnind.> > >> 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.I agree with this.> > 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. >Im willing to do the leg work if thats what it takes, however, Id like to get a clear consensus before proceeding. So far, it seems that there is no argument against moving it into a separate repository. Everyone agrees that this would be conducive to ensuring that we don't end up with unintended layering violations. Why don't we give it a few days to allow others to comment before going with that solution (hopefully all the particularly interested parties are already on this thread).> > Cheers, > > Jon > > >> -- >> Saleem Abdulrasool >> compnerd (at) compnerd (dot) org >> > > -- > Jon Roelofs > jonathan at codesourcery.com > CodeSourcery / Mentor Embedded >-- Saleem Abdulrasool compnerd (at) compnerd (dot) org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150130/6a0f729f/attachment.html>
On Jan 30, 2015, at 12:43 PM, Jonathan Roelofs <jonathan at codesourcery.com> wrote:> 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?Here is what Apple has: http://opensource.apple.com/source/libunwind/libunwind-35.3/testsuite/ If you think these are useful, I can upstream them. But, they are either generic C++ exception tests (already covered by libc++ tests) or are darwin specific assembly that tests that registers are restored properly. The latter are good things to test in the unwinder but will need some massaging (and a test harness) to work cross platform. -Nick> >> >> 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 30 January 2015 at 20:43, Jonathan Roelofs <jonathan at codesourcery.com> wrote:> 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.That's precisely the point. People should use unwinders they want, and libc++abi should be completely agnostic of which one you use.> +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?+1> 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.Well, people that didn't mind it in compiler-rt were the last ones to drop the thread, so it "seemed" like we forced a consensus, but there really wasn't one. At least, I wasn't convinced, but wasn't bothered either. Moving to its own repo is also a good solution. cheers, --renato
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. cheers, --renato