| Message ID | 20110428133118.GA20409@virgil.arch.suse.de |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 28, 2011 at 3:31 PM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > > when I was removing the cgraph_node function I noticed that > cgraph_preserve_function_body_p takes a tree decl parameter which it > immediately converts to a call graph node while its two callers get > the decl from a node they already have. This seems wasteful and so > the patch below changes the parameter to be a cgraph_node which avoids > the lookup. > > Because cgraph_get_node(node->decl) may be a way of getting the actual > node for a same body alias (a strange way, but well...), I added a > test for same body aliases to the function too. > > I have bootstrapped and tested the patch on x86_64-linux without > problems. OK for trunk? An alias shouldn't have a body, so why not assert !node->same_body_alias instead.? Richard. > Thanks, > > Martin > > > 2011-04-22 Martin Jambor <mjambor@suse.cz> > > * cgraphunit.c (cgraph_preserve_function_body_p): Accept a cgraph > node instead of a decl. Update all callers. > * cgraph.h: Update declaration. > > Index: src/gcc/cgraphunit.c > =================================================================== > --- src.orig/gcc/cgraphunit.c > +++ src/gcc/cgraphunit.c > @@ -1578,7 +1578,7 @@ cgraph_expand_function (struct cgraph_no > /* Make sure that BE didn't give up on compiling. */ > gcc_assert (TREE_ASM_WRITTEN (decl)); > current_function_decl = NULL; > - gcc_assert (!cgraph_preserve_function_body_p (decl)); > + gcc_assert (!cgraph_preserve_function_body_p (node)); > cgraph_release_function_body (node); > /* Eliminate all call edges. This is important so the GIMPLE_CALL no longer > points to the dead function body. */ > @@ -1756,13 +1756,12 @@ cgraph_output_in_order (void) > /* Return true when function body of DECL still needs to be kept around > for later re-use. */ > bool > -cgraph_preserve_function_body_p (tree decl) > +cgraph_preserve_function_body_p (struct cgraph_node *node) > { > - struct cgraph_node *node; > - > gcc_assert (cgraph_global_info_ready); > + if (node->same_body_alias) > + node = node->same_body; > /* Look if there is any clone around. */ > - node = cgraph_get_node (decl); > if (node->clones) > return true; > return false; > Index: src/gcc/ipa-inline-transform.c > =================================================================== > --- src.orig/gcc/ipa-inline-transform.c > +++ src/gcc/ipa-inline-transform.c > @@ -307,7 +307,7 @@ inline_transform (struct cgraph_node *no > > /* We might need the body of this function so that we can expand > it inline somewhere else. */ > - if (cgraph_preserve_function_body_p (node->decl)) > + if (cgraph_preserve_function_body_p (node)) > save_inline_function_body (node); > > for (e = node->callees; e; e = e->next_callee) > Index: src/gcc/cgraph.h > =================================================================== > --- src.orig/gcc/cgraph.h > +++ src/gcc/cgraph.h > @@ -578,7 +578,7 @@ void cgraph_mark_needed_node (struct cgr > void cgraph_mark_address_taken_node (struct cgraph_node *); > void cgraph_mark_reachable_node (struct cgraph_node *); > bool cgraph_inline_p (struct cgraph_edge *, cgraph_inline_failed_t *reason); > -bool cgraph_preserve_function_body_p (tree); > +bool cgraph_preserve_function_body_p (struct cgraph_node *); > void verify_cgraph (void); > void verify_cgraph_node (struct cgraph_node *); > void cgraph_build_static_cdtor (char which, tree body, int priority); > >
Hi, On Thu, Apr 28, 2011 at 03:39:11PM +0200, Richard Guenther wrote: > On Thu, Apr 28, 2011 at 3:31 PM, Martin Jambor <mjambor@suse.cz> wrote: > > Hi, > > > > when I was removing the cgraph_node function I noticed that > > cgraph_preserve_function_body_p takes a tree decl parameter which it > > immediately converts to a call graph node while its two callers get > > the decl from a node they already have. This seems wasteful and so > > the patch below changes the parameter to be a cgraph_node which avoids > > the lookup. > > > > Because cgraph_get_node(node->decl) may be a way of getting the actual > > node for a same body alias (a strange way, but well...), I added a > > test for same body aliases to the function too. > > > > I have bootstrapped and tested the patch on x86_64-linux without > > problems. OK for trunk? > > An alias shouldn't have a body, so why not assert !node->same_body_alias > instead.? That would work as well and actually might be better, it just did not occur to me but I did not think about this very deeply. I'm about to bootstrap a version of the patch with an assert instead. Thanks, Martin > > Richard. > > > Thanks, > > > > Martin > > > > > > 2011-04-22 Martin Jambor <mjambor@suse.cz> > > > > * cgraphunit.c (cgraph_preserve_function_body_p): Accept a cgraph > > node instead of a decl. Update all callers. > > * cgraph.h: Update declaration. > > > > Index: src/gcc/cgraphunit.c > > =================================================================== > > --- src.orig/gcc/cgraphunit.c > > +++ src/gcc/cgraphunit.c > > @@ -1578,7 +1578,7 @@ cgraph_expand_function (struct cgraph_no > > /* Make sure that BE didn't give up on compiling. */ > > gcc_assert (TREE_ASM_WRITTEN (decl)); > > current_function_decl = NULL; > > - gcc_assert (!cgraph_preserve_function_body_p (decl)); > > + gcc_assert (!cgraph_preserve_function_body_p (node)); > > cgraph_release_function_body (node); > > /* Eliminate all call edges. This is important so the GIMPLE_CALL no longer > > points to the dead function body. */ > > @@ -1756,13 +1756,12 @@ cgraph_output_in_order (void) > > /* Return true when function body of DECL still needs to be kept around > > for later re-use. */ > > bool > > -cgraph_preserve_function_body_p (tree decl) > > +cgraph_preserve_function_body_p (struct cgraph_node *node) > > { > > - struct cgraph_node *node; > > - > > gcc_assert (cgraph_global_info_ready); > > + if (node->same_body_alias) > > + node = node->same_body; > > /* Look if there is any clone around. */ > > - node = cgraph_get_node (decl); > > if (node->clones) > > return true; > > return false; > > Index: src/gcc/ipa-inline-transform.c > > =================================================================== > > --- src.orig/gcc/ipa-inline-transform.c > > +++ src/gcc/ipa-inline-transform.c > > @@ -307,7 +307,7 @@ inline_transform (struct cgraph_node *no > > > > /* We might need the body of this function so that we can expand > > it inline somewhere else. */ > > - if (cgraph_preserve_function_body_p (node->decl)) > > + if (cgraph_preserve_function_body_p (node)) > > save_inline_function_body (node); > > > > for (e = node->callees; e; e = e->next_callee) > > Index: src/gcc/cgraph.h > > =================================================================== > > --- src.orig/gcc/cgraph.h > > +++ src/gcc/cgraph.h > > @@ -578,7 +578,7 @@ void cgraph_mark_needed_node (struct cgr > > void cgraph_mark_address_taken_node (struct cgraph_node *); > > void cgraph_mark_reachable_node (struct cgraph_node *); > > bool cgraph_inline_p (struct cgraph_edge *, cgraph_inline_failed_t *reason); > > -bool cgraph_preserve_function_body_p (tree); > > +bool cgraph_preserve_function_body_p (struct cgraph_node *); > > void verify_cgraph (void); > > void verify_cgraph_node (struct cgraph_node *); > > void cgraph_build_static_cdtor (char which, tree body, int priority); > > > >
> > I have bootstrapped and tested the patch on x86_64-linux without > > problems. OK for trunk? > > An alias shouldn't have a body, so why not assert !node->same_body_alias > instead.? Yep, patch is OK with that change. Honza
Index: src/gcc/cgraphunit.c =================================================================== --- src.orig/gcc/cgraphunit.c +++ src/gcc/cgraphunit.c @@ -1578,7 +1578,7 @@ cgraph_expand_function (struct cgraph_no /* Make sure that BE didn't give up on compiling. */ gcc_assert (TREE_ASM_WRITTEN (decl)); current_function_decl = NULL; - gcc_assert (!cgraph_preserve_function_body_p (decl)); + gcc_assert (!cgraph_preserve_function_body_p (node)); cgraph_release_function_body (node); /* Eliminate all call edges. This is important so the GIMPLE_CALL no longer points to the dead function body. */ @@ -1756,13 +1756,12 @@ cgraph_output_in_order (void) /* Return true when function body of DECL still needs to be kept around for later re-use. */ bool -cgraph_preserve_function_body_p (tree decl) +cgraph_preserve_function_body_p (struct cgraph_node *node) { - struct cgraph_node *node; - gcc_assert (cgraph_global_info_ready); + if (node->same_body_alias) + node = node->same_body; /* Look if there is any clone around. */ - node = cgraph_get_node (decl); if (node->clones) return true; return false; Index: src/gcc/ipa-inline-transform.c =================================================================== --- src.orig/gcc/ipa-inline-transform.c +++ src/gcc/ipa-inline-transform.c @@ -307,7 +307,7 @@ inline_transform (struct cgraph_node *no /* We might need the body of this function so that we can expand it inline somewhere else. */ - if (cgraph_preserve_function_body_p (node->decl)) + if (cgraph_preserve_function_body_p (node)) save_inline_function_body (node); for (e = node->callees; e; e = e->next_callee) Index: src/gcc/cgraph.h =================================================================== --- src.orig/gcc/cgraph.h +++ src/gcc/cgraph.h @@ -578,7 +578,7 @@ void cgraph_mark_needed_node (struct cgr void cgraph_mark_address_taken_node (struct cgraph_node *); void cgraph_mark_reachable_node (struct cgraph_node *); bool cgraph_inline_p (struct cgraph_edge *, cgraph_inline_failed_t *reason); -bool cgraph_preserve_function_body_p (tree); +bool cgraph_preserve_function_body_p (struct cgraph_node *); void verify_cgraph (void); void verify_cgraph_node (struct cgraph_node *); void cgraph_build_static_cdtor (char which, tree body, int priority);