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

[Jan Hubicka <hubicka@ucw.cz>]

> 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