| Message ID | 5449FC98.1060404@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 24, 2014 at 08:15:36AM +0100, Phil Muldoon wrote: > On 10/10/14 22:58, Jeff Law wrote: > > On 10/09/14 03:07, Phil Muldoon wrote: > >> > >> Sorry for taking so long to reply. We've talked, on irc and elsewhere > >> a little (some at the Cauldron too!). I think the consensus is as > >> nobody has explicitly mentioned anything, this is OK to go in? > > Yes, please go ahead and check it in. You'll be the first contact point if something goes wrong :-) > > > > Given the length of time since the original post and now, can you please do sanity bootstrap to make sure nothing's bitrotted before you commit? > > I rebased the patch on top of GCC head (from the git repository), > updated the ChangeLogs, etc from two days ago (it takes two days to do > a full rebase, pristine and patched bootstrap and testrun on my poor > laptop ;). > > I've built both pristine and patched branches with bootstrap enabled. > I ran both testsuites and used contrib/compare_tests to make sure > everything was as it should be. compare_tests reports everything as > fine. One minor change I found, was due to some ongoing work on > hash_tables. It seems to parameterless constructor call for a new > hash table has been removed. This was trivially fixed with the patch > attached. Even though (to me) it is obvious, what do you think? > --- a/libcc1/plugin.cc > +++ b/libcc1/plugin.cc > @@ -220,13 +220,10 @@ static plugin_context *current_context; > > plugin_context::plugin_context (int fd) > : cc1_plugin::connection (fd), > - address_map (), > - preserved (), > - file_names () > + address_map (30), > + preserved (30), > + file_names (30) > { > - address_map.create (20); > - preserved.create (20); > - file_names.create (20); This is http://gcc.gnu.org/r211936 , i.e. https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01598.html so the changes are fine. > } > > void > @@ -236,8 +233,8 @@ plugin_context::mark () > it != address_map.end (); > ++it) > { > - ggc_mark ((*it).decl); > - ggc_mark ((*it).address); > + ggc_mark ((*it)->decl); > + ggc_mark ((*it)->address); > } And this is http://gcc.gnu.org/r211937 , i.e. https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01599.html in action. > for (hash_table< pointer_hash<tree_node> >::iterator it = preserved.begin (); So, if these are the only non-obvious changes you needed, please go ahead and commit. Jakub
On 10/24/14 01:15, Phil Muldoon wrote: > On 10/10/14 22:58, Jeff Law wrote: >> On 10/09/14 03:07, Phil Muldoon wrote: >>> >>> Sorry for taking so long to reply. We've talked, on irc and elsewhere >>> a little (some at the Cauldron too!). I think the consensus is as >>> nobody has explicitly mentioned anything, this is OK to go in? >> Yes, please go ahead and check it in. You'll be the first contact point if something goes wrong :-) >> >> Given the length of time since the original post and now, can you please do sanity bootstrap to make sure nothing's bitrotted before you commit? > > I rebased the patch on top of GCC head (from the git repository), > updated the ChangeLogs, etc from two days ago (it takes two days to do > a full rebase, pristine and patched bootstrap and testrun on my poor > laptop ;). Get a new laptop :-) The process for getting one from corporate isn't bad. In fact, I just got my refreshed laptop last week. > > I've built both pristine and patched branches with bootstrap enabled. > I ran both testsuites and used contrib/compare_tests to make sure > everything was as it should be. compare_tests reports everything as > fine. One minor change I found, was due to some ongoing work on > hash_tables. It seems to parameterless constructor call for a new > hash table has been removed. This was trivially fixed with the patch > attached. Even though (to me) it is obvious, what do you think? Looks fine to me. jeff
> On 10/10/14 22:58, Jeff Law wrote: >> On 10/09/14 03:07, Phil Muldoon wrote: > Given the length of time since the original post and now, can you please do sanity bootstrap to make sure nothing's bitrotted before you commit? >> I've built both pristine and patched branches with bootstrap enabled. >> I ran both testsuites and used contrib/compare_tests to make sure >> everything was as it should be. compare_tests reports everything as >> fine. One minor change I found, was due to some ongoing work on >> hash_tables. It seems to parameterless constructor call for a new >> hash table has been removed. This was trivially fixed with the patch >> attached. Even though (to me) it is obvious, what do you think? > Looks fine to me. > > jeff > On 24/10/14 08:43, Jakub Jelinek wrote: > So, if these are the only non-obvious changes you needed, please go > ahead and commit. > Jakub Thanks. This patch has now been committed. Bootstrap and testsuite pre and post are as expected. Thanks for your help in getting this patch-series committed. Cheers Phil
I'm seeing a different bootstrap failure from those already discussed:
In file included from
/scratch/jmyers/fsf/gcc-mainline/libcc1/../gcc/gcc-plugin.h:28:0,
from
/scratch/jmyers/fsf/gcc-mainline/libcc1/plugin.cc:34:
/scratch/jmyers/fsf/gcc-mainline/libcc1/../gcc/system.h:653:17: fatal error: gmp.h: No such file or directory
It appears the build is ignoring the --with-gmp option passed to
configure. Since <gmp.h> is included in system.h, if you include system.h
you have to pass the right -I option corresponding to --with-gmp /
--with-gmp-include. (There are several other such configure options for
MPFR, MPC, CLooG, ISL, libiconv at least - whether they are relevant
depends on whether your code ends up including the relevant headers.)
On 28/10/14 13:19, Joseph S. Myers wrote: > I'm seeing a different bootstrap failure from those already discussed: > > In file included from > /scratch/jmyers/fsf/gcc-mainline/libcc1/../gcc/gcc-plugin.h:28:0, > from > /scratch/jmyers/fsf/gcc-mainline/libcc1/plugin.cc:34: > /scratch/jmyers/fsf/gcc-mainline/libcc1/../gcc/system.h:653:17: fatal error: gmp.h: No such file or directory > > It appears the build is ignoring the --with-gmp option passed to > configure. Since <gmp.h> is included in system.h, if you include system.h > you have to pass the right -I option corresponding to --with-gmp / > --with-gmp-include. (There are several other such configure options for > MPFR, MPC, CLooG, ISL, libiconv at least - whether they are relevant > depends on whether your code ends up including the relevant headers.) Joseph, Hi, sorry for the troubles! I am having difficulty seeing this fail on my system. I built gmp from upstream, installed it, and pointed to the install location with --with-gmp. Which stage does your build fail at? I am actually not totally sure how to respect the -with-gmp argument in libcc1. auto* tools are not my strongest skill. ;) I notice gcc/configure.ac I think just exports the variables to Makefile.in from the main configure script. That what we should do in this case? Cheers Phil
On Tue, 28 Oct 2014, Phil Muldoon wrote: > Joseph, > > Hi, sorry for the troubles! I am having difficulty seeing this fail on > my system. I built gmp from upstream, installed it, and pointed to > the install location with --with-gmp. Which stage does your build fail > at? To get the failure you need not to have GMP installed somewhere the bootstrap compiler would otherwise find (e.g. uninstall your system GMP package before testing). The failure is building stage 1. > I am actually not totally sure how to respect the -with-gmp argument > in libcc1. auto* tools are not my strongest skill. ;) > > I notice gcc/configure.ac I think just exports the variables to > Makefile.in from the main configure script. That what we should do in > this case? Toplevel passes GMPINC down to subdirectories. I think you should (a) copy AC_ARG_VAR(GMPINC,[How to find GMP include files]) from gcc/configure.ac; (b) copy GMPINC = @GMPINC@ from gcc/Makefile.in; (c) add $(GMPINC) to AM_CPPFLAGS.
diff --git a/libcc1/plugin.cc b/libcc1/plugin.cc index fbb49d3..5cdd19d 100644 --- a/libcc1/plugin.cc +++ b/libcc1/plugin.cc @@ -220,13 +220,10 @@ static plugin_context *current_context; plugin_context::plugin_context (int fd) : cc1_plugin::connection (fd), - address_map (), - preserved (), - file_names () + address_map (30), + preserved (30), + file_names (30) { - address_map.create (20); - preserved.create (20); - file_names.create (20); } void @@ -236,8 +233,8 @@ plugin_context::mark () it != address_map.end (); ++it) { - ggc_mark ((*it).decl); - ggc_mark ((*it).address); + ggc_mark ((*it)->decl); + ggc_mark ((*it)->address); } for (hash_table< pointer_hash<tree_node> >::iterator it = preserved.begin ();