<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 12, 2014 at 4:09 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2014 Dec 12, at 15:47, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Fri, Dec 12, 2014 at 2:37 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> It's horrible to inspect `MDNode`s in a debugger.  All of their operands<br>
> that are `MDNode`s get dumped as `<badref>`, since we can't assign<br>
> metadata slots in the context of a `Metadata::dump()`.  (Why not?  Why<br>
> not assign numbers lazily?  Because then each time you called `dump()`,<br>
> a given `MDNode` could have a different lazily assigned number.)<br>
><br>
> Fortunately, the C memory model gives us perfectly good identifiers for<br>
> `MDNode`.  Add pointer addresses to the dumps, transforming this:<br>
><br>
>     (lldb) e N->dump()<br>
>     metadata !{i32 662302, i32 26, metadata <badref>, null}<br>
><br>
>     (lldb) e ((MDNode*)N->getOperand(2))->dump()<br>
>     metadata !{i32 4, metadata !"foo"}<br>
><br>
> into:<br>
><br>
>     (lldb) e N->dump()<br>
>     metadata !{i32 662302, i32 26, metadata <badref:0x100706ee0>, null}<br>
><br>
>     (lldb) e ((MDNode*)0x100706ee0)->dump()<br>
>     metadata !{i32 4, metadata !"foo"}<br>
><br>
> and this:<br>
><br>
>     (lldb) e N->dump()<br>
>     metadata !{metadata <badref>, metadata <badref>, metadata <badref>, metadata <badref>, metadata <badref>}<br>
><br>
>     (lldb) e N->getOperand(0)<br>
>     (const llvm::MDOperand) $0 = {<br>
>       MD = 0x00000001012004e0<br>
>     }<br>
>     (lldb) e N->getOperand(1)<br>
>     (const llvm::MDOperand) $1 = {<br>
>       MD = 0x00000001012004e0<br>
>     }<br>
>     (lldb) e N->getOperand(2)<br>
>     (const llvm::MDOperand) $2 = {<br>
>       MD = 0x0000000101200058<br>
>     }<br>
>     (lldb) e N->getOperand(3)<br>
>     (const llvm::MDOperand) $3 = {<br>
>       MD = 0x00000001012004e0<br>
>     }<br>
>     (lldb) e N->getOperand(4)<br>
>     (const llvm::MDOperand) $4 = {<br>
>       MD = 0x0000000101200058<br>
>     }<br>
>     (lldb) e ((MDNode*)0x00000001012004e0)->dump()<br>
>     metadata !{}<br>
><br>
>     (lldb) e ((MDNode*)0x0000000101200058)->dump()<br>
>     metadata !{null}<br>
><br>
> into:<br>
><br>
>     (lldb) e N->dump()<br>
>     metadata !{metadata <badref:0x1012004e0>, metadata <badref:0x1012004e0>, metadata <badref:0x101200058>, metadata <badref:0x1012004e0>, metadata <badref:0x101200058>}<br>
><br>
>     (lldb) e ((MDNode*)0x1012004e0)->dump()<br>
>     metadata !{}<br>
><br>
>     (lldb) e ((MDNode*)0x101200058)->dump()<br>
>     metadata !{null}<br>
><br>
> I've left `badref` in the operand to keep it `FileCheck`-able, via:<br>
><br>
>     CHECK-NOT: badref<br>
><br>
> badref seems like a pretty weird anachronism & I don't see any instances of "CHECK.*badref" in 'test' (but it could be under other prefixes)<br>
><br>
><br>
> Another option would be:<br>
><br>
>     (lldb) e N->dump()<br>
>     metadata !{metadata !0x1012004e0, metadata !0x1012004e0, metadata !0x101200058, metadata !0x1012004e0, metadata !0x101200058}<br>
><br>
> which would allow:<br>
><br>
>     CHECK-NOT: !0x<br>
><br>
> I'd just imagine you'd check for "{.*metadata!" - which will be harder in your changes, but still possible I would imagine?<br>
><br>
><br>
> but IMO that's too subtle...<br>
><br>
> Not sure - seems almost less confusing than having 'badref' around.<br>
><br>
<br>
</div></div>`badref` is used consistently for `Value`s that don't have an assigned slot,<br>
but now that `Metadata` is separate it can probably do its own thing.<br>
<br>
There is only once use in LLVM, not sure about clang:<br>
<br>
    $ git grep badref<br>
    Assembler/2010-02-05-FunctionLocalMetadataBecomesNull.ll:; RUN: opt -O3 < %s | llvm-dis | not grep badref<br>
<br>
But this test should probably just CHECK for the expected result anyway.<br>
<br>
I'm pretty much happy with anything that gives a pointer address.  Here are<br>
some paint colours:<br>
<br>
    !{!0, null, <badref>}<br>
    !{!0, null, 0xabc}<br>
    !{!0, null, !0xabc}<br>
    !{!0, null, <0xabc>}<br>
    !{!0, null, !<0xabc>}<br>
    !{!0, null, <badref:0xabc>}<br></blockquote><div><br>I'd prefer not the 'badref' versions, and as you say, verbosity is a concern so that tends towards the more brief versions... <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Everyone okay with `!{!0, null, <0xabc>}`?</blockquote><div><br>I'm OK with that.<br><br>I kind of thought of ! as being the prefix for a value/metadata reference, but it does seem that the ! is sort of part of the numbered/named metadata, so it shouldn't necessarily appear for these address references. *shrug*</div></div></div></div>