[PATCH] D29110: LangRef: Document the allowed metadata dropping transforms.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D29110#655887, @mehdi_amini wrote:

> In https://reviews.llvm.org/D29110#655763, @pcc wrote:
>
> > In https://reviews.llvm.org/D29110#655741, @mehdi_amini wrote:
> >
> > > I believe the implication of this hasn't been considered enough, and ultimately it means that any pass that looks at a global needs to understand the associated metadata.
> >
> >
> > Only passes that rebuild globals without dropping them
>
>
> What do you mean "without dropping them"? Aren't you making illegal to drop them?


I meant without dropping the globals.

>> There is already support for copying metadata when rebuilding globals (see `GlobalObject::copyMetadata`). Look at its callers [0] for how often it is needed. Somehow we've been able to launch features which rely on global-metadata-for-correctness (e.g. CFI and devirtualization on Linux Chrome) with that level of support.
> 
> What about merging globals?

http://llvm-cs.pcc.me.uk/lib/CodeGen/GlobalMerge.cpp#483

> What about shrinking a global based on the uses? (Int-> bool for example).

GlobalSplit has custom code for copying type metadata. I was curious to see if there were any other passes like that, and found a global sroa pass in GlobalOpt:
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/GlobalOpt.cpp#457
So I guess we need to move that code into a central location.

> 
> 
>> Ultimately I care about keeping things simple.
> 
> I'm not convinced simplicity has to go before soundness.

Where did you get the idea that I am trying to make things unsound? Of course I want a design that is both simple and sound.

>> Should we invent some other way of representing type information? I suppose we could, but that would seem to involve a significant parallel infrastructure to metadata and we wouldn't be in much of a better place to where we started, i.e. we would still need to preserve whatever we replace type metadata with, etc.
> 
> Right, the problem is not that you're using metadata, the problem is that you put a burden on the rest of the compiler to understand your construct, that's a flaw in the way the feature itself is designed.

If it is possible to write a pass that copies a global without copying thread_local (or any other semantically significant attribute), would you say that attribute is unsound or flawed? Of course not. And there are ways of copying those attributes (copyAttributesFrom).

Maybe to reduce the overall burden we can teach copyAttributesFrom to copy metadata? Then passes would just need to call one function.

>> Perhaps a better way of thinking about whether to keep metadata is by analogy to linkage. We can drop unreferenced globals with internal linkage without breaking semantics, but not external or weak globals. In principle we could add a similar concept for metadata attached to globals, but I don't see a pressing need at this point.
> 
> Sorry I don't understand the analogy here. Linkage have a clear semantic and a clear set of rules. Metadata don't.

That is, unless we define them. Per above I think it can be done broadly in terms of copying.

> Dropping them is *not* the only problem, the problem is updating them when mutating the IR and keeping their semantic valid.

Right, part of that is what copyAttributesFrom does. By calling that function you are saying "I am moving some unspecified part of this global into some unspecified part of that global" (and if you look at the callers that's basically what they are doing). Another part is what copyMetadata does: "I am moving this entire global into this location in this other global". The idea would be to unify them and have a way of saying "I am moving this byte range in this global into this location in this other global".


https://reviews.llvm.org/D29110