diff mbox

[PR,46823] Redirect calls to aliases according to cgraph_edges too

Message ID 20110112230433.GA27901@virgil.arch.suse.de
State New
Headers show

Commit Message

Martin Jambor Jan. 12, 2011, 11:04 p.m. UTC
Hi,

the problem in PR 46823 is that because
cgraph_redirect_edge_call_stmt_to_callee tries to avoid seemingly
unnecessary work of redirecting a call to a same_body_alias, we have
stray same_body_alias function declarations still lurking in the IL
for which then cgraph_node() function lazily (and wrongly) creates new
nodes which triggers a call graph verifier failure.  More details on
the exact mechanism are in bugzilla.

This can be fixed either by simply redirecting these calls to the
callee declarations of the real nodes too or hacked around in
tree-inline (patch doing that is in bugzilla too).  Nevertheless, the
always redirecting approach is more principled (it's really better to
do the redirections at fewest places possible) and the overhead should
not be that big.

Unfortunately, so far I have not had much luck with creating a
standalone, small testcase.

Nevertheless, I have bootstrapped and tested the patch below on
x86_64-linx.  Is OK for trunk?

Thanks,

Martin


2011-01-12  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/46823
	* cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Update also
	calls aliases.

Comments

Jan Hubicka Jan. 13, 2011, 4 p.m. UTC | #1
> Hi,
> 
> the problem in PR 46823 is that because
> cgraph_redirect_edge_call_stmt_to_callee tries to avoid seemingly
> unnecessary work of redirecting a call to a same_body_alias, we have
> stray same_body_alias function declarations still lurking in the IL
> for which then cgraph_node() function lazily (and wrongly) creates new
> nodes which triggers a call graph verifier failure.  More details on
> the exact mechanism are in bugzilla.
> 
> This can be fixed either by simply redirecting these calls to the
> callee declarations of the real nodes too or hacked around in
> tree-inline (patch doing that is in bugzilla too).  Nevertheless, the
> always redirecting approach is more principled (it's really better to
> do the redirections at fewest places possible) and the overhead should
> not be that big.
> 
> Unfortunately, so far I have not had much luck with creating a
> standalone, small testcase.
> 
> Nevertheless, I have bootstrapped and tested the patch below on
> x86_64-linx.  Is OK for trunk?

I know that Jakub was against redirecting to same body aliases to avoid user confussion
in calling function A and getting call to function B.

So to be sure that I understand the problem

1) we have call to function with same body alias
2) we redirect the call to a clone
3) we remove the original from callgraph because it is unreachable
4) we inline the function containing the redirected call and still have it pointing to the original alias, but not to the redirected node
5) we get confused because verifier is called when the call to the unreachable function is still in IL and gets confused by cgraph_get_node.
right?

Can't we just defer verification until after inliner is done with redirection?

Honza
diff mbox

Patch

Index: icln/gcc/cgraphunit.c
===================================================================
--- icln.orig/gcc/cgraphunit.c
+++ icln/gcc/cgraphunit.c
@@ -2155,9 +2155,7 @@  cgraph_redirect_edge_call_stmt_to_callee
 #endif
 
   if (e->indirect_unknown_callee
-      || decl == e->callee->decl
-      /* Don't update call from same body alias to the real function.  */
-      || (decl && cgraph_get_node (decl) == cgraph_get_node (e->callee->decl)))
+      || decl == e->callee->decl)
     return e->call_stmt;
 
 #ifdef ENABLE_CHECKING