This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
Hi,
On Wed, Dec 01, 2010 at 09:58:08PM +0100, Jan Hubicka wrote:
> > 2010-11-08 Martin Jambor <mjambor@suse.cz>
> >
> > PR tree-optimization/46053
> > PR middle-end/46287
> > * cgraph.h (cgraph_indirect_call_info): New field.
> > * gimple.h (gimple_fold_obj_type_ref): Declaration removed.
> > (gimple_fold_call): Declare.
> > (gimple_adjust_this_by_delta): Likewise.
> > * cgraph.c (cgraph_make_edge_direct): New parameter delta. Updated
> > all users.
> > (cgraph_clone_edge): Create a copy of indirect_info also for direct
> > edges.
> > * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Adjust this
> > parameters.
> > * gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Renamed to
> > gimple_get_virt_mehtod_for_binfo, new parameter delta. Do not search
> > through thunks, in fact bail out if we encounter one, check that
> > BINFO_VIRTUALS is not NULL.
> > (gimple_adjust_this_by_delta): New function.
> > (gimple_fold_obj_type_ref): Removed.
> > (gimple_fold_obj_type_ref_call): New function.
> > (fold_gimple_call): Renamed to gimple_fold_call, made external.
> > Updated users. Call gimple_fold_obj_type_ref_call instead of
> > gimple_fold_obj_type_ref.
> > * ipa-cp.c (ipcp_process_devirtualization_opportunities): Process
> > thunk deltas.
> > (ipcp_discover_new_direct_edges): Likewise.
> > * ipa-prop.c (ipa_make_edge_direct_to_target): New parameter delta.
> > Updated callers.
> > (ipa_write_indirect_edge_info): Stream thunk_delta.
> > (ipa_read_indirect_edge_info): Likewise.
> > * tree-ssa-ccp.c (ccp_fold_stmt): Use gimple_fold_call instead of
> > gimple_fold_obj_type_ref.
> >
> > * testsuite/g++.dg/ipa/pr46053.C: New test.
> > * testsuite/g++.dg/ipa/pr46287-1.C: Likewise.
> > * testsuite/g++.dg/ipa/pr46287-2.C: Likewise.
> > * testsuite/g++.dg/ipa/pr46287-3.C: Likewise.
> > * testsuite/g++.dg/torture/covariant-1.C: Likewise.
> > * testsuite/g++.dg/torture/pr46287.C: Likewise.
> >
>
> > /* Make an indirect EDGE with an unknown callee an ordinary edge leading to
> > - CALLEE. */
> > + CALLEE. DELTA, if non-NULL, is an integer constant that is to be added to
> > + the this pointer (first parameter). */
> >
> > void
> > -cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee)
> > +cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee,
> > + tree delta)
> > {
> > edge->indirect_unknown_callee = 0;
> > + edge->indirect_info->thunk_delta = delta;
>
> I guess it is still fine as it is definite improvement over current
> situation, but won't we need to handle all of cgraph_thunk_info
> here?
Eventually we will, certainly before we start propagating constants to
zeroth arguments of OBJ_TYPE_REFs. And not only here but also at
other places (e.g. cgraph_set_call_stmt and company... and of course
there is the weird cgraph_create_edge_including_clones I wrote you
about earlier).
> In thunk_info it is HOST_WIDE_INT, I would expect it to be here as well.
I extract it from BINFOs as tree constants and I need to use it in
POINTER_PLUSes as tree constants so there is probably no point in
striing it as H_W_I.
>
> > + if (e->indirect_info && e->indirect_info->thunk_delta
> > + && integer_nonzerop (e->indirect_info->thunk_delta))
> > + {
> > + 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");
> > + }
> > + 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;
> > + }
>
> Should've test here if this parameter was eliminated or not?
> (i.e. first bit of e->callee->clone.combined_args_to_skip?
It is not strictly necessary because I do the callee modification
before removing parameters but on the second thought, yes, we should
avoid creating unnecessary stuff when it is this easy. So I will add
the check and the condition will be:
if (e->indirect_info && e->indirect_info->thunk_delta
&& integer_nonzerop (e->indirect_info->thunk_delta)
&& (!e->callee->clone.combined_args_to_skip
|| !bitmap_bit_p (e->callee->clone.combined_args_to_skip, 0)))
>
> > Index: icln/gcc/ipa-prop.c
> > ===================================================================
> > --- icln.orig/gcc/ipa-prop.c
> > +++ icln/gcc/ipa-prop.c
>
> I would expect somewhere here to be code handling updating of operand
> value for non-0 delta when doing ipa-cp propagation, but don't seem to
> be able to find it?
What do you mean? IPA-CP stores the delta into the indirect info
structure of the corresponding edge and then the code in
gimple_adjust_this_by_delta performs the adjustment.
>
> The rest seems OK. I am most concerned that we implement just part of thunk
> logic, but I see that you get deltas from BINFOs and the rest of adjustments
> are not there?
>
Correct, for simple this adjusting thunks, BINFOs contain the decl of
the real function and the this delta separately, for more complex
thunks it stores the decl of an underlying thunk + a this delta which
would otherwise be adjusted in this thunk.
And yes, we will certainly have to re-think how to represent thunks in
the cgraph in a more general way.
Anyway, is the patch OK with the above change then?
Thanks,
Martin