[PATCH] IR: Add specialized debug info metadata nodes

David Blaikie dblaikie at gmail.com
Fri Feb 6 10:36:08 PST 2015


On Wed, Feb 4, 2015 at 4:17 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Feb 4, 2015, at 3:53 PM, Frédéric Riss <friss at apple.com> wrote:
>
>
> On Feb 4, 2015, at 3:46 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com>
> wrote:
>
>
> On 2015-Feb-04, at 15:37, Frédéric Riss <friss at apple.com> wrote:
>
>
> - The word 'context' is overloaded: `MDNode::getContext()` already
> exists, and returns an `LLVMContext&`; `DIDescriptor` uses 'context'
> to mean "the node that this one is defined inside".  I chose the
> word 'parent' instead of 'context' here.  Is this word okay?  If
> not, what about 'scope'?  This will be reflected in the assembly
> changes to come (I'd like the C++ names to match the assembly names,
> although technically it's not necessary).
>
> I'd /probably/ go with scope (we already have scope in the MDLocations, so
> that seems consistent), but fairly on-the-fence.
>
>
> Weirdly, I didn't even notice that :).  In that case I like 'scope'
> better too.  I'll update to that before commit.
>
>
> Seems most natural. Can the futur getScope() return something that doesn’t
> derive from MDScope?
>
>
> I don't think so.
>
>
> That was my impression also, and it makes it even more appropriate IMHO.
>
>
> Just a few general remarks to throw into the discussion:
>
> - Would it make sense to use something like tablegen to generate the
> repetitive parts? I’m slightly worried about copy&paste bugs, moderately
> worried about refactoring it in the future.
>
> [image: Inheritance graph]
>
> As for scopes, there are several things that bug me about the current
> class hierarchy that we could fix now:
> - DIFile should not be a scope (the concept of files is IMO orthogonal to
> scoping and there is always something more appropriate to put a node into:
> compile unit, module, namespace)
>

IIRC we do this when # line directives change the file-name part-way
through. But I could be wrong... only vague idea.


> - DIBasicType should not be scope
>

Part of this is the ability to treat all types the same - multiple
inheritance might be an alternative, but without that we want to treat all
types equally in some codepaths, but then only some types are valid scopes,
etc. - so there's a few different axes on which we want to refer to these
things. It's tricky. But certainly worth thinking about.


> - It’s questionable whether a DICompositetype should be a DIDerivedType
> - A DISubroutineType should be neither a DIDerivedType nor DIScope
> - Using DIDerivedTypes for CV qualifiers is a bit wasteful but it does map
> nicely to DWARF
>
> otherwise, thanks for doing this!
> -- adrian
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150206/34bb610a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: classllvm_1_1DIScope__inherit__graph.png
Type: image/png
Size: 28374 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150206/34bb610a/attachment.png>


More information about the llvm-commits mailing list