[LLVMdev] [RFC] Separating Metadata from the Value hierarchy

[Bob Wilson]

> On Nov 9, 2014, at 5:49 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
> FWIW, this completely addresses my only ill feelings about the changes you were making. I really like it.

I really like it, too.

Eric, thanks for the great suggestion!

> 
> I might bikeshed some of the names, but whatever. I would suggest maybe augmenting some of the doxygen comments to help show the specific use case that motivates the node? You already have this in a few places and it really helped me skimming the code. In particular, there are a bunch of very similar nodes around the tracking, temp, uniquable, etc. and I think it'll be important to clearly distinguish each use case that these are designed to address.
> 
> Also just want to say thanks for diving into the design and finding something (at least, I'm hoping!) works even better.
> 
> On Sun, Nov 9, 2014 at 7:02 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
> TL;DR: If you use metadata (especially if it's out-of-tree), check the
> numbered list of lost functionality below to see whether I'm trying to
> break your compiler permanently.
> 
> In response to my recent commits (e.g., [1]) that changed API from
> `MDNode` to `Value`, Eric had a really interesting idea [2] -- split
> metadata entirely from the `Value` hierarchy, and drop general support
> for metadata RAUW.
> 
> After hacking together some example code, this seems overwhelmingly to
> me like the right direction.  See the attached metadata-v2.patch for my
> sketch of what the current metadata primitives might look like in the
> new hierarchy (includes LLVMContextImpl uniquing support).
> 
> The initial motivation was to avoid the API weaknesses caused by having
> non-`MDNode` metadata that inherits from `Value`.  In particular,
> instead of changing API from `MDNode` to `Value`, change it to a base
> class called `Metadata`, which sheds the underused and expensive `Value`
> base class entirely.
> 
> The implications are broader: an enormous reduction in complexity (and
> overhead) for metadata.
> 
> This change implies minor (major?) loss of functionality for metadata,
> but Eric and I suspect that the only hard-to-fix user is debug info
> itself, whose IR infrastructure I'm rewriting anyway.
> 
> Here is what we'd lose:
> 
>  1. No more arbitrary RAUW of metadata.
> 
>     While we'd keep support for RAUW of temporary MDNodes for use as
>     forward declarations (when parsing assembly or constructing cyclic
>     graphs), drop RAUW support for all other metadata.
> 
>     Note that we'd also keep support for RAUW of `Value` operands of
>     metadata.
> 
>     If the RAUW of an operand causes a uniquing collision, uniquing
>     would be dropped for that node.  This matches the current behaviour
>     when an operand goes to null.
> 
>     Upgrade path: none.
> 
>  2. No more function-local metadata.
> 
>     AFAICT, function-local metadata is *only* used for indirect
>     references to instructions and arguments in `@llvm.dbg.value` and
>     `@llvm.dbg.declare` intrinsics.  The first argument of the following
>     is an example:
> 
>         call void @llvm.dbg.value(metadata !{i32 %val}, metadata !0,
>                                   metadata !1)
> 
>     Note that the debug info people uniformly seem to dislike the status
>     quo, since it's awkward to get from a `Value` to the corresponding
>     intrinsic.
> 
>     Upgrade path: Instead of using an intrinsic that references a
>     function-local value through an `MDNode`, attach metadata to the
>     corresponding argument or instruction, or to the terminating
>     instruction of the basic block.  (This requires new support for
>     attaching metadata to function arguments, which I'll have to add for
>     debug info anyway.)
> 
> Is this going to break your compiler?  How?  Why is your use case worth
> supporting?
> 
> [1]: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141103/242667.html <http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141103/242667.html>
>     "r221167 - IR: MDNode => Value: Instruction::getAllMetadataOtherThanDebugLoc()"
> [2]: http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-November/078581.html <http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-November/078581.html>
>     "Re: First-class debug info IR: MDLocation"
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu>         http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141109/d064c823/attachment.html>