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

[David Blaikie]

On Mon, May 11, 2015 at 3:09 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
> > On May 11, 2015, at 3:04 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >
> >
> >> On 2015 May 11, at 14:51, David Blaikie <dblaikie at gmail.com> wrote:
> >>
> >>
> >>
> >> 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?
> >
> > Hmm.  I just did a `git grep DICompositeType`, and there's less
> > in the backend than I thought.
> >
> > I guess the only real problem I have is with the type refs.  It
> > doesn't make sense to me that we'd add these to the retained
> > type list.
>
> The fwddecl-style references (with full DeclContext + Name) that David
> referred to actually make some sense in the retained types list.


Not sure I follow how one (fwddecl style references) relate to the other
(retained types list)?


> As I recently noted in the thread about DIModule there are good reasons to
> emit them even on Darwin (they tell us which module a type originated from)


Don't follow this either - what does the type reference style (DW_TAG_type
+ DW_FORM_ref_sig8 compared to DW_TAG_type as a reference to a declaration
type DIE) relate to module origin and dsymutil's behavior?


> and let dsymutil optimize them away.
>
> -- adrian
>
> >
> >>
> >> 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/b30b0be0/attachment.html>