[tree-profiling] fix some side corners of finalization machinery
Kenneth Zadeck
zadeck@naturalbridge.com
Fri Oct 29 15:33:00 GMT 2004
Jan,
I think that I (actually you) found the problem. I added the code to
copy my info that you suggested to the clone routine.
The problem went away. It seems that the code that I had in the
remove_cgraph_node was only triggered if the master_clone had been set.
I also added code to set the master_clone to cgraph_node and to
clone_cgraph_node.
Yan I will send you this patch when I am finished testing it. I also
needed to add code so that I only look at nodes that have been either
finalized, external or externally_visible since I hit an orphan one of
the in the java libraries.
kenny
Jan Hubicka wrote:
>Hi,
>this patch adds sanity checking that verify that flags works as expected
>(ie that finalize bit means that either the function or it's original
>clone has been passed to the analyze hooks of IPA passes). We've run
>across possible problems here with kennet's new aliasing code.
>Unfortunately the code don't find bug in C++ testcase he is considering,
>but it found bug elsewhere. For the testcase:
>
>extern inline int foo (void) { return 23; }
>int xxx(void) __asm__("xxx");
>int xxx(void) { return 23; }
>extern int foo (void) __attribute__ ((weak, alias ("xxx")));
>
>frontend first construct function foo but later takes it back and replace by
>the alias to foo. This is arguably broken semantics from frontend, but we are
>somewhat loose in the cases of extern inline semantics anyway to allow glibc to
>compile. I've worked around by making cgraph code to work harder to remove
>tracks of the lost function.
>
>I am testing the patch and will commit it to tree-profiling once it passes.
>I will now try Kenneth's patch and figure out what goes wrong directly
>;)
>
>Honza
>
>2004-10-29 Jan Hubicka <jh@suse.cz>
> * cgraphunit.c (cgraph_reset_node): Break out from ...
> (cgraph_finalize_function): ... here.
> (cgraph_finalize_compilation_unit): Prune out finalized functions
> that has been taken back by frontend (ugh ugh); add sanity checking
>Index: cgraphunit.c
>===================================================================
>RCS file: /cvs/gcc/gcc/gcc/cgraphunit.c,v
>retrieving revision 1.1.4.35.2.22
>diff -c -3 -p -r1.1.4.35.2.22 cgraphunit.c
>*** cgraphunit.c 26 Oct 2004 14:06:51 -0000 1.1.4.35.2.22
>--- cgraphunit.c 29 Oct 2004 13:11:10 -0000
>*************** cgraph_analyze_function_inlinability (st
>*** 353,358 ****
>--- 353,402 ----
> node->global.insns = node->local.self_insns;
> }
>
>+ /* As an GCC extension we allow redefinition of the function. The
>+ semantics when both copies of bodies differ is not well defined.
>+ We replace the old body with new body so in unit at a time mode
>+ we always use new body, while in normal mode we may end up with
>+ old body inlined into some functions and new body expanded and
>+ inlined in others.
>+
>+ ??? It may make more sense to use one body for inlining and other
>+ body for expanding the function but this is difficult to do. */
>+
>+ static void
>+ cgraph_reset_node (struct cgraph_node *node)
>+ {
>+ /* If node->output is set, then this is a unit-at-a-time compilation
>+ and we have already begun whole-unit analysis. This is *not*
>+ testing for whether we've already emitted the function. That
>+ case can be sort-of legitimately seen with real function
>+ redefinition errors. I would argue that the front end should
>+ never present us with such a case, but don't enforce that for now. */
>+ gcc_assert (!node->output);
>+
>+ /* Reset our data structures so we can analyze the function again. */
>+ memset (&node->local, 0, sizeof (node->local));
>+ memset (&node->global, 0, sizeof (node->global));
>+ memset (&node->rtl, 0, sizeof (node->rtl));
>+ node->analyzed = node->local.finalized = false;
>+ node->local.redefined_extern_inline = true;
>+ while (node->callees)
>+ cgraph_remove_edge (node->callees);
>+
>+ /* We may need to re-queue the node for assembling in case
>+ we already proceeded it and ignored as not needed. */
>+ if (node->reachable && !flag_unit_at_a_time)
>+ {
>+ struct cgraph_node *n;
>+
>+ for (n = cgraph_nodes_queue; n; n = n->next_needed)
>+ if (n == node)
>+ break;
>+ if (!n)
>+ node->reachable = 0;
>+ }
>+ }
>+
> /* DECL has been parsed. Take it, queue it, compile it at the whim of the
> logic in effect. If NESTED is true, then our caller cannot stand to have
> the garbage collector run at the moment. We would need to either create
>*************** cgraph_finalize_function (tree decl, boo
>*** 364,410 ****
> struct cgraph_node *node = cgraph_node (decl);
>
> if (node->local.finalized)
>! {
>! /* As an GCC extension we allow redefinition of the function. The
>! semantics when both copies of bodies differ is not well defined.
>! We replace the old body with new body so in unit at a time mode
>! we always use new body, while in normal mode we may end up with
>! old body inlined into some functions and new body expanded and
>! inlined in others.
>!
>! ??? It may make more sense to use one body for inlining and other
>! body for expanding the function but this is difficult to do. */
>!
>! /* If node->output is set, then this is a unit-at-a-time compilation
>! and we have already begun whole-unit analysis. This is *not*
>! testing for whether we've already emitted the function. That
>! case can be sort-of legitimately seen with real function
>! redefinition errors. I would argue that the front end should
>! never present us with such a case, but don't enforce that for now. */
>! gcc_assert (!node->output);
>!
>! /* Reset our data structures so we can analyze the function again. */
>! memset (&node->local, 0, sizeof (node->local));
>! memset (&node->global, 0, sizeof (node->global));
>! memset (&node->rtl, 0, sizeof (node->rtl));
>! node->analyzed = false;
>! node->local.redefined_extern_inline = true;
>! while (node->callees)
>! cgraph_remove_edge (node->callees);
>!
>! /* We may need to re-queue the node for assembling in case
>! we already proceeded it and ignored as not needed. */
>! if (node->reachable && !flag_unit_at_a_time)
>! {
>! struct cgraph_node *n;
>!
>! for (n = cgraph_nodes_queue; n; n = n->next_needed)
>! if (n == node)
>! break;
>! if (!n)
>! node->reachable = 0;
>! }
>! }
>
> notice_global_symbol (decl);
> node->decl = decl;
>--- 408,414 ----
> struct cgraph_node *node = cgraph_node (decl);
>
> if (node->local.finalized)
>! cgraph_reset_node (node);
>
> notice_global_symbol (decl);
> node->decl = decl;
>*************** cgraph_finalize_compilation_unit (void)
>*** 798,804 ****
> weak alas attribute to kill its body. See
> gcc.c-torture/compile/20011119-1.c */
> if (!DECL_SAVED_TREE (decl))
>! continue;
>
> gcc_assert (!node->analyzed && node->reachable);
> gcc_assert (DECL_SAVED_TREE (decl));
>--- 802,811 ----
> weak alas attribute to kill its body. See
> gcc.c-torture/compile/20011119-1.c */
> if (!DECL_SAVED_TREE (decl))
>! {
>! cgraph_reset_node (node);
>! continue;
>! }
>
> gcc_assert (!node->analyzed && node->reachable);
> gcc_assert (DECL_SAVED_TREE (decl));
>*************** cgraph_finalize_compilation_unit (void)
>*** 831,844 ****
> {
> tree decl = node->decl;
>
>! if (!node->reachable && DECL_SAVED_TREE (decl))
> {
> if (cgraph_dump_file)
> fprintf (cgraph_dump_file, " %s", cgraph_node_name (node));
> cgraph_remove_node (node);
> }
> else
>! node->next_needed = NULL;
> }
> if (cgraph_dump_file)
> {
>--- 838,861 ----
> {
> tree decl = node->decl;
>
>! if (node->local.finalized && !DECL_SAVED_TREE (decl))
>! cgraph_reset_node (node);
>!
>! if (!node->reachable && node->local.finalized)
> {
> if (cgraph_dump_file)
> fprintf (cgraph_dump_file, " %s", cgraph_node_name (node));
> cgraph_remove_node (node);
>+ continue;
> }
> else
>! {
>! node->next_needed = NULL;
>! if (!node->local.finalized)
>! DECL_SAVED_TREE (decl) = NULL;
>! }
>! gcc_assert (!node->local.finalized || DECL_SAVED_TREE (decl));
>! gcc_assert (node->analyzed == node->local.finalized);
> }
> if (cgraph_dump_file)
> {
>
>
More information about the Gcc-patches
mailing list