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

[Eric Christopher]

>
>
> > I’m not following your algorithm intentions well yet, but this seems
> like a really heavy-weight implementation, given that this is a common base
> class for a number of other important things.
>
> Unfortunately I think this weight will be hard to optimize out (although
> I'm sure it could be a little smaller).
>
> In the `Value` hierarchy, you pay memory for RAUW at the site of each
> `Use`.  This makes sense, since most values can be RAUW'ed.
>
> Since very little metadata needs to be RAUW'ed, we don't want to pay
> memory at every use-site.  Instead, we pay the cost inside the
> (hopefully) few instances that support RAUW.
>

Agreed. I don't see us needing to RAUW debug info outside of llvm-link and
the forward references. For other metadata uses RAUW doesn't seem to be
used at all (at least a quick glance didn't show TempMDNodes anywhere).


>
> The core assumption is that there are relatively few replaceable
> metadata instances.  The only subclasses are `ValueAsMetadata` and
> `TempMDNode`.  How many of these are live?
>
>   - There will be at most one `ValueAsMetadata` instance for each
>     metadata operand that points at a `Value`.  My data from a couple of
>     months ago showed that there are very few of these.
>
>   - `TempMDNodes` are used as forward references.  You only pay their
>     cost until the forward reference gets resolved (i.e., deleted).
>
> The main case I'm worried about here are references to `ConstantInt`s,
> which are used pretty heavily outside of debug info (e.g., in `!prof`
> attachments).  However, if that shows up in a profile, we can bypass
> `Value`s entirely by creating a new `MDIntArray` subclass (or
> something).
>
>
This makes sense.


>
> The class hierarchy I envision looks something like this:
>
>     Metadata
>       MDNode
>         TempMDNode      // MDNodeFwdRef?
>         UniquableMDNode // GenericMDNode?
>         DINode          // Is this layer useful?
>           DILocation
>           DIScope
>             DIType
>               DIBasicType
>               DICompositeType
>               ...
>             DISubprogram
>             ...
>           DICompileUnit
>

FWIW technically a compile unit is a scope so would probably need to be
under that. Otherwise I'm missing a change rationale (I haven't read the
full patch yet, I apologize if it's clear from the patch).


>           ...
>       MDString
>       ValueAsMetadata
>
> `UniquableMDNode` is a leaf-class that behaves like the current `MDNode`
> (when it's not a temporary).  I called it "uniquable" because, unlike
> `TempMDNode`, it is uniqued by default (although you can opt-out of it,
> and uniquing might be dropped).
>
> Maybe a better name is `GenericMDNode`?
>
> Also, off-topic, but after sketching out the imagined hierarchy above,
> I'm not sure `DINode` is particularly useful.
>

Possibly? Might be from the "organizing all of the metadata types" level?

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141110/904545fc/attachment.html>