| Message ID | 20101221145143.GA1298@virgil.arch.suse.de |
|---|---|
| State | New |
| Headers | show |
V/lt> Hi, > > I made a stupid mistake when adding streaming of the new thunk_delta > field in cgraph_indirect_call_info. Without much thinking I simply > added it to the existing streaming but forgot that the streaming deals > with the analysis information, not decision information (for which > ipa-prop has no streaming). This is problem because thunk_delta is > not an analysis thing but is determined and stored when making > inlining and ipa-cp decisions and must be made available to the > transformation phase. > > I think that adding decision streaming to ipa-prop just for this > purpose is not really a good idea (ipa-prop decisions are basically > stored in call graph modifications and streamed along with call graph) > and so I moved the field to the cgraph_edge and made it a > HOST_WIDE_INT since we cannot stream trees along with the call graph. > > I have bootstrapped and tested this on x86_64-linux without any > problems. OK for trunk? > Index: icln/gcc/lto-cgraph.c > =================================================================== > --- icln.orig/gcc/lto-cgraph.c > +++ icln/gcc/lto-cgraph.c > @@ -278,6 +278,7 @@ lto_output_edge (struct lto_simple_outpu > } > > lto_output_sleb128_stream (ob->main_stream, edge->count); > + lto_output_sleb128_stream (ob->main_stream, edge->thunk_delta); > > bp = bitpack_create (ob->main_stream); > uid = (!gimple_has_body_p (edge->caller->decl) > @@ -1210,6 +1211,7 @@ input_edge (struct lto_input_block *ib, > struct cgraph_edge *edge; > unsigned int stmt_id; > gcov_type count; > + HOST_WIDE_INT thunk_delta; > int freq; > unsigned int nest; > cgraph_inline_failed_t inline_failed; > @@ -1230,6 +1232,7 @@ input_edge (struct lto_input_block *ib, > callee = NULL; > > count = (gcov_type) lto_input_sleb128 (ib); > + thunk_delta = lto_input_sleb128 (ib); > > bp = lto_input_bitpack (ib); > stmt_id = (unsigned int) bp_unpack_value (&bp, HOST_BITS_PER_INT); > @@ -1243,6 +1246,7 @@ input_edge (struct lto_input_block *ib, > else > edge = cgraph_create_edge (caller, callee, NULL, count, freq, nest); > > + edge->thunk_delta = thunk_delta; > edge->indirect_inlining_edge = bp_unpack_value (&bp, 1); > edge->lto_stmt_uid = stmt_id; > edge->inline_failed = inline_failed; You want to store this kind of info into cgraphopt section. (probably adding output_egge_opt_summary function). Consider patch pre-approved with that change. Honza
Hi, On Thu, Dec 23, 2010 at 12:49:16PM +0100, Jan Hubicka wrote: > V/lt> Hi, > > > > I made a stupid mistake when adding streaming of the new thunk_delta > > field in cgraph_indirect_call_info. Without much thinking I simply > > added it to the existing streaming but forgot that the streaming deals > > with the analysis information, not decision information (for which > > ipa-prop has no streaming). This is problem because thunk_delta is > > not an analysis thing but is determined and stored when making > > inlining and ipa-cp decisions and must be made available to the > > transformation phase. > > > > I think that adding decision streaming to ipa-prop just for this > > purpose is not really a good idea (ipa-prop decisions are basically > > stored in call graph modifications and streamed along with call graph) > > and so I moved the field to the cgraph_edge and made it a > > HOST_WIDE_INT since we cannot stream trees along with the call graph. > > > > > I have bootstrapped and tested this on x86_64-linux without any > > problems. OK for trunk? > > Index: icln/gcc/lto-cgraph.c > > =================================================================== > > --- icln.orig/gcc/lto-cgraph.c > > +++ icln/gcc/lto-cgraph.c > > @@ -278,6 +278,7 @@ lto_output_edge (struct lto_simple_outpu > > } > > > > lto_output_sleb128_stream (ob->main_stream, edge->count); > > + lto_output_sleb128_stream (ob->main_stream, edge->thunk_delta); > > > > bp = bitpack_create (ob->main_stream); > > uid = (!gimple_has_body_p (edge->caller->decl) > > @@ -1210,6 +1211,7 @@ input_edge (struct lto_input_block *ib, > > struct cgraph_edge *edge; > > unsigned int stmt_id; > > gcov_type count; > > + HOST_WIDE_INT thunk_delta; > > int freq; > > unsigned int nest; > > cgraph_inline_failed_t inline_failed; > > @@ -1230,6 +1232,7 @@ input_edge (struct lto_input_block *ib, > > callee = NULL; > > > > count = (gcov_type) lto_input_sleb128 (ib); > > + thunk_delta = lto_input_sleb128 (ib); > > > > bp = lto_input_bitpack (ib); > > stmt_id = (unsigned int) bp_unpack_value (&bp, HOST_BITS_PER_INT); > > @@ -1243,6 +1246,7 @@ input_edge (struct lto_input_block *ib, > > else > > edge = cgraph_create_edge (caller, callee, NULL, count, freq, nest); > > > > + edge->thunk_delta = thunk_delta; > > edge->indirect_inlining_edge = bp_unpack_value (&bp, 1); > > edge->lto_stmt_uid = stmt_id; > > edge->inline_failed = inline_failed; > > You want to store this kind of info into cgraphopt section. (probably adding > output_egge_opt_summary function). > Consider patch pre-approved with that change. > I guess that I'd have to add a vector of edges to lto_cgraph_encoder_d and to input_cgraph, input_cgraph_opt_summary and its callees. Hardly something to be pre-approved :-) I've already written some of this but before I finish it, test it and re-post, I'd like to ask you to re-consider. If we ever build call graph edges for calls to thunks (especially as we plan to be able to do it when building the graph), such thunk information would become an integral part of an edge, without which the edge has an invalid meaning. I'd therefore not consider this an "optimization" information in the sense that it is somehow optional (unlike args_to_skip or tree_map which we can ignore without miscompiling). Or do I somehow misunderstand the intended meaning of the cgraphopt section? Thanks, Martin
> I guess that I'd have to add a vector of edges to lto_cgraph_encoder_d > and to input_cgraph, input_cgraph_opt_summary and its callees. Hardly > something to be pre-approved :-) > > I've already written some of this but before I finish it, test it and > re-post, I'd like to ask you to re-consider. If we ever build call > graph edges for calls to thunks (especially as we plan to be able to > do it when building the graph), such thunk information would become an > integral part of an edge, without which the edge has an invalid > meaning. I'd therefore not consider this an "optimization" > information in the sense that it is somehow optional (unlike > args_to_skip or tree_map which we can ignore without miscompiling). > > Or do I somehow misunderstand the intended meaning of the cgraphopt > section? Well, cgraphopt stands for cgraph optimization summary that is everything streamed down from wpa to ltrans but not from lgen to wpa. I see that we might want to store thunk info on edges to represent the direct calls to thunks, but this opens another cam of worms since what is thunk in one unit don't need to be tunk in another. So I think we rather want to lower the calls... Honza > > Thanks, > > Martin
On Mon, Dec 27, 2010 at 01:39:43AM +0100, Jan Hubicka wrote: > > I guess that I'd have to add a vector of edges to lto_cgraph_encoder_d > > and to input_cgraph, input_cgraph_opt_summary and its callees. Hardly > > something to be pre-approved :-) > > > > I've already written some of this but before I finish it, test it and > > re-post, I'd like to ask you to re-consider. If we ever build call > > graph edges for calls to thunks (especially as we plan to be able to > > do it when building the graph), such thunk information would become an > > integral part of an edge, without which the edge has an invalid > > meaning. I'd therefore not consider this an "optimization" > > information in the sense that it is somehow optional (unlike > > args_to_skip or tree_map which we can ignore without miscompiling). > > > > Or do I somehow misunderstand the intended meaning of the cgraphopt > > section? > > Well, cgraphopt stands for cgraph optimization summary that is everything > streamed down from wpa to ltrans but not from lgen to wpa. > I see that we might want to store thunk info on edges to represent the > direct calls to thunks, but this opens another cam of worms since what > is thunk in one unit don't need to be tunk in another. So I think we rather > want to lower the calls... Fair enough. However, if we try to lower the calls to thunks up front, we might actually want to keep the thunk_delta field in cgraph_indirect_call_info in order to save memory for the vast majority of call graph edges that have nothing to do with thunks (but still convert it to HOST_WIDE_INT and stream as such in cgraphopt). I'll continue working on this along these lines then. Thanks, Martin
Index: icln/gcc/cgraph.c =================================================================== --- icln.orig/gcc/cgraph.c +++ icln/gcc/cgraph.c @@ -860,7 +860,7 @@ cgraph_set_call_stmt (struct cgraph_edge indirect call into a direct one. */ struct cgraph_node *new_callee = cgraph_node (decl); - cgraph_make_edge_direct (e, new_callee, NULL); + cgraph_make_edge_direct (e, new_callee, 0); } push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl)); @@ -1022,6 +1022,7 @@ cgraph_create_edge_1 (struct cgraph_node edge->next_caller = NULL; edge->prev_callee = NULL; edge->next_callee = NULL; + edge->thunk_delta = 0; edge->count = count; gcc_assert (count >= 0); @@ -1195,15 +1196,15 @@ cgraph_redirect_edge_callee (struct cgra } /* Make an indirect EDGE with an unknown callee an ordinary edge leading to - CALLEE. DELTA, if non-NULL, is an integer constant that is to be added to - the this pointer (first parameter). */ + CALLEE. DELTA is an integer constant that is to be added to the this + pointer (first parameter) to compensate for skipping a thunk adjustment. */ void cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee, - tree delta) + HOST_WIDE_INT delta) { edge->indirect_unknown_callee = 0; - edge->indirect_info->thunk_delta = delta; + edge->thunk_delta = delta; /* Get the edge out of the indirect edge list. */ if (edge->prev_callee) @@ -2130,6 +2131,7 @@ cgraph_clone_edge (struct cgraph_edge *e } } + new_edge->thunk_delta = e->thunk_delta; new_edge->inline_failed = e->inline_failed; new_edge->indirect_inlining_edge = e->indirect_inlining_edge; new_edge->lto_stmt_uid = stmt_uid; Index: icln/gcc/cgraph.h =================================================================== --- icln.orig/gcc/cgraph.h +++ icln/gcc/cgraph.h @@ -388,9 +388,6 @@ struct GTY(()) cgraph_indirect_call_info HOST_WIDE_INT otr_token; /* Type of the object from OBJ_TYPE_REF_OBJECT. */ tree otr_type; - /* Delta by which must be added to this parameter. For polymorphic calls - only. */ - tree thunk_delta; /* Index of the parameter that is called. */ int param_index; /* ECF flags determined from the caller. */ @@ -404,6 +401,10 @@ struct GTY(()) cgraph_indirect_call_info struct GTY((chain_next ("%h.next_caller"), chain_prev ("%h.prev_caller"))) cgraph_edge { /* Expected number of executions: calculated in profile.c. */ gcov_type count; + /* Delta by which must be added to this parameter to compensate for a skipped + this adjusting thunk. */ + HOST_WIDE_INT thunk_delta; + struct cgraph_node *caller; struct cgraph_node *callee; struct cgraph_edge *prev_caller; @@ -578,7 +579,8 @@ struct cgraph_node * cgraph_clone_node ( int, bool, VEC(cgraph_edge_p,heap) *); void cgraph_redirect_edge_callee (struct cgraph_edge *, struct cgraph_node *); -void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *, tree); +void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *, + HOST_WIDE_INT); struct cgraph_asm_node *cgraph_add_asm_node (tree); Index: icln/gcc/cgraphunit.c =================================================================== --- icln.orig/gcc/cgraphunit.c +++ icln/gcc/cgraphunit.c @@ -2168,22 +2168,18 @@ cgraph_redirect_edge_call_stmt_to_callee } } - if (e->indirect_info && e->indirect_info->thunk_delta - && integer_nonzerop (e->indirect_info->thunk_delta) + if (e->thunk_delta != 0 && (!e->callee->clone.combined_args_to_skip || !bitmap_bit_p (e->callee->clone.combined_args_to_skip, 0))) { if (cgraph_dump_file) - { - fprintf (cgraph_dump_file, " Thunk delta is "); - print_generic_expr (cgraph_dump_file, - e->indirect_info->thunk_delta, 0); - fprintf (cgraph_dump_file, "\n"); - } + fprintf (cgraph_dump_file, " Thunk delta is " + HOST_WIDE_INT_PRINT_DEC "\n", e->thunk_delta); gsi = gsi_for_stmt (e->call_stmt); gsi_computed = true; - gimple_adjust_this_by_delta (&gsi, e->indirect_info->thunk_delta); - e->indirect_info->thunk_delta = NULL_TREE; + gimple_adjust_this_by_delta (&gsi, build_int_cst (sizetype, + e->thunk_delta)); + e->thunk_delta = 0; } if (e->callee->clone.combined_args_to_skip) Index: icln/gcc/ipa-prop.c =================================================================== --- icln.orig/gcc/ipa-prop.c +++ icln/gcc/ipa-prop.c @@ -1483,7 +1483,7 @@ ipa_make_edge_direct_to_target (struct c return NULL; ipa_check_create_node_params (); - cgraph_make_edge_direct (ie, callee, delta); + cgraph_make_edge_direct (ie, callee, delta ? tree_low_cst (delta, 0) : 0); if (dump_file) { fprintf (dump_file, "ipa-prop: Discovered %s call to a known target " @@ -2549,7 +2549,6 @@ ipa_write_indirect_edge_info (struct out { lto_output_sleb128_stream (ob->main_stream, ii->otr_token); lto_output_tree (ob, ii->otr_type, true); - lto_output_tree (ob, ii->thunk_delta, true); } } @@ -2572,7 +2571,6 @@ ipa_read_indirect_edge_info (struct lto_ { ii->otr_token = (HOST_WIDE_INT) lto_input_sleb128 (ib); ii->otr_type = lto_input_tree (ib, data_in); - ii->thunk_delta = lto_input_tree (ib, data_in); } } Index: icln/gcc/lto-cgraph.c =================================================================== --- icln.orig/gcc/lto-cgraph.c +++ icln/gcc/lto-cgraph.c @@ -278,6 +278,7 @@ lto_output_edge (struct lto_simple_outpu } lto_output_sleb128_stream (ob->main_stream, edge->count); + lto_output_sleb128_stream (ob->main_stream, edge->thunk_delta); bp = bitpack_create (ob->main_stream); uid = (!gimple_has_body_p (edge->caller->decl) @@ -1210,6 +1211,7 @@ input_edge (struct lto_input_block *ib, struct cgraph_edge *edge; unsigned int stmt_id; gcov_type count; + HOST_WIDE_INT thunk_delta; int freq; unsigned int nest; cgraph_inline_failed_t inline_failed; @@ -1230,6 +1232,7 @@ input_edge (struct lto_input_block *ib, callee = NULL; count = (gcov_type) lto_input_sleb128 (ib); + thunk_delta = lto_input_sleb128 (ib); bp = lto_input_bitpack (ib); stmt_id = (unsigned int) bp_unpack_value (&bp, HOST_BITS_PER_INT); @@ -1243,6 +1246,7 @@ input_edge (struct lto_input_block *ib, else edge = cgraph_create_edge (caller, callee, NULL, count, freq, nest); + edge->thunk_delta = thunk_delta; edge->indirect_inlining_edge = bp_unpack_value (&bp, 1); edge->lto_stmt_uid = stmt_id; edge->inline_failed = inline_failed; Index: icln/gcc/testsuite/g++.dg/ipa/pr46984.C =================================================================== --- /dev/null +++ icln/gcc/testsuite/g++.dg/ipa/pr46984.C @@ -0,0 +1,62 @@ +// { dg-options "-O -fipa-cp -fno-early-inlining -flto" } +// { dg-do run } + +extern "C" void abort (); + +class A +{ +public: + virtual void foo () {abort();} +}; + +class B : public A +{ +public: + int z; + virtual void foo () {abort();} +}; + +class C : public A +{ +public: + void *a[32]; + unsigned long b; + long c[32]; + + virtual void foo () {abort();} +}; + +class D : public C, public B +{ +public: + D () : C(), B() + { + int i; + for (i = 0; i < 32; i++) + { + a[i] = (void *) 0; + c[i] = 0; + } + b = 0xaaaa; + } + + virtual void foo (); +}; + +void D::foo() +{ + if (b != 0xaaaa) + abort(); +} + +static inline void bar (B &b) +{ + b.foo (); +} + +int main() +{ + D d; + bar (d); + return 0; +}