[PATCH] IR: Add specialized debug info metadata nodes

Duncan P. N. Exon Smith dexonsmith at apple.com
Fri Feb 6 13:20:40 PST 2015


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

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





More information about the llvm-commits mailing list