[PATCH] D18634: Don't IPO over functions that can be de-refined

[Mehdi Amini via llvm-commits]

> On Mar 30, 2016, at 11:51 PM, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> 
> 
> Mehdi Amini wrote:
> > Same here: we could benefit from a good cleanup.
> > These predicates were designed with a model in mind, and the problem Sanjoy uncovered shows how this model is broken. I
> > suspect that comments like this one: "Using this method outside of the code generators is almost always a mistake"
> > discouraging the use of "isXXXForLinker" are reminiscent of the model.
> > Ideally we should rethink the set of abstract properties that are relevant, how they map to linkage types, and then
> > re-define the predicates (and revisit their uses).
> 
> This is how I see the problem at this time (repeats a bunch of things
> we've discussed in the past, but I'd rather be explicit here).
> 
> We have a set of linkage types, and you can group them in various
> ways.  Different groupings are useful in different contexts.
> 
> In the "IR optimization" context, the three groupings we're interested
> in are:

Thanks for summarizing that! This is exactly the kind of "abstract properties" I was referring to.


> 
> 1. Does this linkage type guarantee that the callee we see at runtime
>    is *exactly* the same as what we see during IR level optimization?
>    e.g. the internal linkage

If you want to support ELF function interposition, you can't answer this with linkage types. Otherwise "external" globals are in this category.

> 2. If not, does this linkage type guarantee that the callee we see at
>    runtime is some differently optimized variant of the source level
>    function the callee implements? [0] e.g. the linkonce_odr linkage
> 
> 3. The linkage type does not guarantee anything about what may be
>    substituted for the callee at runtime (i.e. the linker or loader
>    can basically do arbitrary function interposition). e.g. the
>    linkonce linkage
> (1) allows all IPO.  (2) allows inlining, but almost no IPO.  (3)
> allows no inlining, no IPO.
> 
> 
> The above classifications are not the same groupings we're interested
> in at a lower level (by "lower level", I mean places like
> X86Subtarget::ClassifyGlobalReference).  Some of the groupings we're
> interested in at this lower level (e.g. isStrongDefinitionForLinker)
> can *incidentally* be the same as one of the (1), (2) or (3); but to
> me that is more of "two things that just happened to be the same" than
> "two things that are the same due to some fundamental reason".

This is the part that is still not clear to me, I'd expect the logic to be similar and the kind of query to be on the same level as 1-2-3 above. And the answer should be the same at the IR level as well as at the backend level.
For instance I think X86Subtarget::ClassifyGlobalReference is interested in 1) above.


>  For instance, in the future, we could consider consistently marking some
> functions with linkage types in (2) as `optnone`, and (perhaps because
> we really want functionattrs to kick in for these functions) pretend
> that they were in (1).  But despite these functions being in (1) from
> the perspective of the IR context, their linkage type hasn't changed
> as far as the "lower level" view is concerned.

That sounds "black magic", and it seems to me that you can't handle this with predicates purely based on the linkage types. We'll probably need another abstraction (in the function class) for that.


> Thus, I see merit in keeping these two distinct views of of linkage
> types separated, even if there is some overlap in functionality.
> 
> Right now the mapping for the "IR optimization" context is
> 
> mayBeOverridden == (3)
> mayBeDerefined == (3) \union (2)
> NOT(mayBeDerefined) == (1)

Isn't it  "mayBeOverridden == (2)"?
(I ranted on IRC about how badly that named)


> I don't understand the backend sufficiently to enumerate what the
> current "lower level" classification is or should be.

Same.
Ideally a good start would be to have the same "high level description" as you did for backend purpose (CC Rafael and Eric maybe).


-- 
Mehdi



> 
> [0]: The callee we see during optimization is a correct implementation
> of source level function by assumption.
> 
> -- Sanjoy