Patchwork [PR,55264] Do not remove as unreachable any virtual methods before inlining

login
register
mail settings
Submitter Martin Jambor
Date Jan. 18, 2013, 5:47 p.m.
Message ID <20130118174750.GD26626@virgil.suse>
Download mbox | patch
Permalink /patch/213698/
State New
Headers show

Comments

Martin Jambor - Jan. 18, 2013, 5:47 p.m.
Hi,

On Wed, Jan 16, 2013 at 10:24:10PM +0100, Jan Hubicka wrote:
> > On Wed, Jan 16, 2013 at 01:44:20PM +0100, Jan Hubicka wrote:
> > > > Perhaps could you first change cgraph_non_local_node_p_1 and try to check some code
> > > > if codegen differs significantly? It should not at all.
> > > > ipa-cp is the sole user of this flag in IPA passes, so you should know what it does.
> > > 
> > > Thinking deeper of ipa-cp and local virtuals, I think this is all slipperly.
> > > Local means that all calls to the functions are explicit and known. Obviously
> > > if function is virutal and new calls may appear by devirtualization, the local
> > > flag is bogus.  I guess the external functions are the only that may be local
> > > and virtual because somewhere there must be a vtable reference, but to play
> > > safe, I would suggest marking all virtuals non-local.
> > > 
> > 
> > Right, as discussed on IRC, the patch below therfore modifies
> > cgraph_only_called_directly_or_aliased_p to return false for virtual
> > functions (which translates into cleared local flag) and the cloning
> > machinery to clear that flag.
> > 
> > Bootstrapped and tested on x86_64-linux without any problems.  OK for
> > trunk?
> OK, thanks!
> 
> Honza

Great, thanks.  I will therefore also commit the following equivalent
to the 4.6 branch on Monday unless someone objects.  It passes
bootstrap and testsuite without any issues, the differences are very
small in both backports (and also compared to the trunk version).

Thanks,

Martin


2013-01-17  Martin Jambor  <mjambor@suse.cz>

        PR tree-optimizations/55264
	* cgraph.c (cgraph_create_virtual_clone): Mark clones as non-virtual.
	* cgraph.h (cgraph_only_called_directly_p): Return false for virtual
	functions.
	* ipa-inline.c (cgraph_clone_inlined_nodes): Do reuse nodes of any
	virtual function.
	* ipa.c (cgraph_remove_unreachable_nodes): Never return true for
	virtual methods before inlining is over.

testsuite/
	* g++.dg/ipa/pr55264.C: New test.

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index d62674a..201e77d 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2342,6 +2342,7 @@  cgraph_create_virtual_clone (struct cgraph_node *old_node,
   TREE_PUBLIC (new_node->decl) = 0;
   DECL_COMDAT (new_node->decl) = 0;
   DECL_WEAK (new_node->decl) = 0;
+  DECL_VIRTUAL_P (new_node->decl) = 0;
   new_node->clone.tree_map = tree_map;
   new_node->clone.args_to_skip = args_to_skip;
   FOR_EACH_VEC_ELT (ipa_replace_map_p, tree_map, i, map)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index c83b4d3..bad1bb9 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -912,6 +912,7 @@  cgraph_only_called_directly_p (struct cgraph_node *node)
   gcc_assert (!node->global.inlined_to);
   return (!node->needed && !node->address_taken
 	  && !node->reachable_from_other_partition
+	  && !DECL_VIRTUAL_P (node->decl)
 	  && !DECL_STATIC_CONSTRUCTOR (node->decl)
 	  && !DECL_STATIC_DESTRUCTOR (node->decl)
 	  && !node->local.externally_visible);
diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 62e1610..3fc796a 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -243,8 +243,7 @@  cgraph_clone_inlined_nodes (struct cgraph_edge *e, bool duplicate,
 	     those only after all devirtualizable virtual calls are processed.
 	     Lacking may edges in callgraph we just preserve them post
 	     inlining.  */
-	  && (!DECL_VIRTUAL_P (e->callee->decl)
-	      || (!DECL_COMDAT (e->callee->decl) && !DECL_EXTERNAL (e->callee->decl)))
+	  && !DECL_VIRTUAL_P (e->callee->decl)
 	  /* Don't reuse if more than one function shares a comdat group.
 	     If the other function(s) are needed, we need to emit even
 	     this function out of line.  */
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 4955408..ade8706 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -235,9 +235,7 @@  cgraph_remove_unreachable_nodes (bool before_inlining_p, FILE *file)
     if (node->analyzed && !node->global.inlined_to
 	&& (!cgraph_can_remove_if_no_direct_calls_and_refs_p (node)
 	    /* Keep around virtual functions for possible devirtualization.  */
-	    || (before_inlining_p
-		&& DECL_VIRTUAL_P (node->decl)
-		&& (DECL_COMDAT (node->decl) || DECL_EXTERNAL (node->decl)))
+	    || (before_inlining_p && DECL_VIRTUAL_P (node->decl))
 	    /* Also external functions with address taken are better to stay
 	       for indirect inlining.  */
 	    || (before_inlining_p
diff --git a/gcc/testsuite/g++.dg/ipa/pr55264.C b/gcc/testsuite/g++.dg/ipa/pr55264.C
new file mode 100644
index 0000000..cf54d6a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr55264.C
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-early-inlining -fno-weak"  } */
+
+struct S
+{
+  S();
+  virtual inline void foo ()
+  {
+    foo();
+  }
+};
+
+void
+B ()
+{
+  S().foo ();
+}