<div class="gmail_quote"><div>Hi Duncan,</div><div><br></div><div>Here's a bit of a long delayed reply, I've been thinking about this for a bit now and have a bit (I think) of a more firm idea on how I see this.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Always takes a patch to draw people in :).<br>
<br></blockquote><div><br></div><div>Agreed. I've been looking at the patches as they've been going in. I think it's mostly making me hate the Value->User hierarchy.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> Using `Value` instead of `MDNode`<br>
> ==============================<u></u>===<br>
><br>
> A number of APIs expect `MDNode` -- previously, the only referenceable<br>
> type of metadata -- but this patch (and the ones that will follow) have<br>
> referenceable metadata that *do not* inherit from `MDNode`.  Metadata<br>
> APIs such as `Instruction::getMetadata()` and<br>
> `NamedMDNode::getOperand()` need to return non-`MDNode` metadata.<br>
><br>
> To me, this change is a red flag<br>
<br>
This bothers me too -- which is why I highlighted it -- but I don't<br>
see any alternatives.  It seems like a natural fallout of the rest of<br>
the proposal.<br>
<br></blockquote><div><br></div><div>This hurts a lot. It's by far the worst part of this currently.</div><div><br></div><div>I think ideally I'd almost rather see Metadata not inherit from Value. Metadata doesn't need to be typed, it describes typed things. It isn't or doesn't produce a value, it's just a side structure that we find worth serializing. A container of some bits that can reference IR when necessary. </div><div><br></div><div>The problematic thing is the User-ness of Metadata. It definitely wants to point to Values/Users, it definitely can use a list of where it's used for things like RAUW - which we still use for temporary MDNodes right now. I think that aspect is a good thing, but it doesn't necessarily need to be blazingly fast.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
`MDUser`/`CustomMD` inherits from `User` so that its subclasses can<br>
leverage the use-list infrastructure (8B per operand, fast RAUW).<br>
<br>
> I also have to ask because I can't currently see it: what does debug info being metadata buy us?<br>
<br>
I suppose it buys us:<br>
<br>
  - the guarantee that debug info doesn't modify optimizations or code<br>
    generation, and<br>
<br>
  - the flexibility for optimizations to ignore/drop it when they're<br>
    not smart enough to update it.<br></blockquote><div><br></div><div>Agreed. I think this is important.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> How much code is simplified by that, and at what cost?<br>
<br>
I think that's hard to quantify.<br>
<br></blockquote><div><br></div><div>Quite a bit IMO. If we could get rid of dbg_declare and dbg_value it'd be even more. Ideally, as we've discussed in the past, these could be metadata on instructions or <things that turn into lexical blocks>. Some work would need to be done to handle this, but we'd get rid of the instructions out of the IR.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I suppose the obvious alternative is to rewrite debug info from the<br>
ground up, without using metadata at all.  I considered this, but<br>
didn't find a compelling argument for it.  Main arguments against it:<br>
it would be harder to implement incrementally, and it would increase<br>
the amount of non-code IR.<br>
<br>
Moreover, once we have specific subclasses and bitcode support for<br>
debug info types, moving away from metadata (or even the `Value`<br>
hierarchy entirely) would be an incremental step.<br>
<br>
Do you have any specific alternatives mind?<br></blockquote><div><br></div><div>How about the above discussion? I think it might end up being a bit more work in the short term, but it moves metadata to a separate hierarchy. I don't think a fast RAUW is too important (call it once per type - I think we RAUW much more on instructions/constants/etc and so speed is an advantage there). Module already has a separate set of iterators for named metadata so that side of things isn't too bad.</div><div><br></div><div>Thoughts?</div><div><br></div><div>-eric</div></div>