| Message ID | ri6k12ozg34.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | ipa: Make call redirection detect already adjusted calls (PR 93621) | expand |
> > 2020-04-09 Martin Jambor <mjambor@suse.cz> > > PR ipa/93621 > * ipa-inline.h (ipa_saved_clone_sources): Declare. > * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. > (save_inline_function_body): Link the new body holder with the > previous one. > * cgraph.c: Include ipa-inline.h. > (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from > the statement in ipa_saved_clone_sources. > * cgraphunit.c: Include ipa-inline.h. > (expand_all_functions): Free ipa_saved_clone_sources. As discussed on IRC this is kind of hack - we should keep track of the cloning paths if we want to allow calls to already modified decls. However this is OK for gcc 10. I believe this can trigger wrong code with earlier releases? > diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h > index 5025b6045fc..c596f77d0e7 100644 > --- a/gcc/ipa-inline.h > +++ b/gcc/ipa-inline.h > @@ -65,6 +65,7 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool, bool, int *); > > extern int ncalls_inlined; > extern int nfunctions_inlined; > +extern function_summary <cgraph_node **> *ipa_saved_clone_sources; Please make it point to DECL itself. While symbols can be removed decls stays and we only care about decl anyway. Honza
Hi, On Thu, Apr 16 2020, Jan Hubicka wrote: >> >> 2020-04-09 Martin Jambor <mjambor@suse.cz> >> >> PR ipa/93621 >> * ipa-inline.h (ipa_saved_clone_sources): Declare. >> * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. >> (save_inline_function_body): Link the new body holder with the >> previous one. >> * cgraph.c: Include ipa-inline.h. >> (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from >> the statement in ipa_saved_clone_sources. >> * cgraphunit.c: Include ipa-inline.h. >> (expand_all_functions): Free ipa_saved_clone_sources. > As discussed on IRC this is kind of hack - we should keep track of the > cloning paths if we want to allow calls to already modified decls. > However this is OK for gcc 10. Thanks. > I believe this can trigger wrong code with earlier releases? No, the same assert is there in previous releases too, it just tests !node->clone.combined_args_to_skip instead of !node->clone.param_adjustments. >> diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h >> index 5025b6045fc..c596f77d0e7 100644 >> --- a/gcc/ipa-inline.h >> +++ b/gcc/ipa-inline.h >> @@ -65,6 +65,7 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool, bool, int *); >> >> extern int ncalls_inlined; >> extern int nfunctions_inlined; >> +extern function_summary <cgraph_node **> *ipa_saved_clone_sources; > Please make it point to DECL itself. While symbols can be removed decls > stays and we only care about decl anyway. OK, the following has passed bootstrap and testing on x86-64-linux. I'm running LTO bootstrap now and will commit it if it passes. Thanks, Martin 2020-04-16 Martin Jambor <mjambor@suse.cz> PR ipa/93621 * ipa-inline.h (ipa_saved_clone_sources): Declare. * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. (save_inline_function_body): Link the new body holder with the previous one. * cgraph.c: Include ipa-inline.h. (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from the statement in ipa_saved_clone_sources. * cgraphunit.c: Include ipa-inline.h. (expand_all_functions): Free ipa_saved_clone_sources. testsuite/ * g++.dg/ipa/pr93621.C: New test. --- gcc/ChangeLog | 13 +++++++++++++ gcc/cgraph.c | 11 +++++++++++ gcc/cgraphunit.c | 4 +++- gcc/ipa-inline-transform.c | 19 +++++++++++++++++++ gcc/ipa-inline.h | 1 + gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/g++.dg/ipa/pr93621.C | 29 +++++++++++++++++++++++++++++ 7 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr93621.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 726e629188a..834026c8f16 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,16 @@ +2020-04-16 Martin Jambor <mjambor@suse.cz> + + PR ipa/93621 + * ipa-inline.h (ipa_saved_clone_sources): Declare. + * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. + (save_inline_function_body): Link the new body holder with the + previous one. + * cgraph.c: Include ipa-inline.h. + (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from + the statement in ipa_saved_clone_sources. + * cgraphunit.c: Include ipa-inline.h. + (expand_all_functions): Free ipa_saved_clone_sources. + 2020-04-13 Martin Sebor <msebor@redhat.com> * doc/extend.texi (-Wall): Mention -Wformat-overflow and diff --git a/gcc/cgraph.c b/gcc/cgraph.c index ecb234d032f..72d7cb54301 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see #include "attribs.h" #include "selftest.h" #include "tree-into-ssa.h" +#include "ipa-inline.h" /* FIXME: Only for PROP_loops, but cgraph shouldn't have to know about this. */ #include "tree-pass.h" @@ -1470,6 +1471,16 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) || decl == e->callee->decl) return e->call_stmt; + if (decl && ipa_saved_clone_sources) + { + tree *p = ipa_saved_clone_sources->get (e->callee); + if (p && decl == *p) + { + gimple_call_set_fndecl (e->call_stmt, e->callee->decl); + return e->call_stmt; + } + } + if (flag_checking && decl) { cgraph_node *node = cgraph_node::get (decl); diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 0e255f25b7d..a1ace95879a 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -205,6 +205,7 @@ along with GCC; see the file COPYING3. If not see #include "lto-section-names.h" #include "stringpool.h" #include "attribs.h" +#include "ipa-inline.h" /* Queue of cgraph nodes scheduled to be added into cgraph. This is a secondary queue used during optimization to accommodate passes that @@ -2481,7 +2482,8 @@ expand_all_functions (void) symtab->process_new_functions (); free_gimplify_stack (); - + delete ipa_saved_clone_sources; + ipa_saved_clone_sources = NULL; free (order); } diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c index eed992d314d..be60bbccb5c 100644 --- a/gcc/ipa-inline-transform.c +++ b/gcc/ipa-inline-transform.c @@ -531,6 +531,11 @@ inline_call (struct cgraph_edge *e, bool update_original, return new_edges_found; } +/* For each node that was made the holder of function body by + save_inline_function_body, this summary contains pointer to the previous + holder of the body. */ + +function_summary <tree *> *ipa_saved_clone_sources; /* Copy function body of NODE and redirect all inline clones to it. This is done before inline plan is applied to NODE when there are @@ -588,6 +593,20 @@ save_inline_function_body (struct cgraph_node *node) first_clone->next_sibling_clone = NULL; gcc_assert (!first_clone->prev_sibling_clone); } + + tree prev_body_holder = node->decl; + if (!ipa_saved_clone_sources) + ipa_saved_clone_sources = new function_summary <tree *> (symtab); + else + { + tree *p = ipa_saved_clone_sources->get (node); + if (p) + { + prev_body_holder = *p; + gcc_assert (prev_body_holder); + } + } + *ipa_saved_clone_sources->get_create (first_clone) = prev_body_holder; first_clone->clone_of = NULL; /* Now node in question has no clones. */ diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h index 5025b6045fc..74c08685e49 100644 --- a/gcc/ipa-inline.h +++ b/gcc/ipa-inline.h @@ -65,6 +65,7 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool, bool, int *); extern int ncalls_inlined; extern int nfunctions_inlined; +extern function_summary <tree *> *ipa_saved_clone_sources; /* Return estimated size of the inline sequence of EDGE. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 2035cf6fd1f..5587a98f7d5 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-04-16 Martin Jambor <mjambor@suse.cz> + + PR ipa/93621 + * g++.dg/ipa/pr93621.C: New test. + 2020-04-13 Marek Polacek <polacek@redhat.com> PR c++/94588 diff --git a/gcc/testsuite/g++.dg/ipa/pr93621.C b/gcc/testsuite/g++.dg/ipa/pr93621.C new file mode 100644 index 00000000000..ffe6bbdae2e --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr93621.C @@ -0,0 +1,29 @@ +// PR ipa/93621 +// { dg-do compile } +// { dg-options "-O3 --param ipa-cp-eval-threshold=100 --param large-function-growth=60 --param large-function-insns=10 --param uninlined-thunk-insns=1000" } + +typedef enum { X } E; +struct A { + virtual void bar (); +}; +struct B { + virtual E fn (const char *, int, int *) = 0; +}; +struct C : A, B { + E fn (const char *, int, int *); + void fn2 (); + B *foo; +}; +void C::fn2 () { + if (!foo) + return; + foo->fn (0, 0, 0); +} +E +C::fn (const char *, int, int *) +{ + fn2 (); + foo = 0; + fn (0, 0, 0); + return X; +}
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e10fb251c16..d68f5a68f3e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,16 @@ +2020-04-09 Martin Jambor <mjambor@suse.cz> + + PR ipa/93621 + * ipa-inline.h (ipa_saved_clone_sources): Declare. + * ipa-inline-transform.c (ipa_saved_clone_sources): New variable. + (save_inline_function_body): Link the new body holder with the + previous one. + * cgraph.c: Include ipa-inline.h. + (cgraph_edge::redirect_call_stmt_to_callee): Try to find the decl from + the statement in ipa_saved_clone_sources. + * cgraphunit.c: Include ipa-inline.h. + (expand_all_functions): Free ipa_saved_clone_sources. + 2020-04-05 Zachary Spytz <zspytz@gmail.com> * extend.texi: Add free to list of ISO C90 functions that diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 6b780f80eb3..b4b00b3b1c1 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see #include "attribs.h" #include "selftest.h" #include "tree-into-ssa.h" +#include "ipa-inline.h" /* FIXME: Only for PROP_loops, but cgraph shouldn't have to know about this. */ #include "tree-pass.h" @@ -1470,6 +1471,16 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) || decl == e->callee->decl) return e->call_stmt; + if (decl && ipa_saved_clone_sources) + { + cgraph_node **p = ipa_saved_clone_sources->get (e->callee); + if (p && decl == (*p)->decl) + { + gimple_call_set_fndecl (e->call_stmt, e->callee->decl); + return e->call_stmt; + } + } + if (flag_checking && decl) { cgraph_node *node = cgraph_node::get (decl); diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 0e255f25b7d..a1ace95879a 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -205,6 +205,7 @@ along with GCC; see the file COPYING3. If not see #include "lto-section-names.h" #include "stringpool.h" #include "attribs.h" +#include "ipa-inline.h" /* Queue of cgraph nodes scheduled to be added into cgraph. This is a secondary queue used during optimization to accommodate passes that @@ -2481,7 +2482,8 @@ expand_all_functions (void) symtab->process_new_functions (); free_gimplify_stack (); - + delete ipa_saved_clone_sources; + ipa_saved_clone_sources = NULL; free (order); } diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c index eed992d314d..cd64a643506 100644 --- a/gcc/ipa-inline-transform.c +++ b/gcc/ipa-inline-transform.c @@ -531,6 +531,11 @@ inline_call (struct cgraph_edge *e, bool update_original, return new_edges_found; } +/* For each node that was made the holder of function body by + save_inline_function_body, this summary contains pointer to the previous + holder of the body. */ + +function_summary <cgraph_node **> *ipa_saved_clone_sources; /* Copy function body of NODE and redirect all inline clones to it. This is done before inline plan is applied to NODE when there are @@ -588,6 +593,20 @@ save_inline_function_body (struct cgraph_node *node) first_clone->next_sibling_clone = NULL; gcc_assert (!first_clone->prev_sibling_clone); } + + cgraph_node *prev_body_holder = node; + if (!ipa_saved_clone_sources) + ipa_saved_clone_sources = new function_summary <cgraph_node **> (symtab); + else + { + cgraph_node **p = ipa_saved_clone_sources->get (node); + if (p) + { + prev_body_holder = *p; + gcc_assert (prev_body_holder); + } + } + *ipa_saved_clone_sources->get_create (first_clone) = prev_body_holder; first_clone->clone_of = NULL; /* Now node in question has no clones. */ diff --git a/gcc/ipa-inline.h b/gcc/ipa-inline.h index 5025b6045fc..c596f77d0e7 100644 --- a/gcc/ipa-inline.h +++ b/gcc/ipa-inline.h @@ -65,6 +65,7 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool, bool, int *); extern int ncalls_inlined; extern int nfunctions_inlined; +extern function_summary <cgraph_node **> *ipa_saved_clone_sources; /* Return estimated size of the inline sequence of EDGE. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 88bab5d3d19..88f33ab2b90 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-04-04 Martin Jambor <mjambor@suse.cz> + + PR ipa/93621 + * g++.dg/ipa/pr93621.C: New test. + 2020-04-05 Iain Sandoe <iain@sandoe.co.uk> * g++.dg/coroutines/torture/co-await-14-template-traits.C: Rename... diff --git a/gcc/testsuite/g++.dg/ipa/pr93621.C b/gcc/testsuite/g++.dg/ipa/pr93621.C new file mode 100644 index 00000000000..ffe6bbdae2e --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr93621.C @@ -0,0 +1,29 @@ +// PR ipa/93621 +// { dg-do compile } +// { dg-options "-O3 --param ipa-cp-eval-threshold=100 --param large-function-growth=60 --param large-function-insns=10 --param uninlined-thunk-insns=1000" } + +typedef enum { X } E; +struct A { + virtual void bar (); +}; +struct B { + virtual E fn (const char *, int, int *) = 0; +}; +struct C : A, B { + E fn (const char *, int, int *); + void fn2 (); + B *foo; +}; +void C::fn2 () { + if (!foo) + return; + foo->fn (0, 0, 0); +} +E +C::fn (const char *, int, int *) +{ + fn2 (); + foo = 0; + fn (0, 0, 0); + return X; +}