[PATCH] IR: Add specialized debug info metadata nodes

[David Blaikie]

On Fri, Feb 6, 2015 at 2:37 PM, Sean Silva <chisophugis at gmail.com> wrote:

>
>
> 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).
>

Yeah, I was wondering about that, since DeclContext + Decl is pretty much
the same data representation problem (some decls are contexts for other
decls, some aren't) as we have here in debug info.


>
> (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/22e7bd1b/attachment.html>