Hi,
On Fri, Mar 21, 2014 at 09:40:39PM +0100, Jan Hubicka wrote:
> > On Thu, 20 Mar 2014, Martin Jambor wrote:
> >
> > > Hi,
> > >
> > > On Thu, Mar 20, 2014 at 07:40:56PM +0100, Jakub Jelinek wrote:
> > > > On Thu, Mar 20, 2014 at 05:07:32PM +0100, Martin Jambor wrote:
> > > > > in the PR, verifier claims an edge is pointing to a wrong declaration
> > > > > even though it has successfully verified the edge multiple times
> > > > > before. The reason is that symtab_remove_unreachable_nodes decides to
> > > > > "remove the body" of a node and also clear any information that it is
> > > > > an alias of another in the process (more detailed analysis in comment
> > > > > #9 of the bug).
> > > > >
> > > > > In bugzilla Honza wrote that "silencing the verifier" is the way to
> > > > > go. Either we can dedicate a new flag in each cgraph_node or
> > > > > symtab_node just for the purpose of verification or do something more
> > > > > hackish like the patch below which re-uses the former_clone_of field
> > > > > for this purpose. Since clones are always private nodes, they should
> > > > > always either survive removal of unreachable nodes or be completely
> > > > > killed by it and should never enter the in_border zombie state.
> > > > > Therefore their former_clone_of must always be NULL. So I added a new
> > > > > special value, error_mark_node, to mark this zombie state and taught
> > > > > the verifier to be happy with such nodes.
> > > > >
> > > > > Bootstrapped and tested on x86_64-linux. What do you think?
> > > >
> > > > Don't we have like 22 spare bits in cgraph_node and 20 spare bits in
> > > > symtab_node? I'd find it clearer if you just used a new flag to mark
> > > > the
> > > > zombie nodes. Though, I'll let Richard or Honza to decide, don't feel
> > > > strongly about it.
> > > >
> > >
> > > I guess you are right, here is the proper version which is currently
> > > undergoing bootstrap and testing.
> >
> > I agree with Jakub, the following variant is ok.
>
> With the extra bit, you probably will need to LTO pickle it, too.
Oops, that omission is a mistake. I'd like to blame it on quilt but
it is more probable I simply forgot. I will commit the patch below as
obvious after it passes a bootstrap.
> I would go with just clerning the thunk flag: this makes thunk to behave like
> external function that is safe to do.
No, no thunks are involved here, it is an alias and the alias flag is
cleared and was even before I fixed PR 60419. The problem is exactly
that after symtab_remove_unreachable_nodes clears the flag (and the
associated reference), the verifier sees just an ordinary symbol, now
seemingly unrelated to the edge destination (or the node it is cloned
from).
> (I am back in civilization from Alaska camping, will catch up with email
> early next week)
Well, if camping in Alaska was not punishable by sever email backlog,
life would not be fair at all.
Thanks,
Martin
2014-03-24 Martin Jambor <mjam...@suse.cz>
PR ipa/59176
* lto-cgraph.c (lto_output_node): Stream body_removed flag.
(lto_output_varpool_node): Likewise.
(input_overwrite_node): Likewise.
(input_varpool_node): Likewise.
Index: src/gcc/lto-cgraph.c
===================================================================
--- src.orig/gcc/lto-cgraph.c
+++ src/gcc/lto-cgraph.c
@@ -500,6 +500,7 @@ lto_output_node (struct lto_simple_outpu
bp_pack_value (&bp, node->force_output, 1);
bp_pack_value (&bp, node->forced_by_abi, 1);
bp_pack_value (&bp, node->unique_name, 1);
+ bp_pack_value (&bp, node->body_removed, 1);
bp_pack_value (&bp, node->address_taken, 1);
bp_pack_value (&bp, tag == LTO_symtab_analyzed_node
&& symtab_get_symbol_partitioning_class (node) ==
SYMBOL_PARTITION
@@ -560,6 +561,7 @@ lto_output_varpool_node (struct lto_simp
bp_pack_value (&bp, node->force_output, 1);
bp_pack_value (&bp, node->forced_by_abi, 1);
bp_pack_value (&bp, node->unique_name, 1);
+ bp_pack_value (&bp, node->body_removed, 1);
bp_pack_value (&bp, node->definition, 1);
alias_p = node->alias && (!boundary_p || node->weakref);
bp_pack_value (&bp, alias_p, 1);
@@ -969,6 +971,7 @@ input_overwrite_node (struct lto_file_de
node->force_output = bp_unpack_value (bp, 1);
node->forced_by_abi = bp_unpack_value (bp, 1);
node->unique_name = bp_unpack_value (bp, 1);
+ node->body_removed = bp_unpack_value (bp, 1);
node->address_taken = bp_unpack_value (bp, 1);
node->used_from_other_partition = bp_unpack_value (bp, 1);
node->lowered = bp_unpack_value (bp, 1);
@@ -1147,6 +1150,7 @@ input_varpool_node (struct lto_file_decl
node->force_output = bp_unpack_value (&bp, 1);
node->forced_by_abi = bp_unpack_value (&bp, 1);
node->unique_name = bp_unpack_value (&bp, 1);
+ node->body_removed = bp_unpack_value (&bp, 1);
node->definition = bp_unpack_value (&bp, 1);
node->alias = bp_unpack_value (&bp, 1);
node->weakref = bp_unpack_value (&bp, 1);
