[PING] Contributing new gcc targets: i386-*-dragonfly and x86-64-*-dragonfly
Hi folks, Does anyone have any issues with this set of patches to add support for the DragonFly targets? It's a blocker for other patches of mine that have a more general benefit, but this (relatively simple) one has to go in first. Thanks, John On 4/20/2014 21:04, John Marino wrote: On 4/20/2014 14:51, Jonathan Wakely wrote: On 19 April 2014 20:39, John Marino wrote: Hello GCC developers, For the last few years, I have been maintaining a large set of patches that add support for the DragonFly BSD target and also complete Ada frontend support on all four major BSDs among other things. Before I can submit patches for Ada or testsuite cases, DragonFly must be a recognized, working target. The patches attached here will provide out-of-the-box support for the C, C++, Objective-C and Fortran frontends. Thanks for the patch - I only have a few general, minor comments. As noted at http://gcc.gnu.org/lists.html C++ library patches should go to the libstdc++ list as well as gcc-patches, so I've CC'd that list (original mail and patch are at http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01128.html) Patches should not include generated files such as configure, as the diffs don't always apply cleanly and the changes are implied by the patches to files such as acinclude.m4 and configure.ac. The regenerated versions should of course be committed, and the ChangeLog should mention they are regenerated, as you've done. Thanks for your advice, Jonathan. I've updated the patch to remove the two configure file patches. I also removed an errant -rpath from the dragonfly.h specs that crept in from FreeBSD ports. I've attached the updated patch to this email. The changelog text should be correctly capitalised and sentences ended with a period (e.g. New target. and New. not New target and new). The individual ChangeLog entries at http://leaf.dragonflybsd.org/~marino/gcc-df-target/changelog_entries/ would generally be used as the commit message, grouped and prefixed by the name of the sub-directory: I have updated the six entry files at http://leaf.dragonflybsd.org/~marino/gcc-df-target/changelog_entries/ to conform to this style. I updated the proposed commit message accordingly: http://leaf.dragonflybsd.org/~marino/gcc-df-target/proposed_commit-msg.txt The libstdc++ changes are OK for trunk if the rest gets approved. Thanks! I see from the critique of another submitted patch that also touches liberty that I'm supposed to cross-post to gdb and binutils, so I've cc'd them as well. Regards, John
Re: [PING] Contributing new gcc targets: i386-*-dragonfly and x86-64-*-dragonfly
On Tue, Apr 29, 2014 at 11:37 AM, John Marino gnu...@marino.st wrote: Does anyone have any issues with this set of patches to add support for the DragonFly targets? It's a blocker for other patches of mine that have a more general benefit, but this (relatively simple) one has to go in first. It's inconvenient, but patches are much more likely to be reviewed when they cover a separate part of the tree, as different people maintain different parts. I expect your libitm and libcilkrts could be approved trivially if you send them separately. The change to include/libiberty.h is fine. I don't understand the benefit of libgcc/enable-execute-stack-bsd.c. The code seems the same as the existing libgcc/enable-execute-stack-mprotect.c. All you are changing is omitting need_enable_exec_stack. If you just drop the FreeBSD constructor, you will get the behaviour you want. Ian
Re: [PING] Contributing new gcc targets: i386-*-dragonfly and x86-64-*-dragonfly
On 4/29/2014 19:23, Ian Lance Taylor wrote: On Tue, Apr 29, 2014 at 11:37 AM, John Marino gnu...@marino.st wrote: Does anyone have any issues with this set of patches to add support for the DragonFly targets? It's a blocker for other patches of mine that have a more general benefit, but this (relatively simple) one has to go in first. It's inconvenient, but patches are much more likely to be reviewed when they cover a separate part of the tree, as different people maintain different parts. I expect your libitm and libcilkrts could be approved trivially if you send them separately. Hi Ian, I was trying to identify specific people (e.g. an libitm person) and have them approve specific files since they are trivial as you saw. I decided to keep the patch set as an atomic unit because it needs to be committed as a unit, and also because I assumed it provided the necessary context. The change to include/libiberty.h is fine. thanks! I don't understand the benefit of libgcc/enable-execute-stack-bsd.c. The code seems the same as the existing libgcc/enable-execute-stack-mprotect.c. All you are changing is omitting need_enable_exec_stack. If you just drop the FreeBSD constructor, you will get the behaviour you want. With the caveat that this patch is over 2 years old, I just took a look at both files. I would have not needed to modify this file at all for DragonFly. In fact, I seem to recall that I didn't modify it for DragonFly, but rather for FreeBSD. If I had to guess, it would be that I found mprotect() was needed regardless of value of kern.stackprot. I must have traced some test failures back to this. Which I guess that's what you mean - just delete the block between #if defined __FreeBSD__ and the next #elif which should be equivalent. I can tweak the patch set to do that. And what about the dl_iterate_phdr changes? Do they look good to you? Thanks, John
Re: [PING] Contributing new gcc targets: i386-*-dragonfly and x86-64-*-dragonfly
On Tue, Apr 29, 2014 at 2:37 PM, John Marino gnu...@marino.st wrote: I don't understand the benefit of libgcc/enable-execute-stack-bsd.c. The code seems the same as the existing libgcc/enable-execute-stack-mprotect.c. All you are changing is omitting need_enable_exec_stack. If you just drop the FreeBSD constructor, you will get the behaviour you want. With the caveat that this patch is over 2 years old, I just took a look at both files. I would have not needed to modify this file at all for DragonFly. In fact, I seem to recall that I didn't modify it for DragonFly, but rather for FreeBSD. If I had to guess, it would be that I found mprotect() was needed regardless of value of kern.stackprot. I must have traced some test failures back to this. Which I guess that's what you mean - just delete the block between #if defined __FreeBSD__ and the next #elif which should be equivalent. I can tweak the patch set to do that. Yes. And what about the dl_iterate_phdr changes? Do they look good to you? They looked fine to me but I'm not a build system maintainer. Ian
