* [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
@ 2013-01-16 10:01 Martin Jambor
2013-01-16 10:30 ` Richard Biener
0 siblings, 1 reply; 8+ messages in thread
From: Martin Jambor @ 2013-01-16 10:01 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
Hi,
PR 55264 is caused by cgraph machinery thinking it knows all calls to
a virtual method when that is actually not true. As discussed with
Honza, prior to inlining, we should not assume some virtual functions
(namely those that are neither DECL_COMDAT nor DECL_EXTERNAL) are not
reachable.
DECL_EXTERNAL however still affects the cgraph_node->local.local flag
and so in order to avoid some LTO failures, I had to adjust IPA-CP to
consider such virtual functions non-local so that verification of
lattice propagation does not complain. I'm a bit puzzled by the value
of the flag in this situation but at least it does not seem to cause
any other problems.
Below is a trunk patch to do all this. It does not apply to released
versions which also suffer from the same problem so I'll prepare a
special version(s) for them once this gets approved. Bootstrapped and
tested on x86_64-linux. OK for trunk?
Thanks,
Martin
2013-01-15 Martin Jambor <mjambor@suse.cz>
PR tree-optimizations/55264
* ipa-inline-transform.c (can_remove_node_now_p_1): Never return true
for virtual methods.
* ipa.c (symtab_remove_unreachable_nodes): Never return true for
virtual methods before inlining is over.
* ipa-cp.c (initialize_node_lattices): Also consider all virtual
methods non-local.
testsuite/
* g++.dg/ipa/pr55264.C: New test.
Index: src/gcc/ipa-inline-transform.c
===================================================================
--- src.orig/gcc/ipa-inline-transform.c
+++ src/gcc/ipa-inline-transform.c
@@ -92,9 +92,7 @@ can_remove_node_now_p_1 (struct cgraph_n
those only after all devirtualizable virtual calls are processed.
Lacking may edges in callgraph we just preserve them post
inlining. */
- && (!DECL_VIRTUAL_P (node->symbol.decl)
- || (!DECL_COMDAT (node->symbol.decl)
- && !DECL_EXTERNAL (node->symbol.decl)))
+ && !DECL_VIRTUAL_P (node->symbol.decl)
/* During early inlining some unanalyzed cgraph nodes might be in the
callgraph and they might reffer the function in question. */
&& !cgraph_new_nodes);
Index: src/gcc/ipa.c
===================================================================
--- src.orig/gcc/ipa.c
+++ src/gcc/ipa.c
@@ -241,8 +241,7 @@ symtab_remove_unreachable_nodes (bool be
&& (!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->symbol.decl)
- && (DECL_COMDAT (node->symbol.decl) || DECL_EXTERNAL (node->symbol.decl)))))
+ && DECL_VIRTUAL_P (node->symbol.decl))))
{
gcc_assert (!node->global.inlined_to);
pointer_set_insert (reachable, node);
Index: src/gcc/testsuite/g++.dg/ipa/pr55264.C
===================================================================
--- /dev/null
+++ src/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 ();
+}
Index: src/gcc/ipa-cp.c
===================================================================
--- src.orig/gcc/ipa-cp.c
+++ src/gcc/ipa-cp.c
@@ -699,7 +699,8 @@ initialize_node_lattices (struct cgraph_
int i;
gcc_checking_assert (cgraph_function_with_gimple_body_p (node));
- if (!node->local.local)
+ if (!node->local.local
+ || DECL_VIRTUAL_P (node->symbol.decl))
{
/* When cloning is allowed, we can assume that externally visible
functions are not called. We will compensate this by cloning
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 10:01 [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining Martin Jambor
@ 2013-01-16 10:30 ` Richard Biener
2013-01-16 11:21 ` Jan Hubicka
0 siblings, 1 reply; 8+ messages in thread
From: Richard Biener @ 2013-01-16 10:30 UTC (permalink / raw)
To: GCC Patches, Jan Hubicka
On Wed, Jan 16, 2013 at 11:01 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> PR 55264 is caused by cgraph machinery thinking it knows all calls to
> a virtual method when that is actually not true. As discussed with
> Honza, prior to inlining, we should not assume some virtual functions
> (namely those that are neither DECL_COMDAT nor DECL_EXTERNAL) are not
> reachable.
Are they not reachable from the VTABLE which is referenced from all
calls that eventually reach the virtual method? Thus, isn't the issue that
the VTABLE is not correctly handled by ipa-references code?
> DECL_EXTERNAL however still affects the cgraph_node->local.local flag
> and so in order to avoid some LTO failures, I had to adjust IPA-CP to
> consider such virtual functions non-local so that verification of
> lattice propagation does not complain. I'm a bit puzzled by the value
> of the flag in this situation but at least it does not seem to cause
> any other problems.
>
> Below is a trunk patch to do all this. It does not apply to released
> versions which also suffer from the same problem so I'll prepare a
> special version(s) for them once this gets approved. Bootstrapped and
> tested on x86_64-linux. OK for trunk?
>
> Thanks,
>
> Martin
>
>
> 2013-01-15 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimizations/55264
> * ipa-inline-transform.c (can_remove_node_now_p_1): Never return true
> for virtual methods.
> * ipa.c (symtab_remove_unreachable_nodes): Never return true for
> virtual methods before inlining is over.
> * ipa-cp.c (initialize_node_lattices): Also consider all virtual
> methods non-local.
>
> testsuite/
> * g++.dg/ipa/pr55264.C: New test.
>
> Index: src/gcc/ipa-inline-transform.c
> ===================================================================
> --- src.orig/gcc/ipa-inline-transform.c
> +++ src/gcc/ipa-inline-transform.c
> @@ -92,9 +92,7 @@ can_remove_node_now_p_1 (struct cgraph_n
> those only after all devirtualizable virtual calls are processed.
> Lacking may edges in callgraph we just preserve them post
> inlining. */
> - && (!DECL_VIRTUAL_P (node->symbol.decl)
> - || (!DECL_COMDAT (node->symbol.decl)
> - && !DECL_EXTERNAL (node->symbol.decl)))
> + && !DECL_VIRTUAL_P (node->symbol.decl)
> /* During early inlining some unanalyzed cgraph nodes might be in the
> callgraph and they might reffer the function in question. */
> && !cgraph_new_nodes);
> Index: src/gcc/ipa.c
> ===================================================================
> --- src.orig/gcc/ipa.c
> +++ src/gcc/ipa.c
> @@ -241,8 +241,7 @@ symtab_remove_unreachable_nodes (bool be
> && (!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->symbol.decl)
> - && (DECL_COMDAT (node->symbol.decl) || DECL_EXTERNAL (node->symbol.decl)))))
> + && DECL_VIRTUAL_P (node->symbol.decl))))
> {
> gcc_assert (!node->global.inlined_to);
> pointer_set_insert (reachable, node);
> Index: src/gcc/testsuite/g++.dg/ipa/pr55264.C
> ===================================================================
> --- /dev/null
> +++ src/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 ();
> +}
> Index: src/gcc/ipa-cp.c
> ===================================================================
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -699,7 +699,8 @@ initialize_node_lattices (struct cgraph_
> int i;
>
> gcc_checking_assert (cgraph_function_with_gimple_body_p (node));
> - if (!node->local.local)
> + if (!node->local.local
> + || DECL_VIRTUAL_P (node->symbol.decl))
> {
> /* When cloning is allowed, we can assume that externally visible
> functions are not called. We will compensate this by cloning
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 10:30 ` Richard Biener
@ 2013-01-16 11:21 ` Jan Hubicka
2013-01-16 12:44 ` Jan Hubicka
0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2013-01-16 11:21 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Jan Hubicka
> On Wed, Jan 16, 2013 at 11:01 AM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > PR 55264 is caused by cgraph machinery thinking it knows all calls to
> > a virtual method when that is actually not true. As discussed with
> > Honza, prior to inlining, we should not assume some virtual functions
> > (namely those that are neither DECL_COMDAT nor DECL_EXTERNAL) are not
> > reachable.
>
> Are they not reachable from the VTABLE which is referenced from all
> calls that eventually reach the virtual method? Thus, isn't the issue that
> the VTABLE is not correctly handled by ipa-references code?
No, the problem is that VTABLE can be keyed to other unit and not present in current
unit at all and devirtualization is possible only via BINFOs. We already handle
COMDAT/EXTERNALs like this.
In longer run, I think we should build "may" edges for virtual calls that will render
the corresponding methods reachable.
>
> > DECL_EXTERNAL however still affects the cgraph_node->local.local flag
> > and so in order to avoid some LTO failures, I had to adjust IPA-CP to
> > consider such virtual functions non-local so that verification of
> > lattice propagation does not complain. I'm a bit puzzled by the value
> > of the flag in this situation but at least it does not seem to cause
> > any other problems.
Hmm, perhaps we could simply set local flag to be false for external functions?
The local flag is mostly used by codegen at a time the external functions are either
inlined or removed, so it is never used in that context.
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.
> >
> > Index: src/gcc/ipa-inline-transform.c
> > ===================================================================
> > --- src.orig/gcc/ipa-inline-transform.c
> > +++ src/gcc/ipa-inline-transform.c
> > @@ -92,9 +92,7 @@ can_remove_node_now_p_1 (struct cgraph_n
> > those only after all devirtualizable virtual calls are processed.
> > Lacking may edges in callgraph we just preserve them post
> > inlining. */
> > - && (!DECL_VIRTUAL_P (node->symbol.decl)
> > - || (!DECL_COMDAT (node->symbol.decl)
> > - && !DECL_EXTERNAL (node->symbol.decl)))
> > + && !DECL_VIRTUAL_P (node->symbol.decl)
> > /* During early inlining some unanalyzed cgraph nodes might be in the
> > callgraph and they might reffer the function in question. */
> > && !cgraph_new_nodes);
> > Index: src/gcc/ipa.c
> > ===================================================================
> > --- src.orig/gcc/ipa.c
> > +++ src/gcc/ipa.c
> > @@ -241,8 +241,7 @@ symtab_remove_unreachable_nodes (bool be
> > && (!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->symbol.decl)
> > - && (DECL_COMDAT (node->symbol.decl) || DECL_EXTERNAL (node->symbol.decl)))))
> > + && DECL_VIRTUAL_P (node->symbol.decl))))
> > {
> > gcc_assert (!node->global.inlined_to);
> > pointer_set_insert (reachable, node);
> > Index: src/gcc/testsuite/g++.dg/ipa/pr55264.C
> > ===================================================================
> > --- /dev/null
> > +++ src/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 ();
> > +}
These changes are OK.
> > Index: src/gcc/ipa-cp.c
> > ===================================================================
> > --- src.orig/gcc/ipa-cp.c
> > +++ src/gcc/ipa-cp.c
> > @@ -699,7 +699,8 @@ initialize_node_lattices (struct cgraph_
> > int i;
> >
> > gcc_checking_assert (cgraph_function_with_gimple_body_p (node));
> > - if (!node->local.local)
> > + if (!node->local.local
> > + || DECL_VIRTUAL_P (node->symbol.decl))
> > {
> > /* When cloning is allowed, we can assume that externally visible
> > functions are not called. We will compensate this by cloning
As mentioned above I would preffer the nonlocal_node_p change. If it passes testing and does not
regress, consider the patch pre-approved.
Honza
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 11:21 ` Jan Hubicka
@ 2013-01-16 12:44 ` Jan Hubicka
2013-01-16 18:42 ` Martin Jambor
0 siblings, 1 reply; 8+ messages in thread
From: Jan Hubicka @ 2013-01-16 12:44 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Richard Biener, GCC Patches
> 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.
Honza
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 12:44 ` Jan Hubicka
@ 2013-01-16 18:42 ` Martin Jambor
2013-01-16 21:24 ` Jan Hubicka
0 siblings, 1 reply; 8+ messages in thread
From: Martin Jambor @ 2013-01-16 18:42 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Richard Biener, GCC Patches
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?
Thanks,
Martin
2013-01-16 Martin Jambor <mjambor@suse.cz>
PR tree-optimizations/55264
* ipa-inline-transform.c (can_remove_node_now_p_1): Never return true
for virtual methods.
* ipa.c (symtab_remove_unreachable_nodes): Never return true for
virtual methods before inlining is over.
* cgraph.h (cgraph_only_called_directly_or_aliased_p): Return false for
virtual functions.
* cgraphclones.c (cgraph_create_virtual_clone): Mark clones as
non-virtual.
testsuite/
* g++.dg/ipa/pr55264.C: New test.
Index: src/gcc/ipa-inline-transform.c
===================================================================
--- src.orig/gcc/ipa-inline-transform.c
+++ src/gcc/ipa-inline-transform.c
@@ -92,9 +92,7 @@ can_remove_node_now_p_1 (struct cgraph_n
those only after all devirtualizable virtual calls are processed.
Lacking may edges in callgraph we just preserve them post
inlining. */
- && (!DECL_VIRTUAL_P (node->symbol.decl)
- || (!DECL_COMDAT (node->symbol.decl)
- && !DECL_EXTERNAL (node->symbol.decl)))
+ && !DECL_VIRTUAL_P (node->symbol.decl)
/* During early inlining some unanalyzed cgraph nodes might be in the
callgraph and they might reffer the function in question. */
&& !cgraph_new_nodes);
Index: src/gcc/ipa.c
===================================================================
--- src.orig/gcc/ipa.c
+++ src/gcc/ipa.c
@@ -241,8 +241,7 @@ symtab_remove_unreachable_nodes (bool be
&& (!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->symbol.decl)
- && (DECL_COMDAT (node->symbol.decl) || DECL_EXTERNAL (node->symbol.decl)))))
+ && DECL_VIRTUAL_P (node->symbol.decl))))
{
gcc_assert (!node->global.inlined_to);
pointer_set_insert (reachable, node);
Index: src/gcc/testsuite/g++.dg/ipa/pr55264.C
===================================================================
--- /dev/null
+++ src/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 ();
+}
Index: src/gcc/cgraph.h
===================================================================
--- src.orig/gcc/cgraph.h
+++ src/gcc/cgraph.h
@@ -1164,6 +1164,7 @@ cgraph_only_called_directly_or_aliased_p
gcc_assert (!node->global.inlined_to);
return (!node->symbol.force_output && !node->symbol.address_taken
&& !node->symbol.used_from_other_partition
+ && !DECL_VIRTUAL_P (node->symbol.decl)
&& !DECL_STATIC_CONSTRUCTOR (node->symbol.decl)
&& !DECL_STATIC_DESTRUCTOR (node->symbol.decl)
&& !node->symbol.externally_visible);
Index: src/gcc/cgraphclones.c
===================================================================
--- src.orig/gcc/cgraphclones.c
+++ src/gcc/cgraphclones.c
@@ -319,6 +319,7 @@ cgraph_create_virtual_clone (struct cgra
TREE_PUBLIC (new_node->symbol.decl) = 0;
DECL_COMDAT (new_node->symbol.decl) = 0;
DECL_WEAK (new_node->symbol.decl) = 0;
+ DECL_VIRTUAL_P (new_node->symbol.decl) = 0;
DECL_STATIC_CONSTRUCTOR (new_node->symbol.decl) = 0;
DECL_STATIC_DESTRUCTOR (new_node->symbol.decl) = 0;
new_node->clone.tree_map = tree_map;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 18:42 ` Martin Jambor
@ 2013-01-16 21:24 ` Jan Hubicka
2013-01-18 17:46 ` Martin Jambor
2013-01-18 17:48 ` Martin Jambor
0 siblings, 2 replies; 8+ messages in thread
From: Jan Hubicka @ 2013-01-16 21:24 UTC (permalink / raw)
To: Jan Hubicka, Richard Biener, GCC Patches
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 21:24 ` Jan Hubicka
@ 2013-01-18 17:46 ` Martin Jambor
2013-01-18 17:48 ` Martin Jambor
1 sibling, 0 replies; 8+ messages in thread
From: Martin Jambor @ 2013-01-18 17:46 UTC (permalink / raw)
To: gcc-patches
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.7 branch on Monday unless someone objects. It passes
bootstrap and testsuite without any issues.
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_or_aliased_p): Return false
for virtual functions.
* ipa-inline-transform.c (can_remove_node_now_p_1): Never return true
for virtual methods.
* ipa.c (cgraph_remove_unreachable_nodes): Never return true for
virtual methods before inlining is over.
testsuite/
* g++.dg/ipa/pr55264.C: New test.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 9cc3690..78e131b 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -2269,6 +2269,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;
DECL_STATIC_CONSTRUCTOR (new_node->decl) = 0;
DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
new_node->clone.tree_map = tree_map;
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 122a44c..7abfb8a 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -935,6 +935,7 @@ cgraph_only_called_directly_or_aliased_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-transform.c b/gcc/ipa-inline-transform.c
index 75b8e9d..ce1bc6e 100644
--- a/gcc/ipa-inline-transform.c
+++ b/gcc/ipa-inline-transform.c
@@ -95,9 +95,7 @@ can_remove_node_now_p_1 (struct cgraph_node *node)
those only after all devirtualizable virtual calls are processed.
Lacking may edges in callgraph we just preserve them post
inlining. */
- && (!DECL_VIRTUAL_P (node->decl)
- || (!DECL_COMDAT (node->decl)
- && !DECL_EXTERNAL (node->decl)))
+ && !DECL_VIRTUAL_P (node->decl)
/* During early inlining some unanalyzed cgraph nodes might be in the
callgraph and they might reffer the function in question. */
&& !cgraph_new_nodes);
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 388291a..8c2b3bd 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -194,9 +194,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))))
{
gcc_assert (!node->global.inlined_to);
enqueue_cgraph_node (node, &first);
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 ();
+}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining
2013-01-16 21:24 ` Jan Hubicka
2013-01-18 17:46 ` Martin Jambor
@ 2013-01-18 17:48 ` Martin Jambor
1 sibling, 0 replies; 8+ messages in thread
From: Martin Jambor @ 2013-01-18 17:48 UTC (permalink / raw)
To: gcc-patches
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.
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 ();
+}
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-01-18 17:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-16 10:01 [PATCH, PR 55264] Do not remove as unreachable any virtual methods before inlining Martin Jambor
2013-01-16 10:30 ` Richard Biener
2013-01-16 11:21 ` Jan Hubicka
2013-01-16 12:44 ` Jan Hubicka
2013-01-16 18:42 ` Martin Jambor
2013-01-16 21:24 ` Jan Hubicka
2013-01-18 17:46 ` Martin Jambor
2013-01-18 17:48 ` Martin Jambor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).