| Message ID | 20130612115928.GB26236@virgil.suse |
|---|---|
| State | New |
| Headers | show |
> Hi, > > PR 57539 revealed two problems with remapping reference descriptors > during cloning of trees of inlined call graph nodes. First, when > indirect inlining is involved, we happily remove the reference > descriptor itself by calling ipa_free_edge_args_substructures in > ipa_propagate_indirect_call_infos. Second, the current remapping code > does not work because the global.inlined_to field of the destination > caller is not yet set. > > The patch below fixes the first problem by not calling the freeing > function and the second one by making cgraph_clone_node set the > required field prior to calling any duplication hooks (which is the > only place where we have the pair of corresponding source and > destination edge at our disposal so the duplication/remapping has to > happen there). I have also shortened the lists of corresponding > references and cleared up the search loop a little. > > I'll post a testcase in a separate patch. This one bootstrapped and > passed testsuite on x86_64-linux without any issues. OK for trunk? > > Thanks, > > Martin > > > 2013-06-10 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/57539 > * cgraphclones.c (cgraph_clone_node): Add parameter new_inlined_to, set > global.inlined_to of the new node to it. All callers changed. > * ipa-inline-transform.c (clone_inlined_nodes): New variable > inlining_into, pass it to cgraph_clone_node. > * ipa-prop.c (ipa_propagate_indirect_call_infos): Do not call > ipa_free_edge_args_substructures. > (ipa_edge_duplication_hook): Only add edges from inlined nodes to > rdesc linked list. Do not assert rdesc edges have inlined caller. > Assert we have found an rdesc in the rdesc list. OK, thanks Honza > > Index: src/gcc/cgraph.h > =================================================================== > --- src.orig/gcc/cgraph.h > +++ src/gcc/cgraph.h > @@ -707,7 +707,7 @@ struct cgraph_edge * cgraph_clone_edge ( > unsigned, gcov_type, int, bool); > struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type, > int, bool, vec<cgraph_edge_p>, > - bool); > + bool, struct cgraph_node *); > tree clone_function_name (tree decl, const char *); > struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node, > vec<cgraph_edge_p>, > Index: src/gcc/cgraphclones.c > =================================================================== > --- src.orig/gcc/cgraphclones.c > +++ src/gcc/cgraphclones.c > @@ -167,13 +167,19 @@ cgraph_clone_edge (struct cgraph_edge *e > function's profile to reflect the fact that part of execution is handled > by node. > When CALL_DUPLICATOIN_HOOK is true, the ipa passes are acknowledged about > - the new clone. Otherwise the caller is responsible for doing so later. */ > + the new clone. Otherwise the caller is responsible for doing so later. > + > + If the new node is being inlined into another one, NEW_INLINED_TO should be > + the outline function the new one is (even indirectly) inlined to. All hooks > + will see this in node's global.inlined_to, when invoked. Can be NULL if the > + node is not inlined. */ > > struct cgraph_node * > cgraph_clone_node (struct cgraph_node *n, tree decl, gcov_type count, int freq, > bool update_original, > vec<cgraph_edge_p> redirect_callers, > - bool call_duplication_hook) > + bool call_duplication_hook, > + struct cgraph_node *new_inlined_to) > { > struct cgraph_node *new_node = cgraph_create_empty_node (); > struct cgraph_edge *e; > @@ -195,6 +201,7 @@ cgraph_clone_node (struct cgraph_node *n > new_node->symbol.externally_visible = false; > new_node->local.local = true; > new_node->global = n->global; > + new_node->global.inlined_to = new_inlined_to; > new_node->rtl = n->rtl; > new_node->count = count; > new_node->frequency = n->frequency; > @@ -307,7 +314,7 @@ cgraph_create_virtual_clone (struct cgra > > new_node = cgraph_clone_node (old_node, new_decl, old_node->count, > CGRAPH_FREQ_BASE, false, > - redirect_callers, false); > + redirect_callers, false, NULL); > /* Update the properties. > Make clone visible only within this translation unit. Make sure > that is not weak also. > Index: src/gcc/ipa-inline-transform.c > =================================================================== > --- src.orig/gcc/ipa-inline-transform.c > +++ src/gcc/ipa-inline-transform.c > @@ -132,6 +132,13 @@ void > clone_inlined_nodes (struct cgraph_edge *e, bool duplicate, > bool update_original, int *overall_size) > { > + struct cgraph_node *inlining_into; > + > + if (e->caller->global.inlined_to) > + inlining_into = e->caller->global.inlined_to; > + else > + inlining_into = e->caller; > + > if (duplicate) > { > /* We may eliminate the need for out-of-line copy to be output. > @@ -167,18 +174,15 @@ clone_inlined_nodes (struct cgraph_edge > { > struct cgraph_node *n; > n = cgraph_clone_node (e->callee, e->callee->symbol.decl, > - e->count, e->frequency, > - update_original, vNULL, true); > + e->count, e->frequency, update_original, > + vNULL, true, inlining_into); > cgraph_redirect_edge_callee (e, n); > } > } > else > symtab_dissolve_same_comdat_group_list ((symtab_node) e->callee); > > - if (e->caller->global.inlined_to) > - e->callee->global.inlined_to = e->caller->global.inlined_to; > - else > - e->callee->global.inlined_to = e->caller; > + e->callee->global.inlined_to = inlining_into; > > /* Recursively clone all bodies. */ > for (e = e->callee->callees; e; e = e->next_callee) > Index: src/gcc/ipa-inline.c > =================================================================== > --- src.orig/gcc/ipa-inline.c > +++ src/gcc/ipa-inline.c > @@ -1315,7 +1315,7 @@ recursive_inlining (struct cgraph_edge * > /* We need original clone to copy around. */ > master_clone = cgraph_clone_node (node, node->symbol.decl, > node->count, CGRAPH_FREQ_BASE, > - false, vNULL, true); > + false, vNULL, true, NULL); > for (e = master_clone->callees; e; e = e->next_callee) > if (!e->inline_failed) > clone_inlined_nodes (e, true, false, NULL); > Index: src/gcc/ipa-prop.c > =================================================================== > --- src.orig/gcc/ipa-prop.c > +++ src/gcc/ipa-prop.c > @@ -2683,9 +2683,6 @@ ipa_propagate_indirect_call_infos (struc > propagate_controlled_uses (cs); > changed = propagate_info_to_inlined_callees (cs, cs->callee, new_edges); > > - /* We do not keep jump functions of inlined edges up to date. Better to free > - them so we do not access them accidentally. */ > - ipa_free_edge_args_substructures (IPA_EDGE_REF (cs)); > return changed; > } > > @@ -2815,9 +2812,12 @@ ipa_edge_duplication_hook (struct cgraph > dst_rdesc > = (struct ipa_cst_ref_desc *) pool_alloc (ipa_refdesc_pool); > dst_rdesc->cs = dst; > - dst_rdesc->next_duplicate = src_rdesc->next_duplicate; > - src_rdesc->next_duplicate = dst_rdesc; > dst_rdesc->refcount = src_rdesc->refcount; > + if (dst->caller->global.inlined_to) > + { > + dst_rdesc->next_duplicate = src_rdesc->next_duplicate; > + src_rdesc->next_duplicate = dst_rdesc; > + } > dst_jf->value.constant.rdesc = dst_rdesc; > } > else > @@ -2832,13 +2832,10 @@ ipa_edge_duplication_hook (struct cgraph > for (dst_rdesc = src_rdesc->next_duplicate; > dst_rdesc; > dst_rdesc = dst_rdesc->next_duplicate) > - { > - gcc_assert (dst_rdesc->cs->caller->global.inlined_to); > - if (dst_rdesc->cs->caller->global.inlined_to > - == dst->caller->global.inlined_to) > - break; > - } > - > + if (dst_rdesc->cs->caller->global.inlined_to > + == dst->caller->global.inlined_to) > + break; > + gcc_assert (dst_rdesc); > dst_jf->value.constant.rdesc = dst_rdesc; > } > } > Index: src/gcc/lto-cgraph.c > =================================================================== > --- src.orig/gcc/lto-cgraph.c > +++ src/gcc/lto-cgraph.c > @@ -951,7 +951,7 @@ input_node (struct lto_file_decl_data *f > { > node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl, > 0, CGRAPH_FREQ_BASE, false, > - vNULL, false); > + vNULL, false, NULL); > } > else > node = cgraph_get_create_node (fn_decl);
Index: src/gcc/cgraph.h =================================================================== --- src.orig/gcc/cgraph.h +++ src/gcc/cgraph.h @@ -707,7 +707,7 @@ struct cgraph_edge * cgraph_clone_edge ( unsigned, gcov_type, int, bool); struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type, int, bool, vec<cgraph_edge_p>, - bool); + bool, struct cgraph_node *); tree clone_function_name (tree decl, const char *); struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node, vec<cgraph_edge_p>, Index: src/gcc/cgraphclones.c =================================================================== --- src.orig/gcc/cgraphclones.c +++ src/gcc/cgraphclones.c @@ -167,13 +167,19 @@ cgraph_clone_edge (struct cgraph_edge *e function's profile to reflect the fact that part of execution is handled by node. When CALL_DUPLICATOIN_HOOK is true, the ipa passes are acknowledged about - the new clone. Otherwise the caller is responsible for doing so later. */ + the new clone. Otherwise the caller is responsible for doing so later. + + If the new node is being inlined into another one, NEW_INLINED_TO should be + the outline function the new one is (even indirectly) inlined to. All hooks + will see this in node's global.inlined_to, when invoked. Can be NULL if the + node is not inlined. */ struct cgraph_node * cgraph_clone_node (struct cgraph_node *n, tree decl, gcov_type count, int freq, bool update_original, vec<cgraph_edge_p> redirect_callers, - bool call_duplication_hook) + bool call_duplication_hook, + struct cgraph_node *new_inlined_to) { struct cgraph_node *new_node = cgraph_create_empty_node (); struct cgraph_edge *e; @@ -195,6 +201,7 @@ cgraph_clone_node (struct cgraph_node *n new_node->symbol.externally_visible = false; new_node->local.local = true; new_node->global = n->global; + new_node->global.inlined_to = new_inlined_to; new_node->rtl = n->rtl; new_node->count = count; new_node->frequency = n->frequency; @@ -307,7 +314,7 @@ cgraph_create_virtual_clone (struct cgra new_node = cgraph_clone_node (old_node, new_decl, old_node->count, CGRAPH_FREQ_BASE, false, - redirect_callers, false); + redirect_callers, false, NULL); /* Update the properties. Make clone visible only within this translation unit. Make sure that is not weak also. Index: src/gcc/ipa-inline-transform.c =================================================================== --- src.orig/gcc/ipa-inline-transform.c +++ src/gcc/ipa-inline-transform.c @@ -132,6 +132,13 @@ void clone_inlined_nodes (struct cgraph_edge *e, bool duplicate, bool update_original, int *overall_size) { + struct cgraph_node *inlining_into; + + if (e->caller->global.inlined_to) + inlining_into = e->caller->global.inlined_to; + else + inlining_into = e->caller; + if (duplicate) { /* We may eliminate the need for out-of-line copy to be output. @@ -167,18 +174,15 @@ clone_inlined_nodes (struct cgraph_edge { struct cgraph_node *n; n = cgraph_clone_node (e->callee, e->callee->symbol.decl, - e->count, e->frequency, - update_original, vNULL, true); + e->count, e->frequency, update_original, + vNULL, true, inlining_into); cgraph_redirect_edge_callee (e, n); } } else symtab_dissolve_same_comdat_group_list ((symtab_node) e->callee); - if (e->caller->global.inlined_to) - e->callee->global.inlined_to = e->caller->global.inlined_to; - else - e->callee->global.inlined_to = e->caller; + e->callee->global.inlined_to = inlining_into; /* Recursively clone all bodies. */ for (e = e->callee->callees; e; e = e->next_callee) Index: src/gcc/ipa-inline.c =================================================================== --- src.orig/gcc/ipa-inline.c +++ src/gcc/ipa-inline.c @@ -1315,7 +1315,7 @@ recursive_inlining (struct cgraph_edge * /* We need original clone to copy around. */ master_clone = cgraph_clone_node (node, node->symbol.decl, node->count, CGRAPH_FREQ_BASE, - false, vNULL, true); + false, vNULL, true, NULL); for (e = master_clone->callees; e; e = e->next_callee) if (!e->inline_failed) clone_inlined_nodes (e, true, false, NULL); Index: src/gcc/ipa-prop.c =================================================================== --- src.orig/gcc/ipa-prop.c +++ src/gcc/ipa-prop.c @@ -2683,9 +2683,6 @@ ipa_propagate_indirect_call_infos (struc propagate_controlled_uses (cs); changed = propagate_info_to_inlined_callees (cs, cs->callee, new_edges); - /* We do not keep jump functions of inlined edges up to date. Better to free - them so we do not access them accidentally. */ - ipa_free_edge_args_substructures (IPA_EDGE_REF (cs)); return changed; } @@ -2815,9 +2812,12 @@ ipa_edge_duplication_hook (struct cgraph dst_rdesc = (struct ipa_cst_ref_desc *) pool_alloc (ipa_refdesc_pool); dst_rdesc->cs = dst; - dst_rdesc->next_duplicate = src_rdesc->next_duplicate; - src_rdesc->next_duplicate = dst_rdesc; dst_rdesc->refcount = src_rdesc->refcount; + if (dst->caller->global.inlined_to) + { + dst_rdesc->next_duplicate = src_rdesc->next_duplicate; + src_rdesc->next_duplicate = dst_rdesc; + } dst_jf->value.constant.rdesc = dst_rdesc; } else @@ -2832,13 +2832,10 @@ ipa_edge_duplication_hook (struct cgraph for (dst_rdesc = src_rdesc->next_duplicate; dst_rdesc; dst_rdesc = dst_rdesc->next_duplicate) - { - gcc_assert (dst_rdesc->cs->caller->global.inlined_to); - if (dst_rdesc->cs->caller->global.inlined_to - == dst->caller->global.inlined_to) - break; - } - + if (dst_rdesc->cs->caller->global.inlined_to + == dst->caller->global.inlined_to) + break; + gcc_assert (dst_rdesc); dst_jf->value.constant.rdesc = dst_rdesc; } } Index: src/gcc/lto-cgraph.c =================================================================== --- src.orig/gcc/lto-cgraph.c +++ src/gcc/lto-cgraph.c @@ -951,7 +951,7 @@ input_node (struct lto_file_decl_data *f { node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl, 0, CGRAPH_FREQ_BASE, false, - vNULL, false); + vNULL, false, NULL); } else node = cgraph_get_create_node (fn_decl);