| Message ID | ri6lfrzt2d3.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | ipa-cp: Avoid ICEs when looking at expanded thunks and unoptimized functions (PR 92476) | expand |
> Hi, > > the patch below fixes the i686 failures reported in PR 92476. Newly > expanded "artificial" thunks need to be analyzed when expanded so that > we create necessary function summaries and jump functions for them. > They still don't get IPA-CP lattices, so I looked at all accesses to > those and verified that only the functions saving IPA-VR and IPA-bits > analyses could try to access non-existing lattices. > > After that, Martin's testcase in comment 4 of the bug also revealed two > places where we try to access summaries of unoptimized functions and > segfault, so I fixed those too. Unfortunately it seems our testsuite > cannot optimize different LTO compilation units with different options > and so I could not add the testcase there. But it no longer ICEs. I think you can simply add different flag into different testcases: 20090210_1.c:/* { dg-options "-fPIC" { target { ! sparc*-*-* } } } */ 20090218-1_1.c:/* { dg-options "-fgnu89-inline" } */ 20090218-2_1.c:/* { dg-options { -fgnu89-inline } } */ 20111207-1_1.c:/* { dg-options "-fno-lto" } */ > > Bootstrapped and LTO-profile-bootstrapped and tested on x86_64-linux and > I also verified the -m32 testsuite failures are all gone. OK for trunk? > > Thanks, > > Martin > > > 2019-11-28 Martin Jambor <mjambor@suse.cz> > Jan Hubicka <jh@suse.cz> > > PR ipa/92476 > * ipa-cp.c (set_single_call_flag): Set node_calling_single_call in > the summary only if the summary exists. > (find_more_scalar_values_for_callers_subset): Check node_dead in > the summary only if the summary exists. > (ipcp_store_bits_results): Ignore nodes without lattices. > (ipcp_store_vr_results): Likewise. > * cgraphclones.c: Include ipa-fnsummary.h and ipa-prop.h and the > header files required by them. > (cgraph_node::expand_all_artificial_thunks): Analyze expanded thunks. OK, thanks Honza > --- > gcc/cgraphclones.c | 7 +++++++ > gcc/ipa-cp.c | 10 ++++++++-- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c > index ac5c57a47aa..8e86e82a226 100644 > --- a/gcc/cgraphclones.c > +++ b/gcc/cgraphclones.c > @@ -80,6 +80,11 @@ along with GCC; see the file COPYING3. If not see > #include "tree-inline.h" > #include "dumpfile.h" > #include "gimple-pretty-print.h" > +#include "alloc-pool.h" > +#include "symbol-summary.h" > +#include "tree-vrp.h" > +#include "ipa-prop.h" > +#include "ipa-fnsummary.h" > > /* Create clone of edge in the node N represented by CALL_EXPR > the callgraph. */ > @@ -267,6 +272,8 @@ cgraph_node::expand_all_artificial_thunks () > { > thunk->thunk.thunk_p = false; > thunk->analyze (); > + ipa_analyze_node (thunk); > + inline_analyze_function (thunk); > } > thunk->expand_all_artificial_thunks (); > } > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 31a98a3d98a..7fb9f30f709 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -1165,7 +1165,7 @@ set_single_call_flag (cgraph_node *node, void *) > /* Local thunks can be handled transparently, skip them. */ > while (cs && cs->caller->thunk.thunk_p && cs->caller->local) > cs = cs->next_caller; > - if (cs) > + if (cs && IPA_NODE_REF (cs->caller)) > { > IPA_NODE_REF (cs->caller)->node_calling_single_call = true; > return true; > @@ -4411,7 +4411,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node, > struct ipa_jump_func *jump_func; > tree t; > > - if (IPA_NODE_REF (cs->caller)->node_dead) > + if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead) > continue; > > if (!IPA_EDGE_REF (cs) > @@ -5416,6 +5416,9 @@ ipcp_store_bits_results (void) > > if (info->ipcp_orig_node) > info = IPA_NODE_REF (info->ipcp_orig_node); > + if (!info->lattices) > + /* Newly expanded artificial thunks do not have lattices. */ > + continue; > > unsigned count = ipa_get_param_count (info); > for (unsigned i = 0; i < count; i++) > @@ -5489,6 +5492,9 @@ ipcp_store_vr_results (void) > > if (info->ipcp_orig_node) > info = IPA_NODE_REF (info->ipcp_orig_node); > + if (!info->lattices) > + /* Newly expanded artificial thunks do not have lattices. */ > + continue; > > unsigned count = ipa_get_param_count (info); > for (unsigned i = 0; i < count; i++) > -- > 2.24.0 >
Hi, On Fri, Nov 29 2019, Jan Hubicka wrote: >> Hi, >> >> the patch below fixes the i686 failures reported in PR 92476. Newly >> expanded "artificial" thunks need to be analyzed when expanded so that >> we create necessary function summaries and jump functions for them. >> They still don't get IPA-CP lattices, so I looked at all accesses to >> those and verified that only the functions saving IPA-VR and IPA-bits >> analyses could try to access non-existing lattices. >> >> After that, Martin's testcase in comment 4 of the bug also revealed two >> places where we try to access summaries of unoptimized functions and >> segfault, so I fixed those too. Unfortunately it seems our testsuite >> cannot optimize different LTO compilation units with different options >> and so I could not add the testcase there. But it no longer ICEs. > I think you can simply add different flag into different testcases: > 20090210_1.c:/* { dg-options "-fPIC" { target { ! sparc*-*-* } } } */ > 20090218-1_1.c:/* { dg-options "-fgnu89-inline" } */ > 20090218-2_1.c:/* { dg-options { -fgnu89-inline } } */ > 20111207-1_1.c:/* { dg-options "-fno-lto" } */ > Ah, servers me right for only grepping for dg-lto-options. I have committed the fix and will commit the following testcase addition as a follow-up. Thanks, Martin 2019-11-29 Martin Jambor <mjambor@suse.cz> PR ipa/92476 * g++.dg/lto/pr92476_[01].C: New test. --- gcc/testsuite/g++.dg/lto/pr92476_0.C | 20 ++++++++++++++++++++ gcc/testsuite/g++.dg/lto/pr92476_1.C | 13 +++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 gcc/testsuite/g++.dg/lto/pr92476_0.C create mode 100644 gcc/testsuite/g++.dg/lto/pr92476_1.C diff --git a/gcc/testsuite/g++.dg/lto/pr92476_0.C b/gcc/testsuite/g++.dg/lto/pr92476_0.C new file mode 100644 index 00000000000..5bbc9236f4d --- /dev/null +++ b/gcc/testsuite/g++.dg/lto/pr92476_0.C @@ -0,0 +1,20 @@ +// { dg-lto-do link } +// { dg-lto-options { { -O0 -flto -shared -fPIC -fvisibility=hidden } } } +// { dg-require-effective-target fpic } +// { dg-require-effective-target shared } +// { dg-extra-ld-options "-shared" } + +namespace Passenger { +namespace Json { +class Value {}; +} // namespace Json +namespace ConfigKit { +class Translator {}; +} // namespace ConfigKit +namespace LoggingKit { +void initialize(const Json::Value & = Json::Value(), + const ConfigKit::Translator & = ConfigKit::Translator()); +void init_module() { initialize(); } +} // namespace LoggingKit +} // namespace Passenger + diff --git a/gcc/testsuite/g++.dg/lto/pr92476_1.C b/gcc/testsuite/g++.dg/lto/pr92476_1.C new file mode 100644 index 00000000000..cd29613b808 --- /dev/null +++ b/gcc/testsuite/g++.dg/lto/pr92476_1.C @@ -0,0 +1,13 @@ +// { dg-options { -O2 -flto -shared -fPIC -fvisibility=hidden } } + +namespace Passenger { +namespace Json { +class Value; +} +namespace ConfigKit { +class Translator; +} +namespace LoggingKit { +void initialize(const Json::Value &, const ConfigKit::Translator &) {} +} // namespace LoggingKit +} // namespace Passenger
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index ac5c57a47aa..8e86e82a226 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -80,6 +80,11 @@ along with GCC; see the file COPYING3. If not see #include "tree-inline.h" #include "dumpfile.h" #include "gimple-pretty-print.h" +#include "alloc-pool.h" +#include "symbol-summary.h" +#include "tree-vrp.h" +#include "ipa-prop.h" +#include "ipa-fnsummary.h" /* Create clone of edge in the node N represented by CALL_EXPR the callgraph. */ @@ -267,6 +272,8 @@ cgraph_node::expand_all_artificial_thunks () { thunk->thunk.thunk_p = false; thunk->analyze (); + ipa_analyze_node (thunk); + inline_analyze_function (thunk); } thunk->expand_all_artificial_thunks (); } diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 31a98a3d98a..7fb9f30f709 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -1165,7 +1165,7 @@ set_single_call_flag (cgraph_node *node, void *) /* Local thunks can be handled transparently, skip them. */ while (cs && cs->caller->thunk.thunk_p && cs->caller->local) cs = cs->next_caller; - if (cs) + if (cs && IPA_NODE_REF (cs->caller)) { IPA_NODE_REF (cs->caller)->node_calling_single_call = true; return true; @@ -4411,7 +4411,7 @@ find_more_scalar_values_for_callers_subset (struct cgraph_node *node, struct ipa_jump_func *jump_func; tree t; - if (IPA_NODE_REF (cs->caller)->node_dead) + if (IPA_NODE_REF (cs->caller) && IPA_NODE_REF (cs->caller)->node_dead) continue; if (!IPA_EDGE_REF (cs) @@ -5416,6 +5416,9 @@ ipcp_store_bits_results (void) if (info->ipcp_orig_node) info = IPA_NODE_REF (info->ipcp_orig_node); + if (!info->lattices) + /* Newly expanded artificial thunks do not have lattices. */ + continue; unsigned count = ipa_get_param_count (info); for (unsigned i = 0; i < count; i++) @@ -5489,6 +5492,9 @@ ipcp_store_vr_results (void) if (info->ipcp_orig_node) info = IPA_NODE_REF (info->ipcp_orig_node); + if (!info->lattices) + /* Newly expanded artificial thunks do not have lattices. */ + continue; unsigned count = ipa_get_param_count (info); for (unsigned i = 0; i < count; i++)
Hi, the patch below fixes the i686 failures reported in PR 92476. Newly expanded "artificial" thunks need to be analyzed when expanded so that we create necessary function summaries and jump functions for them. They still don't get IPA-CP lattices, so I looked at all accesses to those and verified that only the functions saving IPA-VR and IPA-bits analyses could try to access non-existing lattices. After that, Martin's testcase in comment 4 of the bug also revealed two places where we try to access summaries of unoptimized functions and segfault, so I fixed those too. Unfortunately it seems our testsuite cannot optimize different LTO compilation units with different options and so I could not add the testcase there. But it no longer ICEs. Bootstrapped and LTO-profile-bootstrapped and tested on x86_64-linux and I also verified the -m32 testsuite failures are all gone. OK for trunk? Thanks, Martin 2019-11-28 Martin Jambor <mjambor@suse.cz> Jan Hubicka <jh@suse.cz> PR ipa/92476 * ipa-cp.c (set_single_call_flag): Set node_calling_single_call in the summary only if the summary exists. (find_more_scalar_values_for_callers_subset): Check node_dead in the summary only if the summary exists. (ipcp_store_bits_results): Ignore nodes without lattices. (ipcp_store_vr_results): Likewise. * cgraphclones.c: Include ipa-fnsummary.h and ipa-prop.h and the header files required by them. (cgraph_node::expand_all_artificial_thunks): Analyze expanded thunks. --- gcc/cgraphclones.c | 7 +++++++ gcc/ipa-cp.c | 10 ++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-)