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

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 25 12:35:21 PST 2017


pcc added a comment.

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

> In https://reviews.llvm.org/D29110#656629, @pcc wrote:
>
> > > Right, and there has always been a very clear rule about metadata attachment: *you can't use them for correctness*.
> >
> > Can you show me where that rule is documented for attachments on globals? I know that I did not have this rule in mind when I implemented them. I always viewed them as analogous to global named metadata (which already needs to be preserved for correctness) containing a reference to the global that they are attached to.
>
>
> This is the problem and this is what I told you on IRC... This slipped in with your addition on global MD attachment, without enough scrutiny IMO. 
>  There should have been a dedicated RFC about using Metadata attachment for correctness related thing. I don't believe this happened (At least Duncan and I missed the discussion on this specific aspect if it happened).
>
> >> Then there is also how one approach the design of a given feature in the first place: for instance the !associated does not have to carry correctness: for instance you can add the variable to compiler.user to model the correctness part, and use the MD to instruct a pass that it can ignore the compiler.used for a specific optimization.
> >>  I.e. the MD can be safely dropped, is preserved on a best-effort basis, and optionally passes that are taught about it can optimize better: there's nothing magic about it.
> > 
> > I like the idea of teaching GlobalDCE about llvm.compiler.used + associated, it would allow us to move that attribute out of the set of "attributes as MD*" without inhibiting DCE.
> > 
> > I'm not sure how that approach would work for type metadata though.
>
> I haven't thought about type metadata in particular, and I'm not against attaching a specific property to globals for what is currently the "type metadata", at least on the principle (reusing the MD infrastructure with a different API to differentiate is an option, I haven't given much though about it, and an approach like the one I mentioned for the "associated" is likely preferable when possible). 
>  What I'm worried about is mixing the generic opaque attachment that are considered to always be possible to be ignored with one that can't, for the same reasons exposed by Sanjoy and Krzysztof.


Fair enough. It seems like a reasonable goal to have straightforward ways to

1. copy information from one global to another while preserving correctness (something like the `copyAttributes` and `copyMetadata` I proposed earlier)
2. drop all the opaque attachments (metadata, or a subset of it, or whatever) while preserving correctness

As part of getting there, we can figure out the best way to move type and absolute_symbol into the right places in that mechanism.

Do you agree? If so, I will file a bug to keep track of that work.


https://reviews.llvm.org/D29110





More information about the llvm-commits mailing list