This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [tree-profiling] fix some side corners of finalization machinery


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)
{




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]