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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D29110#655948, @pcc wrote:

> > What about merging globals?
>
> http://llvm-cs.pcc.me.uk/lib/CodeGen/GlobalMerge.cpp#483


I believe this is incorrect.
The metadata being opaque you can't know that they don't prevent some merging in the absolute in the current situation.

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

This is why we explicitly have this difference between first class construct that can't be ignored and metadata that can be. That provides an easy way to not shoot ourselves in the foot and have an easy set of rules about what we can and can't do.

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

That still does not address that the metadata is opaque and carry any semantic that you don't about and may invalidate. Which is fine as long as metadata don't carry any correctness related semantic.


https://reviews.llvm.org/D29110