Re: [PATCH, PR 46984] Move thunk_delta from cgraph_indirect_call_info to cgraph_edge

[Martin Jambor <mjambor at suse dot cz>]

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