diff mbox

[PR,45934,1/6,PR,46287] Do not generate direct calls to thunks

Message ID 20101201201710.617836287@virgil.suse.cz
State New
Headers show

Commit Message

Martin Jambor Dec. 1, 2010, 8:16 p.m. UTC
Hi,

this is a yet another ping for the patch below.  It has already been
approved by Richi but still awaits Honza's assessment.

Re-bootstrapped and re-tested on x86-64-linux this Monday.

Thanks,

Martin

----------------------------------------

Hi,

we currently cannot really represent direct calls to thunks in the
call graph (i.e. in the edges) and this means we can omit the
necessary thunk adjustments when using that edge in IPA
transformations.  This then causes problems filed as PR 46053 and PR
46287.  In 4.5 there were no such calls so it did not matter but with
devirtualization we currently have them and we either need to handle
them or avoid them.  The patch below does both.

Certainly, soon we will need to be able to represent this information
on the edges in the full generality.  In particular we will have to
once we start devirtualizing by folding loads from the VMT.  However,
dealing with result adjusting and virtual_offset adjusting thunks will
require more changes than I am comfortable to push in stage three,
mainly because they are nested but there will also be other unforseen
problems too.

Therefore, I currently handle the simple this adjustment thunks and
punt when a more complex one is discovered.  Calls to this adjusting
thunks are then converted to a POINTER_PLUS in the caller and a call
to the ordinary function declaration.  This way, we still don't have
any direct calls to thunks in the IL.  These adjustments in callees
are also performed during IPA inline and IPA-CP devirtualizations.

I do not perform the thunk check if the second argument of a folded
OBJ_TYPE_REF is an ADDR_EXPR of a simple declaration (as opposed to a
COMPONENT_REF) so that g++.dg/opt/devirt1.C passes.  If I did, the
thunk test would not be possible because there is no call graph node
for the called method (which is not defined in this compilation unit).
In this case there should be no thunks involved ( it is not a call to
an ancestor's virtual method) and it is what previous versions of gcc
do so we should not regress.

Bootstrapped and tested on x86-64-linux without any issues, a very
similar patch has also passed make check-c++ on i686.  OK for trunk?

Thanks,

Martin




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.

Comments

Jan Hubicka Dec. 1, 2010, 8:58 p.m. UTC | #1
> 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?
In thunk_info it is HOST_WIDE_INT, I would expect it to be here as well.

> +  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?

> 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?

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?

Honza
Martin Jambor Dec. 3, 2010, 1 p.m. UTC | #2
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
Jan Hubicka Dec. 14, 2010, 5:14 p.m. UTC | #3
> Hi,
> 
> > 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

I tought we do that via constant folding already.

> 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).

Hmm...
> 
> > 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.

Well, it is similar for the thunk code. In a way I preffer constants to be constants
as they are easier to handle in GGC/LTO etc. but I do not care too much.
> > 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)))

OK...
> 
> > 
> > > 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.

Will then IPA-CP handle correctly call to thunk that calls to real function
that just passes pointer to other function?
> 
> > 
> > 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?

OK.
Honza
> 
> Thanks,
> 
> Martin
Martin Jambor Dec. 15, 2010, 1:44 p.m. UTC | #4
Hi,

On Tue, Dec 14, 2010 at 06:14:05PM +0100, Jan Hubicka wrote:
> > Hi,
> > 
> > > 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
> 
> I tought we do that via constant folding already.

No, things would break just as they did break in the testcases if IPA
saw calls to thunk decls.

> 
> > 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).
> 
> Hmm...
> > 
> > > 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.
> 
> Well, it is similar for the thunk code. In a way I preffer constants
> to be constants as they are easier to handle in GGC/LTO etc. but I
> do not care too much.
> > > 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)))
> 
> OK...
> > 
> > > 
> > > > 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.
> 
> Will then IPA-CP handle correctly call to thunk that calls to real function
> that just passes pointer to other function?

IPA-CP never propagates constants along edges that it has made direct
(becase these edges did not participate in the analysis), if that is
the concern.  I agree that the final mechanism should be more general
and robust (and I guess we should meet for lunch early in January to
discuss it) but I certainly think this is good enough for 4.6.

> > 
> > > 
> > > 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?
> 
> OK.

Thanks a lot, I committed it as revision 167855.

Martin
Jan Hubicka Dec. 15, 2010, 2:38 p.m. UTC | #5
> Hi,
> 
> On Tue, Dec 14, 2010 at 06:14:05PM +0100, Jan Hubicka wrote:
> > > Hi,
> > > 
> > > > 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
> > 
> > I tought we do that via constant folding already.
> 
> No, things would break just as they did break in the testcases if IPA
> saw calls to thunk decls.

Well, I really think it is just matter of producing proper testcase.  Those
functions are in vtables and thus the constant folding should get to them.
> 
> IPA-CP never propagates constants along edges that it has made direct
> (becase these edges did not participate in the analysis), if that is
> the concern.  I agree that the final mechanism should be more general
> and robust (and I guess we should meet for lunch early in January to
> discuss it) but I certainly think this is good enough for 4.6.

Well, this is somewhat fragile - the jump functions should be valid. 
Hmm, so if early optimization devirutalize into thunk, we won't see this problem?
Can't we get problem when ipa-cp make some call direct and we propagate type though
it for furhter devirutalizatoin in inliner?

Honza
> 
> > > 
> > > > 
> > > > 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?
> > 
> > OK.
> 
> Thanks a lot, I committed it as revision 167855.
> 
> Martin
Martin Jambor Dec. 15, 2010, 4:46 p.m. UTC | #6
Hi,

On Wed, Dec 15, 2010 at 03:38:08PM +0100, Jan Hubicka wrote:
> > Hi,
> > 
> > On Tue, Dec 14, 2010 at 06:14:05PM +0100, Jan Hubicka wrote:
> > > > Hi,
> > > > 
> > > > > 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
> > > 
> > > I tought we do that via constant folding already.
> > 
> > No, things would break just as they did break in the testcases if IPA
> > saw calls to thunk decls.
> 
> Well, I really think it is just matter of producing proper testcase.  Those
> functions are in vtables and thus the constant folding should get to them.

I'll prepare a small patch for cgraph_rebuild that will ICE if that
happens (when checking is enabled).

> > 
> > IPA-CP never propagates constants along edges that it has made direct
> > (becase these edges did not participate in the analysis), if that is
> > the concern.  I agree that the final mechanism should be more general
> > and robust (and I guess we should meet for lunch early in January to
> > discuss it) but I certainly think this is good enough for 4.6.
> 
> Well, this is somewhat fragile - the jump functions should be
> valid. 

Jump functions are always valid.  This is because the previously
unknown edges did not participate in the lattice meet operations, and
so we cannot redirect them to the modified clone.

> Hmm, so if early optimization devirutalize into thunk, we won't see
> this problem?

We will.

> Can't we get problem when ipa-cp make some call direct and we
> propagate type through it for furhter devirutalizatoin in inliner?

Actually, we might miss those devirtualizations.

I know this is rather fragile in the sense that it depends on the fact
we do not perform certain optimizations, but exactly so is not doing
anything which is what we had until now.  And a proper framework to
address this will be rather intrusive.

Martin

> 
> Honza
> > 
> > > > 
> > > > > 
> > > > > 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?
> > > 
> > > OK.
> > 
> > Thanks a lot, I committed it as revision 167855.
> > 
> > Martin
H.J. Lu Dec. 17, 2010, 1:31 p.m. UTC | #7
On Wed, Dec 1, 2010 at 12:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> this is a yet another ping for the patch below.  It has already been
> approved by Richi but still awaits Honza's assessment.
>
> Re-bootstrapped and re-tested on x86-64-linux this Monday.
>
> Thanks,
>
> Martin
>
> ----------------------------------------
>
> Hi,
>
> we currently cannot really represent direct calls to thunks in the
> call graph (i.e. in the edges) and this means we can omit the
> necessary thunk adjustments when using that edge in IPA
> transformations.  This then causes problems filed as PR 46053 and PR
> 46287.  In 4.5 there were no such calls so it did not matter but with
> devirtualization we currently have them and we either need to handle
> them or avoid them.  The patch below does both.
>
> Certainly, soon we will need to be able to represent this information
> on the edges in the full generality.  In particular we will have to
> once we start devirtualizing by folding loads from the VMT.  However,
> dealing with result adjusting and virtual_offset adjusting thunks will
> require more changes than I am comfortable to push in stage three,
> mainly because they are nested but there will also be other unforseen
> problems too.
>
> Therefore, I currently handle the simple this adjustment thunks and
> punt when a more complex one is discovered.  Calls to this adjusting
> thunks are then converted to a POINTER_PLUS in the caller and a call
> to the ordinary function declaration.  This way, we still don't have
> any direct calls to thunks in the IL.  These adjustments in callees
> are also performed during IPA inline and IPA-CP devirtualizations.
>
> I do not perform the thunk check if the second argument of a folded
> OBJ_TYPE_REF is an ADDR_EXPR of a simple declaration (as opposed to a
> COMPONENT_REF) so that g++.dg/opt/devirt1.C passes.  If I did, the
> thunk test would not be possible because there is no call graph node
> for the called method (which is not defined in this compilation unit).
> In this case there should be no thunks involved ( it is not a call to
> an ancestor's virtual method) and it is what previous versions of gcc
> do so we should not regress.
>
> Bootstrapped and tested on x86-64-linux without any issues, a very
> similar patch has also passed make check-c++ on i686.  OK for trunk?
>
> Thanks,
>
> Martin
>
>
>
>
> 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.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46987


H.J.
diff mbox

Patch

Index: icln/gcc/cgraph.c
===================================================================
--- icln.orig/gcc/cgraph.c
+++ icln/gcc/cgraph.c
@@ -859,7 +859,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);
+      cgraph_make_edge_direct (e, new_callee, NULL);
     }
 
   push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl));
@@ -1194,12 +1194,15 @@  cgraph_redirect_edge_callee (struct cgra
 }
 
 /* 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;
 
   /* Get the edge out of the indirect edge list. */
   if (edge->prev_callee)
@@ -2115,8 +2118,16 @@  cgraph_clone_edge (struct cgraph_edge *e
 	}
     }
   else
-    new_edge = cgraph_create_edge (n, e->callee, call_stmt, count, freq,
-				   e->loop_nest + loop_nest);
+    {
+      new_edge = cgraph_create_edge (n, e->callee, call_stmt, count, freq,
+				     e->loop_nest + loop_nest);
+      if (e->indirect_info)
+	{
+	  new_edge->indirect_info
+	    = ggc_alloc_cleared_cgraph_indirect_call_info ();
+	  *new_edge->indirect_info = *e->indirect_info;
+	}
+    }
 
   new_edge->inline_failed = e->inline_failed;
   new_edge->indirect_inlining_edge = e->indirect_inlining_edge;
Index: icln/gcc/cgraph.h
===================================================================
--- icln.orig/gcc/cgraph.h
+++ icln/gcc/cgraph.h
@@ -385,6 +385,9 @@  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.  */
@@ -572,7 +575,7 @@  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 *);
+void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *, tree);
 
 struct cgraph_asm_node *cgraph_add_asm_node (tree);
 
Index: icln/gcc/cgraphunit.c
===================================================================
--- icln.orig/gcc/cgraphunit.c
+++ icln/gcc/cgraphunit.c
@@ -2130,6 +2130,8 @@  cgraph_redirect_edge_call_stmt_to_callee
 {
   tree decl = gimple_call_fndecl (e->call_stmt);
   gimple new_stmt;
+  gimple_stmt_iterator gsi;
+  bool gsi_computed = false;
 #ifdef ENABLE_CHECKING
   struct cgraph_node *node;
 #endif
@@ -2162,9 +2164,24 @@  cgraph_redirect_edge_call_stmt_to_callee
 	}
     }
 
+  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;
+    }
+
   if (e->callee->clone.combined_args_to_skip)
     {
-      gimple_stmt_iterator gsi;
       int lp_nr;
 
       new_stmt
@@ -2176,7 +2193,8 @@  cgraph_redirect_edge_call_stmt_to_callee
 	  && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME)
 	SSA_NAME_DEF_STMT (gimple_vdef (new_stmt)) = new_stmt;
 
-      gsi = gsi_for_stmt (e->call_stmt);
+      if (!gsi_computed)
+	gsi = gsi_for_stmt (e->call_stmt);
       gsi_replace (&gsi, new_stmt, false);
       /* We need to defer cleaning EH info on the new statement to
          fixup-cfg.  We may not have dominator information at this point
Index: icln/gcc/gimple-fold.c
===================================================================
--- icln.orig/gcc/gimple-fold.c
+++ icln/gcc/gimple-fold.c
@@ -1442,17 +1442,26 @@  gimple_get_relevant_ref_binfo (tree ref,
     }
 }
 
-/* Fold a OBJ_TYPE_REF expression to the address of a function. TOKEN is
-   integer form of OBJ_TYPE_REF_TOKEN of the reference expression.  KNOWN_BINFO
-   carries the binfo describing the true type of OBJ_TYPE_REF_OBJECT(REF).  */
+/* Return a declaration of a function which an OBJ_TYPE_REF references. TOKEN
+   is integer form of OBJ_TYPE_REF_TOKEN of the reference expression.
+   KNOWN_BINFO carries the binfo describing the true type of
+   OBJ_TYPE_REF_OBJECT(REF).  If a call to the function must be accompanied
+   with a this adjustment, the constant which should be added to this pointer
+   is stored to *DELTA.  If REFUSE_THUNKS is true, return NULL if the function
+   is a thunk (other than a this adjustment which is dealt with by DELTA). */
 
 tree
-gimple_fold_obj_type_ref_known_binfo (HOST_WIDE_INT token, tree known_binfo)
+gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT token, tree known_binfo,
+				  tree *delta, bool refuse_thunks)
 {
   HOST_WIDE_INT i;
-  tree v, fndecl, delta;
+  tree v, fndecl;
+  struct cgraph_node *node;
 
   v = BINFO_VIRTUALS (known_binfo);
+  /* If there is no virtual methods leave the OBJ_TYPE_REF alone.  */
+  if (!v)
+    return NULL_TREE;
   i = 0;
   while (i != token)
     {
@@ -1462,62 +1471,91 @@  gimple_fold_obj_type_ref_known_binfo (HO
     }
 
   fndecl = TREE_VALUE (v);
-  delta = TREE_PURPOSE (v);
-  gcc_assert (host_integerp (delta, 0));
-
-  if (integer_nonzerop (delta))
-    {
-      struct cgraph_node *node = cgraph_get_node (fndecl);
-      HOST_WIDE_INT off = tree_low_cst (delta, 0);
-
-      if (!node)
-        return NULL;
-      for (node = node->same_body; node; node = node->next)
-        if (node->thunk.thunk_p && off == node->thunk.fixed_offset)
-          break;
-      if (node)
-        fndecl = node->decl;
-      else
-        return NULL;
-     }
+  node = cgraph_get_node_or_alias (fndecl);
+  if (refuse_thunks
+      && (!node
+    /* Bail out if it is a thunk declaration.  Since simple this_adjusting
+       thunks are represented by a constant in TREE_PURPOSE of items in
+       BINFO_VIRTUALS, this is a more complicate type which we cannot handle as
+       yet.
+
+       FIXME: Remove the following condition once we are able to represent
+       thunk information on call graph edges.  */
+	  || (node->same_body_alias && node->thunk.thunk_p)))
+    return NULL_TREE;
 
   /* When cgraph node is missing and function is not public, we cannot
      devirtualize.  This can happen in WHOPR when the actual method
      ends up in other partition, because we found devirtualization
      possibility too late.  */
-  if (!can_refer_decl_in_current_unit_p (fndecl))
-    return NULL;
-  return build_fold_addr_expr (fndecl);
+  if (!can_refer_decl_in_current_unit_p (TREE_VALUE (v)))
+    return NULL_TREE;
+
+  *delta = TREE_PURPOSE (v);
+  gcc_checking_assert (host_integerp (*delta, 0));
+  return fndecl;
 }
 
+/* Generate code adjusting the first parameter of a call statement determined
+   by GSI by DELTA.  */
 
-/* Fold a OBJ_TYPE_REF expression to the address of a function.  If KNOWN_TYPE
-   is not NULL_TREE, it is the true type of the outmost encapsulating object if
-   that comes from a pointer SSA_NAME.  If the true outmost encapsulating type
-   can be determined from a declaration OBJ_TYPE_REF_OBJECT(REF), it is used
-   regardless of KNOWN_TYPE (which thus can be NULL_TREE).  */
+void
+gimple_adjust_this_by_delta (gimple_stmt_iterator *gsi, tree delta)
+{
+  gimple call_stmt = gsi_stmt (*gsi);
+  tree parm, tmp;
+  gimple new_stmt;
+
+  delta = fold_convert (sizetype, delta);
+  gcc_assert (gimple_call_num_args (call_stmt) >= 1);
+  parm = gimple_call_arg (call_stmt, 0);
+  gcc_assert (POINTER_TYPE_P (TREE_TYPE (parm)));
+  tmp = create_tmp_var (TREE_TYPE (parm), NULL);
+  add_referenced_var (tmp);
+
+  tmp = make_ssa_name (tmp, NULL);
+  new_stmt = gimple_build_assign_with_ops (POINTER_PLUS_EXPR, tmp, parm, delta);
+  SSA_NAME_DEF_STMT (tmp) = new_stmt;
+  gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
+  gimple_call_set_arg (call_stmt, 0, tmp);
+}
 
-tree
-gimple_fold_obj_type_ref (tree ref, tree known_type)
+/* Fold a call statement to OBJ_TYPE_REF to a direct call, if possible.  GSI
+   determines the statement, generating new statements is allowed only if
+   INPLACE is false.  Return true iff the statement was changed.  */
+
+static bool
+gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi, bool inplace)
 {
+  gimple stmt = gsi_stmt (*gsi);
+  tree ref = gimple_call_fn (stmt);
   tree obj = OBJ_TYPE_REF_OBJECT (ref);
-  tree known_binfo = known_type ? TYPE_BINFO (known_type) : NULL_TREE;
-  tree binfo;
+  tree binfo, fndecl, delta;
+  HOST_WIDE_INT token;
 
   if (TREE_CODE (obj) == ADDR_EXPR)
     obj = TREE_OPERAND (obj, 0);
+  else
+    return false;
+
+  binfo = gimple_get_relevant_ref_binfo (obj, NULL_TREE);
+  if (!binfo)
+    return false;
+  token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
+  fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
+					     !DECL_P (obj));
+  if (!fndecl)
+    return false;
 
-  binfo = gimple_get_relevant_ref_binfo (obj, known_binfo);
-  if (binfo)
+  if (integer_nonzerop (delta))
     {
-      HOST_WIDE_INT token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
-      /* If there is no virtual methods leave the OBJ_TYPE_REF alone.  */
-      if (!BINFO_VIRTUALS (binfo))
-	return NULL_TREE;
-      return gimple_fold_obj_type_ref_known_binfo (token, binfo);
+      if (inplace)
+        return false;
+      gimple_adjust_this_by_delta (gsi, delta);
     }
-  else
-    return NULL_TREE;
+
+  gimple_call_set_fndecl (stmt, fndecl);
+  return true;
 }
 
 /* Attempt to fold a call statement referenced by the statement iterator GSI.
@@ -1525,8 +1563,8 @@  gimple_fold_obj_type_ref (tree ref, tree
    simplifies to a constant value. Return true if any changes were made.
    It is assumed that the operands have been previously folded.  */
 
-static bool
-fold_gimple_call (gimple_stmt_iterator *gsi, bool inplace)
+bool
+gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace)
 {
   gimple stmt = gsi_stmt (*gsi);
 
@@ -1552,18 +1590,8 @@  fold_gimple_call (gimple_stmt_iterator *
          copying EH region info to the new node.  Easier to just do it
          here where we can just smash the call operand.  */
       callee = gimple_call_fn (stmt);
-      if (TREE_CODE (callee) == OBJ_TYPE_REF
-          && TREE_CODE (OBJ_TYPE_REF_OBJECT (callee)) == ADDR_EXPR)
-        {
-          tree t;
-
-          t = gimple_fold_obj_type_ref (callee, NULL_TREE);
-          if (t)
-            {
-              gimple_call_set_fn (stmt, t);
-              return true;
-            }
-        }
+      if (TREE_CODE (callee) == OBJ_TYPE_REF)
+	return gimple_fold_obj_type_ref_call (gsi, inplace);
     }
 
   return false;
@@ -1617,7 +1645,7 @@  fold_stmt_1 (gimple_stmt_iterator *gsi,
 		changed = true;
 	      }
 	  }
-      changed |= fold_gimple_call (gsi, inplace);
+      changed |= gimple_fold_call (gsi, inplace);
       break;
 
     case GIMPLE_ASM:
Index: icln/gcc/gimple.h
===================================================================
--- icln.orig/gcc/gimple.h
+++ icln/gcc/gimple.h
@@ -891,10 +891,10 @@  unsigned get_gimple_rhs_num_ops (enum tr
 #define gimple_alloc(c, n) gimple_alloc_stat (c, n MEM_STAT_INFO)
 gimple gimple_alloc_stat (enum gimple_code, unsigned MEM_STAT_DECL);
 const char *gimple_decl_printable_name (tree, int);
-tree gimple_fold_obj_type_ref (tree, tree);
+bool gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace);
 tree gimple_get_relevant_ref_binfo (tree ref, tree known_binfo);
-tree gimple_fold_obj_type_ref_known_binfo (HOST_WIDE_INT, tree);
-
+tree gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT, tree, tree *, bool);
+void gimple_adjust_this_by_delta (gimple_stmt_iterator *, tree);
 /* Returns true iff T is a valid GIMPLE statement.  */
 extern bool is_gimple_stmt (tree);
 
Index: icln/gcc/ipa-cp.c
===================================================================
--- icln.orig/gcc/ipa-cp.c
+++ icln/gcc/ipa-cp.c
@@ -1214,7 +1214,7 @@  ipcp_process_devirtualization_opportunit
     {
       int param_index, types_count, j;
       HOST_WIDE_INT token;
-      tree target;
+      tree target, delta;
 
       next_ie = ie->next_callee;
       if (!ie->indirect_info->polymorphic)
@@ -1231,7 +1231,8 @@  ipcp_process_devirtualization_opportunit
       for (j = 0; j < types_count; j++)
 	{
 	  tree binfo = VEC_index (tree, info->params[param_index].types, j);
-	  tree t = gimple_fold_obj_type_ref_known_binfo (token, binfo);
+	  tree d;
+	  tree t = gimple_get_virt_mehtod_for_binfo (token, binfo, &d, true);
 
 	  if (!t)
 	    {
@@ -1239,8 +1240,11 @@  ipcp_process_devirtualization_opportunit
 	      break;
 	    }
 	  else if (!target)
-	    target = t;
-	  else if (target != t)
+	    {
+	      target = t;
+	      delta = d;
+	    }
+	  else if (target != t || !tree_int_cst_equal (delta, d))
 	    {
 	      target = NULL_TREE;
 	      break;
@@ -1248,7 +1252,7 @@  ipcp_process_devirtualization_opportunit
 	}
 
       if (target)
-	ipa_make_edge_direct_to_target (ie, target);
+	ipa_make_edge_direct_to_target (ie, target, delta);
     }
 }
 
@@ -1288,6 +1292,7 @@  ipcp_discover_new_direct_edges (struct c
   for (ie = node->indirect_calls; ie; ie = next_ie)
     {
       struct cgraph_indirect_call_info *ici = ie->indirect_info;
+      tree target, delta = NULL_TREE;
 
       next_ie = ie->next_callee;
       if (ici->param_index != index)
@@ -1307,12 +1312,15 @@  ipcp_discover_new_direct_edges (struct c
 	    continue;
 	  gcc_assert (ie->indirect_info->anc_offset == 0);
 	  token = ie->indirect_info->otr_token;
-	  cst = gimple_fold_obj_type_ref_known_binfo (token, binfo);
-	  if (!cst)
+	  target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
+						     true);
+	  if (!target)
 	    continue;
 	}
+      else
+	target = cst;
 
-      ipa_make_edge_direct_to_target (ie, cst);
+      ipa_make_edge_direct_to_target (ie, target, delta);
     }
 }
 
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -1458,35 +1458,43 @@  update_jump_functions_after_inlining (st
 }
 
 /* If TARGET is an addr_expr of a function declaration, make it the destination
-   of an indirect edge IE and return the edge.  Otherwise, return NULL.  */
+   of an indirect edge IE and return the edge.  Otherwise, return NULL.  Delta,
+   if non-NULL, is an integer constant that must be added to this pointer
+   (first parameter).  */
 
 struct cgraph_edge *
-ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target)
+ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target, tree delta)
 {
   struct cgraph_node *callee;
 
-  if (TREE_CODE (target) != ADDR_EXPR)
-    return NULL;
-  target = TREE_OPERAND (target, 0);
+  if (TREE_CODE (target) == ADDR_EXPR)
+    target = TREE_OPERAND (target, 0);
   if (TREE_CODE (target) != FUNCTION_DECL)
     return NULL;
   callee = cgraph_node (target);
   if (!callee)
     return NULL;
   ipa_check_create_node_params ();
-  cgraph_make_edge_direct (ie, callee);
+
+  cgraph_make_edge_direct (ie, callee, delta);
   if (dump_file)
     {
       fprintf (dump_file, "ipa-prop: Discovered %s call to a known target "
-	       "(%s/%i -> %s/%i) for stmt ",
+	       "(%s/%i -> %s/%i), for stmt ",
 	       ie->indirect_info->polymorphic ? "a virtual" : "an indirect",
 	       cgraph_node_name (ie->caller), ie->caller->uid,
 	       cgraph_node_name (ie->callee), ie->callee->uid);
-
       if (ie->call_stmt)
 	print_gimple_stmt (dump_file, ie->call_stmt, 2, TDF_SLIM);
       else
 	fprintf (dump_file, "with uid %i\n", ie->lto_stmt_uid);
+
+      if (delta)
+	{
+	  fprintf (dump_file, "          Thunk delta is ");
+	  print_generic_expr (dump_file, delta, 0);
+	  fprintf (dump_file, "\n");
+	}
     }
 
   if (ipa_get_cs_argument_count (IPA_EDGE_REF (ie))
@@ -1514,7 +1522,7 @@  try_make_edge_direct_simple_call (struct
   else
     return NULL;
 
-  return ipa_make_edge_direct_to_target (ie, target);
+  return ipa_make_edge_direct_to_target (ie, target, NULL_TREE);
 }
 
 /* Try to find a destination for indirect edge IE that corresponds to a
@@ -1526,7 +1534,7 @@  static struct cgraph_edge *
 try_make_edge_direct_virtual_call (struct cgraph_edge *ie,
 				   struct ipa_jump_func *jfunc)
 {
-  tree binfo, type, target;
+  tree binfo, type, target, delta;
   HOST_WIDE_INT token;
 
   if (jfunc->type == IPA_JF_KNOWN_TYPE)
@@ -1550,12 +1558,12 @@  try_make_edge_direct_virtual_call (struc
   type = ie->indirect_info->otr_type;
   binfo = get_binfo_at_offset (binfo, ie->indirect_info->anc_offset, type);
   if (binfo)
-    target = gimple_fold_obj_type_ref_known_binfo (token, binfo);
+    target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta, true);
   else
     return NULL;
 
   if (target)
-    return ipa_make_edge_direct_to_target (ie, target);
+    return ipa_make_edge_direct_to_target (ie, target, delta);
   else
     return NULL;
 }
@@ -2546,6 +2554,7 @@  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);
     }
 }
 
@@ -2568,6 +2577,7 @@  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/ipa-prop.h
===================================================================
--- icln.orig/gcc/ipa-prop.h
+++ icln/gcc/ipa-prop.h
@@ -430,7 +430,8 @@  bool ipa_propagate_indirect_call_infos (
 					VEC (cgraph_edge_p, heap) **new_edges);
 
 /* Indirect edge and binfo processing.  */
-struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree);
+struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree,
+						    tree);
 
 
 /* Debugging interface.  */
Index: icln/gcc/tree-ssa-ccp.c
===================================================================
--- icln.orig/gcc/tree-ssa-ccp.c
+++ icln/gcc/tree-ssa-ccp.c
@@ -2317,16 +2317,8 @@  ccp_fold_stmt (gimple_stmt_iterator *gsi
 	  {
 	    tree expr = OBJ_TYPE_REF_EXPR (callee);
 	    OBJ_TYPE_REF_EXPR (callee) = valueize_op (expr);
-	    if (TREE_CODE (OBJ_TYPE_REF_EXPR (callee)) == ADDR_EXPR)
-	      {
-		tree t;
-		t = gimple_fold_obj_type_ref (callee, NULL_TREE);
-		if (t)
-		  {
-		    gimple_call_set_fn (stmt, t);
-		    changed = true;
-		  }
-	      }
+	    if (gimple_fold_call (gsi, false))
+	      changed = true;
 	    OBJ_TYPE_REF_EXPR (callee) = expr;
 	  }
 
Index: icln/gcc/testsuite/g++.dg/ipa/pr46053.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46053.C
@@ -0,0 +1,41 @@ 
+/* { dg-do run } */
+/* { dg-options "-O -fipa-cp -fno-early-inlining"  } */
+
+extern "C" void abort ();
+
+struct A
+{
+  virtual void foo () = 0;
+};
+
+struct B : A
+{
+  virtual void foo () = 0;
+};
+
+struct C : A
+{
+};
+
+struct D : C, B
+{
+  int i;
+  D () : i(0xaaaa) {}
+  virtual void foo ()
+  {
+    if (i != 0xaaaa)
+      abort();
+  }
+};
+
+static inline void bar (B &b)
+{
+  b.foo ();
+}
+
+int main()
+{
+  D d;
+  bar (d);
+  return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46287-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46287-1.C
@@ -0,0 +1,67 @@ 
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+// { dg-options "-O" }
+
+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 ();
+};
+
+inline void D::foo()
+{
+  if (b != 0xaaaa)
+    abort();
+}
+
+static inline void bar (B &b)
+{
+
+  b.foo ();
+}
+
+int main()
+{
+  int i;
+  D d;
+
+  for (i = 0; i < 5000; i++)
+    bar (d);
+  return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46287-2.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46287-2.C
@@ -0,0 +1,68 @@ 
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+// { dg-options "-O -finline -finline-small-functions -finline-functions" }
+
+
+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()
+{
+  int i;
+  D d;
+
+  for (i = 0; i < 5000; i++)
+    bar (d);
+  return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46287-3.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46287-3.C
@@ -0,0 +1,67 @@ 
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+// { dg-options "-O -fipa-cp" }
+
+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 void bar (B &b)
+{
+
+  b.foo ();
+}
+
+int main()
+{
+  int i;
+  D d;
+
+  for (i = 0; i < 5000; i++)
+    bar (d);
+  return 0;
+}
Index: icln/gcc/testsuite/g++.dg/torture/covariant-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/covariant-1.C
@@ -0,0 +1,33 @@ 
+// { dg-do run }
+
+extern "C" void abort ();
+
+class A {
+public:
+  virtual A* getThis() { return this; }
+};
+
+class B {
+int a;
+public:
+  virtual B* getThis() { return this; }
+};
+
+class AB : public A, public B {
+public:
+  virtual AB* getThis() { return this; }
+};
+
+int main ()
+{
+  AB ab;
+
+  A* a = &ab;
+  B* b = &ab;
+
+  if (a->getThis() != a
+      || b->getThis() != b)
+    abort ();
+
+  return 0;
+}
Index: icln/gcc/testsuite/g++.dg/torture/pr46287.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/pr46287.C
@@ -0,0 +1,66 @@ 
+// Check that indirect calls to thunks do not lead to errors.
+// { 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()
+{
+  int i;
+  D d;
+
+  for (i = 0; i < 5000; i++)
+    bar (d);
+  return 0;
+}