[PATCH] IR: Add specialized debug info metadata nodes

[Sean Silva]

On Fri, Feb 6, 2015 at 1:20 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Feb-06, at 10:36, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > 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.
> >
> > <classllvm_1_1DIScope__inherit__graph.png>
> >
> > 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.
>
> LLVM-style RTTI doesn't handle multiple inheritance, which restricts
> us somewhat here.
>

It definitely does. Check out clang's DeclContext vs. Decl multiple
inheritance.

I'll update
http://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#the-contract-of-classof
to mention this (that is actually the original reason I added that section,
but it appears I forgot to add a concrete reference to MI and in particular
DeclContext vs. Decl).

(I haven't really been following the discussion as to what MI was being
proposed to be used for, but don't let the LLVM-style RTTI block you from
considering it as a possibility)

-- Sean Silva


>
> I imagine having a hierarchy like:
>
>   DIScopeBase
>    -> DIScope
>        -> DINamespace
>        -> DISubprogram
>    -> DIType
>        -> DIBasicType
>        -> DIDerivedType
>        -> DITypeScope
>            -> DICompositeType
>
> would get us most of the way there.
>
> Pointers can be `DIScopeBase`.  Call-sites that care about scopes
> can check:
>
>     assert(isa<DIScope>(N) || isa<DITypeScope>(N));
>
> (We can add `DIScopeBase::isValidScope()` or something to do this.)
>
> Or maybe we don't even need a `DIScopeBase`.
>
> >
> > - 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150206/2faa52cf/attachment.html>