At linktime, consider linkonces as ones that will go away

Michael Matz matz@suse.de
Thu Jul 8 15:37:00 GMT 2010


Hi,

On Thu, 8 Jul 2010, Jan Hubicka wrote:

> Well, it is all about stuble differences ;)) Sadly those differences 
> matter a lot in practice, this is why I worry about them at first place.
> 
> Function that is only caled directly can have API changes,

I wasn't talking about the other predicates which are for uses other than 
determining the removability.  I was only talking about the three 
_can_remove_ predicates and why there are three of them.

> So cgraph_only_called_directly

In particular I wasn't asking about this predicate.

> The difference in between cgraph_can_remove_if_no_direct_calls_p and 
> cgraph_can_remove_if_no_direct_calls_and_refs_p is whether pass cares of 
> indirect calls too.  I could make ipa-inline to use 
> cgraph_can_remove_if_no_direct_calls_and_refs_p and test if address is 
> taken, but since the former existed longer and I think it could be 
> useful elsewhere I decided to keep it.

What's completely confusing is, why _and_refs does _not_ take into account 
node.address_taken.  After all if the address is taken (from whereever) we 
can not remove the function (no matter if no direct calls, or other refs, 
or anything else remains or can be ignored), but still 
cgraph_can_remove_if_no_direct_calls_and_refs_p could return true.  

The two _can_remove (and the new one) are confusing 
because they seem to be strange conditional predicates, where certain 
conditions are established at the callers of those predicates and I have 
a hard time figuring out what those conditions are exactly, and why the 
predicates are named in a way that doesn't reflect what they say, and why 
there are different such sets of condition (requiring different can_remove 
predicates).

> The new predicate is used in cases where you want to see if doing code 
> duplication eliminating direct calls to given function will make the 
> function to disappear from program (not only current unit as 
> cgraph_can_remove_if_no_direct_calls_p).  So it is useful for metrics 
> deciding on how those transforms affect program size, but not for actual 
> removal of function.

Okay, so the _from_program in your initial name did make sense (as in 
different to a hypothetical _from_unit).  So my 
cgraph_remove_from_program_p suggestion still stands.  You could then at 
least rename cgraph_can_remove_if_no_direct_calls_p into 
cgraph_remove_from_unit_p (and I hope that remove_from_program 
implies remove_from_unit).

As for the difference between _and_refs_p to the one without _and_refs_p 
see above for my confusion.

> > In addition the pre-existing two predicates are each used exactly once, 
> > and all uses are itself in between a number of conditions.  This just adds 
> > the confusion between the different predicates (for instance why the 
> > surrounding condition isn't part of the predicate), e.g. here:
> > 
> >       if (!e->callee->callers->next_caller
> >           && cgraph_can_remove_if_no_direct_calls_p (e->callee)
> >           /* Don't reuse if more than one function shares a comdat group.
> >              If the other function(s) are needed, we need to emit even
> >              this function out of line.  */
> >           && !e->callee->same_comdat_group
> > 
> > So, why do you need three cgraph_can_remove predicates (and why don't they 
> > contain all conditions necessary?)
> 
> Well the code test if the function can be removed and has precisely 1 
> call and has no comdat group.  First test is because we know we are 
> going to inline that last call and the second test because inliner is 
> stupid and is not able to deal with comdat groups well.
> 
> This probably can't be broken up into independent predicate with 
> resonable meaning.

As this is the only user of cgraph_can_remove_if_no_direct_calls_p() the 
last test (!e->callee->same_comdat_group) can be merged into the predicate 
itself.  After all, it does belong into it: with inliner being 
stupid we cannot remove the function if e->callee->same_comdat_group.


Ciao,
Michael.



More information about the Gcc-patches mailing list