[PATCH] Add a DIExternalTypeRef debug metadata node to the IR.

[David Blaikie]

On Mon, May 11, 2015 at 2:42 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015 May 11, at 09:43, Adrian Prantl <aprantl at apple.com> wrote:
> >
> >>
> >> On May 9, 2015, at 6:55 AM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>
> >>
> >>> On 2015 May 8, at 17:38, Adrian Prantl <aprantl at apple.com> wrote:
> >>>
> >>>>
> >>>> On May 8, 2015, at 1:00 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>>>
> >>>>
> >>>>> On 2015 May 8, at 15:32, David Blaikie <dblaikie at gmail.com> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Fri, May 8, 2015 at 11:18 AM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >>>>> Hi dexonsmith, dblaikie, echristo,
> >>>>>
> >>>>> This adds a DIExternalTypeRef debug metadata node to the IR that is
> meant to be emitted by Clang when referring to a type that is defined in a
> .pcm file. DIExternalTypeRef is a child of DICompositeTypeBase because it
> shares common traits such as the unique identifier.
> >>>>>
> >>>>> Could this be an attribute to the DIClassType/StructureType? (to
> emit the currently needed type unit references, we still should know the
> correct class/structure type tag) LLVM already knows to hash the ref
> identifier (mangled name) for the type unit identifier.
> >>>
> >>> Note that DIExternalTypeRef also knows the tag.
> >>>>
> >>>> I think they should be separate for now.  When we have an
> >>>> `identifier:` field in `DICompositeType` we use the
> >>>> string-based `DITypeRef`s instead of direct pointers.  I don't
> >>>> imagine there's any value in adding that indirection for these.
> >>>
> >>> It would make lookups slower, but otherwise it would be harmless.
> >>>
> >>>> (Although, this makes me wonder about an LTO scenario.  Imagine you
> >>>> have two translation units, and they both use a type identified by
> >>>> (e.g.) "my-type".  One of them pulled the type in from a module
> >>>> and has a `DIExternalTypeRef`, while the other translation unit
> >>>> didn't use modules and has a `DICompositeType`.  What should the
> >>>> debug info look like?  Is it okay if we end up with both?  If not,
> >>>> which one should win?
> >>>
> >>> This is a scenario that can only happen in C++. For maximum space
> efficiency you would want the external type ref to win or you will end up
> with two copies of the type (which isn’t otherwise harmful). If you are are
> using dsymutil, it will still unique the two copies.
> >>>
> >>>> Will these new nodes show up in the
> >>>> `retainedType:` list?)
> >>>
> >>> They will not show up in the retained types list.
> >>
> >> That's a show stopper for sharing with `DICompositeType`, at least
> >> until we get rid of type refs.
> >>
> >> There's logic scattered around the compiler that assumes that if a
> >> node is `DICompositeType` and it has a valid identifier field, then
> >> it should be referenced by string.  If something is referenced by
> >> a string, then it needs to be in the retained type list, or you
> >> can't convert the string back to a `DICompositeType`.
> >>
> >> I think it would be better to keep these classes distinct.  If it
> >> makes sense to, we can always merge them later (once we no longer
> >> have type refs).
> >
> > There is no hard reason for why the external type references couldn’t be
> added to the list of retained types. We could add code to the DWARF backend
> to not emit the “definitions” of external retained types.
>
> Okay, sure, but that seems like a hack.  To me it looks like all the
> IR-level and backend code that cares that something is a
> DICompositeType will need special handling based on whether it's an
> external type ref.


So far as I recall very little code cares about DICompositeType
specifically - it should just be one place, essentially, that fills out the
attributes/child DIEs of the type DIE. We'd have a special case there that
would be about as costly/shoehorned as the case for emitting a declaration.
(it seems like it's just a different kind of declaration - one with a
DW_AT_signature as well as the DW_AT_declaration and DW_AT_name)

Is there some complexity I'm missing here?


> It seems like it's just being shoehorned in.
>
> @David, if you still feel like they should be the same class, can you
> explain why?
>
> > I’m fine with either variant, with a preference to introducing a
> separate DIExternalTypeRef over having a very special DICompositeType that
> is distinguished by a flag.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150511/ade65bab/attachment.html>