| Message ID | 20140402122349.GZ19304@virgil.suse |
|---|---|
| State | New |
| Headers | show |
On Wed, 2 Apr 2014, Martin Jambor wrote: > Hi, > > recently I've been looking into a number of bugs involving > symtab_remove_unreachable_nodes in one way or another and I have > always started by applying the hunk below. I did this because > distinguishing different symbol nodes only according to their names is > just so inconvenient, especially when compiling C++. The risk is > minimal and therefore I'd like to propose it to trunk even at this > late stage, although I can of course wait until the next stage1. > > The other hunk is something that I think is also useful when looking > into all failures of ipcp_verify_propagated_values like e.g. PR 60727. > > I included the patch in a recent bootstrap and testing and it of > course passes. OK for trunk now? Or later? I'll leave the actual changes for review by Honza, it's fine at this stage if he things the changes make sense and are consistent. Thanks, Richard. > Thanks, > > Martin > > > 2014-04-01 Martin Jambor <mjambor@suse.cz> > > * ipa-cp.c (ipcp_verify_propagated_values): Also dump symtab and > mention gcc_unreachable before failing. > * ipa.c (symtab_remove_unreachable_nodes): Also print order of > removed symbols. > > Index: src/gcc/ipa-cp.c > =================================================================== > --- src.orig/gcc/ipa-cp.c > +++ src/gcc/ipa-cp.c > @@ -884,8 +884,9 @@ ipcp_verify_propagated_values (void) > { > if (dump_file) > { > + dump_symtab (dump_file); > fprintf (dump_file, "\nIPA lattices after constant " > - "propagation:\n"); > + "propagation, before gcc_unreachable:\n"); > print_all_lattices (dump_file, true, false); > } > > Index: src/gcc/ipa.c > =================================================================== > --- src.orig/gcc/ipa.c > +++ src/gcc/ipa.c > @@ -469,7 +469,7 @@ symtab_remove_unreachable_nodes (bool be > if (!node->aux) > { > if (file) > - fprintf (file, " %s", node->name ()); > + fprintf (file, " %s/%i", node->name (), node->order); > cgraph_remove_node (node); > changed = true; > } > @@ -483,7 +483,7 @@ symtab_remove_unreachable_nodes (bool be > if (node->definition) > { > if (file) > - fprintf (file, " %s", node->name ()); > + fprintf (file, " %s/%i", node->name (), node->order); > node->body_removed = true; > node->analyzed = false; > node->definition = false; > @@ -531,7 +531,7 @@ symtab_remove_unreachable_nodes (bool be > && (!flag_ltrans || !DECL_EXTERNAL (vnode->decl))) > { > if (file) > - fprintf (file, " %s", vnode->name ()); > + fprintf (file, " %s/%i", vnode->name (), vnode->order); > varpool_remove_node (vnode); > changed = true; > } > >
> On Wed, 2 Apr 2014, Martin Jambor wrote: > > > Hi, > > > > recently I've been looking into a number of bugs involving > > symtab_remove_unreachable_nodes in one way or another and I have > > always started by applying the hunk below. I did this because > > distinguishing different symbol nodes only according to their names is > > just so inconvenient, especially when compiling C++. The risk is > > minimal and therefore I'd like to propose it to trunk even at this > > late stage, although I can of course wait until the next stage1. > > > > The other hunk is something that I think is also useful when looking > > into all failures of ipcp_verify_propagated_values like e.g. PR 60727. > > > > I included the patch in a recent bootstrap and testing and it of > > course passes. OK for trunk now? Or later? > > I'll leave the actual changes for review by Honza, it's fine at this > stage if he things the changes make sense and are consistent. It seems fine to me... > > Thanks, > Richard. > > > Thanks, > > > > Martin > > > > > > 2014-04-01 Martin Jambor <mjambor@suse.cz> > > > > * ipa-cp.c (ipcp_verify_propagated_values): Also dump symtab and > > mention gcc_unreachable before failing. > > * ipa.c (symtab_remove_unreachable_nodes): Also print order of > > removed symbols. > > > > Index: src/gcc/ipa-cp.c > > =================================================================== > > --- src.orig/gcc/ipa-cp.c > > +++ src/gcc/ipa-cp.c > > @@ -884,8 +884,9 @@ ipcp_verify_propagated_values (void) > > { > > if (dump_file) > > { > > + dump_symtab (dump_file); > > fprintf (dump_file, "\nIPA lattices after constant " > > - "propagation:\n"); > > + "propagation, before gcc_unreachable:\n"); This means before symtab_remove_unreachable_nodes? Honza > > print_all_lattices (dump_file, true, false); > > } > > > > Index: src/gcc/ipa.c > > =================================================================== > > --- src.orig/gcc/ipa.c > > +++ src/gcc/ipa.c > > @@ -469,7 +469,7 @@ symtab_remove_unreachable_nodes (bool be > > if (!node->aux) > > { > > if (file) > > - fprintf (file, " %s", node->name ()); > > + fprintf (file, " %s/%i", node->name (), node->order); > > cgraph_remove_node (node); > > changed = true; > > } > > @@ -483,7 +483,7 @@ symtab_remove_unreachable_nodes (bool be > > if (node->definition) > > { > > if (file) > > - fprintf (file, " %s", node->name ()); > > + fprintf (file, " %s/%i", node->name (), node->order); > > node->body_removed = true; > > node->analyzed = false; > > node->definition = false; > > @@ -531,7 +531,7 @@ symtab_remove_unreachable_nodes (bool be > > && (!flag_ltrans || !DECL_EXTERNAL (vnode->decl))) > > { > > if (file) > > - fprintf (file, " %s", vnode->name ()); > > + fprintf (file, " %s/%i", vnode->name (), vnode->order); > > varpool_remove_node (vnode); > > changed = true; > > } > > > > > > -- > Richard Biener <rguenther@suse.de> > SUSE / SUSE Labs > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 > GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
Hi, On Wed, Apr 02, 2014 at 06:08:27PM +0200, Jan Hubicka wrote: > > On Wed, 2 Apr 2014, Martin Jambor wrote: > > > > > Hi, > > > > > > recently I've been looking into a number of bugs involving > > > symtab_remove_unreachable_nodes in one way or another and I have > > > always started by applying the hunk below. I did this because > > > distinguishing different symbol nodes only according to their names is > > > just so inconvenient, especially when compiling C++. The risk is > > > minimal and therefore I'd like to propose it to trunk even at this > > > late stage, although I can of course wait until the next stage1. > > > > > > The other hunk is something that I think is also useful when looking > > > into all failures of ipcp_verify_propagated_values like e.g. PR 60727. > > > > > > I included the patch in a recent bootstrap and testing and it of > > > course passes. OK for trunk now? Or later? > > > > I'll leave the actual changes for review by Honza, it's fine at this > > stage if he things the changes make sense and are consistent. > > It seems fine to me... Thanks, I will commit it shortly then. > > > > Thanks, > > Richard. > > > > > Thanks, > > > > > > Martin > > > > > > > > > 2014-04-01 Martin Jambor <mjambor@suse.cz> > > > > > > * ipa-cp.c (ipcp_verify_propagated_values): Also dump symtab and > > > mention gcc_unreachable before failing. > > > * ipa.c (symtab_remove_unreachable_nodes): Also print order of > > > removed symbols. > > > > > > Index: src/gcc/ipa-cp.c > > > =================================================================== > > > --- src.orig/gcc/ipa-cp.c > > > +++ src/gcc/ipa-cp.c > > > @@ -884,8 +884,9 @@ ipcp_verify_propagated_values (void) > > > { > > > if (dump_file) > > > { > > > + dump_symtab (dump_file); > > > fprintf (dump_file, "\nIPA lattices after constant " > > > - "propagation:\n"); > > > + "propagation, before gcc_unreachable:\n"); > > This means before symtab_remove_unreachable_nodes? No, there is litrally a call to gcc_unreachable just below this dumping. I added this to grep for it easily when I have a number of dumps lying around because there is the same string in normal dumps too. Thanks, Martin
Index: src/gcc/ipa-cp.c =================================================================== --- src.orig/gcc/ipa-cp.c +++ src/gcc/ipa-cp.c @@ -884,8 +884,9 @@ ipcp_verify_propagated_values (void) { if (dump_file) { + dump_symtab (dump_file); fprintf (dump_file, "\nIPA lattices after constant " - "propagation:\n"); + "propagation, before gcc_unreachable:\n"); print_all_lattices (dump_file, true, false); } Index: src/gcc/ipa.c =================================================================== --- src.orig/gcc/ipa.c +++ src/gcc/ipa.c @@ -469,7 +469,7 @@ symtab_remove_unreachable_nodes (bool be if (!node->aux) { if (file) - fprintf (file, " %s", node->name ()); + fprintf (file, " %s/%i", node->name (), node->order); cgraph_remove_node (node); changed = true; } @@ -483,7 +483,7 @@ symtab_remove_unreachable_nodes (bool be if (node->definition) { if (file) - fprintf (file, " %s", node->name ()); + fprintf (file, " %s/%i", node->name (), node->order); node->body_removed = true; node->analyzed = false; node->definition = false; @@ -531,7 +531,7 @@ symtab_remove_unreachable_nodes (bool be && (!flag_ltrans || !DECL_EXTERNAL (vnode->decl))) { if (file) - fprintf (file, " %s", vnode->name ()); + fprintf (file, " %s/%i", vnode->name (), vnode->order); varpool_remove_node (vnode); changed = true; }