<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 9, 2014, at 5:49 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">FWIW, this completely addresses my only ill feelings about the changes you were making. I really like it.</div></div></blockquote><div><br class=""></div>I really like it, too.</div><div><br class=""></div><div>Eric, thanks for the great suggestion!</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">Also just want to say thanks for diving into the design and finding something (at least, I'm hoping!) works even better.</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Sun, Nov 9, 2014 at 7:02 PM, Duncan P. N. Exon Smith <span dir="ltr" class=""><<a href="mailto:dexonsmith@apple.com" target="_blank" class="">dexonsmith@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">TL;DR: If you use metadata (especially if it's out-of-tree), check the<br class="">
numbered list of lost functionality below to see whether I'm trying to<br class="">
break your compiler permanently.<br class="">
<br class="">
In response to my recent commits (e.g., [1]) that changed API from<br class="">
`MDNode` to `Value`, Eric had a really interesting idea [2] -- split<br class="">
metadata entirely from the `Value` hierarchy, and drop general support<br class="">
for metadata RAUW.<br class="">
<br class="">
After hacking together some example code, this seems overwhelmingly to<br class="">
me like the right direction.  See the attached metadata-v2.patch for my<br class="">
sketch of what the current metadata primitives might look like in the<br class="">
new hierarchy (includes LLVMContextImpl uniquing support).<br class="">
<br class="">
The initial motivation was to avoid the API weaknesses caused by having<br class="">
non-`MDNode` metadata that inherits from `Value`.  In particular,<br class="">
instead of changing API from `MDNode` to `Value`, change it to a base<br class="">
class called `Metadata`, which sheds the underused and expensive `Value`<br class="">
base class entirely.<br class="">
<br class="">
The implications are broader: an enormous reduction in complexity (and<br class="">
overhead) for metadata.<br class="">
<br class="">
This change implies minor (major?) loss of functionality for metadata,<br class="">
but Eric and I suspect that the only hard-to-fix user is debug info<br class="">
itself, whose IR infrastructure I'm rewriting anyway.<br class="">
<br class="">
Here is what we'd lose:<br class="">
<br class="">
 1. No more arbitrary RAUW of metadata.<br class="">
<br class="">
    While we'd keep support for RAUW of temporary MDNodes for use as<br class="">
    forward declarations (when parsing assembly or constructing cyclic<br class="">
    graphs), drop RAUW support for all other metadata.<br class="">
<br class="">
    Note that we'd also keep support for RAUW of `Value` operands of<br class="">
    metadata.<br class="">
<br class="">
    If the RAUW of an operand causes a uniquing collision, uniquing<br class="">
    would be dropped for that node.  This matches the current behaviour<br class="">
    when an operand goes to null.<br class="">
<br class="">
    Upgrade path: none.<br class="">
<br class="">
 2. No more function-local metadata.<br class="">
<br class="">
    AFAICT, function-local metadata is *only* used for indirect<br class="">
    references to instructions and arguments in `@llvm.dbg.value` and<br class="">
    `@llvm.dbg.declare` intrinsics.  The first argument of the following<br class="">
    is an example:<br class="">
<br class="">
        call void @llvm.dbg.value(metadata !{i32 %val}, metadata !0,<br class="">
                                  metadata !1)<br class="">
<br class="">
    Note that the debug info people uniformly seem to dislike the status<br class="">
    quo, since it's awkward to get from a `Value` to the corresponding<br class="">
    intrinsic.<br class="">
<br class="">
    Upgrade path: Instead of using an intrinsic that references a<br class="">
    function-local value through an `MDNode`, attach metadata to the<br class="">
    corresponding argument or instruction, or to the terminating<br class="">
    instruction of the basic block.  (This requires new support for<br class="">
    attaching metadata to function arguments, which I'll have to add for<br class="">
    debug info anyway.)<br class="">
<br class="">
Is this going to break your compiler?  How?  Why is your use case worth<br class="">
supporting?<br class="">
<br class="">
[1]: <a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141103/242667.html" target="_blank" class="">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141103/242667.html</a><br class="">
    "r221167 - IR: MDNode => Value: Instruction::getAllMetadataOtherThanDebugLoc()"<br class="">
[2]: <a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-November/078581.html" target="_blank" class="">http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-November/078581.html</a><br class="">
    "Re: First-class debug info IR: MDLocation"<br class="">
<br class="">
<br class="">_______________________________________________<br class="">
LLVM Developers mailing list<br class="">
<a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
<br class=""></blockquote></div><br class=""></div>
_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" class="">http://llvm.cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class=""></div></blockquote></div><br class=""></body></html>