<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 11, 2015 at 3:25 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On May 11, 2015, at 3:14 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 11, 2015 at 3:09 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
> On May 11, 2015, at 3:04 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>
><br>
><br>
>> On 2015 May 11, at 14:51, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Mon, May 11, 2015 at 2:42 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>
>><br>
>>> On 2015 May 11, at 09:43, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
>>><br>
>>>><br>
>>>> On May 9, 2015, at 6:55 AM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>
>>>><br>
>>>><br>
>>>>> On 2015 May 8, at 17:38, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
>>>>><br>
>>>>>><br>
>>>>>> On May 8, 2015, at 1:00 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> wrote:<br>
>>>>>><br>
>>>>>><br>
>>>>>>> On 2015 May 8, at 15:32, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>>>>>>><br>
>>>>>>><br>
>>>>>>><br>
>>>>>>> On Fri, May 8, 2015 at 11:18 AM, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
>>>>>>> Hi dexonsmith, dblaikie, echristo,<br>
>>>>>>><br>
>>>>>>> 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.<br>
>>>>>>><br>
>>>>>>> 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.<br>
>>>>><br>
>>>>> Note that DIExternalTypeRef also knows the tag.<br>
>>>>>><br>
>>>>>> I think they should be separate for now.  When we have an<br>
>>>>>> `identifier:` field in `DICompositeType` we use the<br>
>>>>>> string-based `DITypeRef`s instead of direct pointers.  I don't<br>
>>>>>> imagine there's any value in adding that indirection for these.<br>
>>>>><br>
>>>>> It would make lookups slower, but otherwise it would be harmless.<br>
>>>>><br>
>>>>>> (Although, this makes me wonder about an LTO scenario.  Imagine you<br>
>>>>>> have two translation units, and they both use a type identified by<br>
>>>>>> (e.g.) "my-type".  One of them pulled the type in from a module<br>
>>>>>> and has a `DIExternalTypeRef`, while the other translation unit<br>
>>>>>> didn't use modules and has a `DICompositeType`.  What should the<br>
>>>>>> debug info look like?  Is it okay if we end up with both?  If not,<br>
>>>>>> which one should win?<br>
>>>>><br>
>>>>> 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.<br>
>>>>><br>
>>>>>> Will these new nodes show up in the<br>
>>>>>> `retainedType:` list?)<br>
>>>>><br>
>>>>> They will not show up in the retained types list.<br>
>>>><br>
>>>> That's a show stopper for sharing with `DICompositeType`, at least<br>
>>>> until we get rid of type refs.<br>
>>>><br>
>>>> There's logic scattered around the compiler that assumes that if a<br>
>>>> node is `DICompositeType` and it has a valid identifier field, then<br>
>>>> it should be referenced by string.  If something is referenced by<br>
>>>> a string, then it needs to be in the retained type list, or you<br>
>>>> can't convert the string back to a `DICompositeType`.<br>
>>>><br>
>>>> I think it would be better to keep these classes distinct.  If it<br>
>>>> makes sense to, we can always merge them later (once we no longer<br>
>>>> have type refs).<br>
>>><br>
>>> 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.<br>
>><br>
>> Okay, sure, but that seems like a hack.  To me it looks like all the<br>
>> IR-level and backend code that cares that something is a<br>
>> DICompositeType will need special handling based on whether it's an<br>
>> external type ref.<br>
>><br>
>> 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)<br>
>><br>
>> Is there some complexity I'm missing here?<br>
><br>
> Hmm.  I just did a `git grep DICompositeType`, and there's less<br>
> in the backend than I thought.<br>
><br>
> I guess the only real problem I have is with the type refs.  It<br>
> doesn't make sense to me that we'd add these to the retained<br>
> type list.<br>
<br>
</div></div>The fwddecl-style references (with full DeclContext + Name) that David referred to actually make some sense in the retained types list.</blockquote><div><br>Not sure I follow how one (fwddecl style references) relate to the other (retained types list)?<br></div></div></div></div></div></blockquote><div><br></div></div></div><div>There is really nothing we could emit for an external type ref that is only a signature or UID.</div></div></div></blockquote><div><br></div><div>Still missing some steps in this thread. Confused.<br><br>If all we have is a signature, then we emit DW_AT_type + DW_FORM_ref_sig8</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class=""><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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) </blockquote><div><br>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?<br></div></div></div></div></div></blockquote><div><br></div></span><div>I was thinking more about DW_AT_type + DW_FORM_ref_sig8 (not sure if that was a typo)</div></div></div></blockquote><div><br>Not a typo - that's one alternative way to reference a type in a type unit if we don't emit a full declaration. (but it presents problems when we want to emit the out-of-line definition of a member function that needs to reference the declaration which we currently put inside the declaration - GCC Produces 3 different ways of referencing type units, I just implemented the most general (the one that allows member function declarations))<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div> versus a DW_AT_TYPE + ref to fwd decl that is a child of the DW_TAG_module or the skeleton CU.</div></div></div></blockquote><div><br>I'd rather not reference things in the module or skeleton CU - I'm hoping to keep this usable by a DWARF 5 debugger supporting fission and type units rather than adding more custom/experimental features.<br><br>For anything outside fission+type units, I really want to talk about that sort of thing in detail to understand the motivation, direction, benefits, tradeoffs, alternatives, etc. That's why I like piggybacking on the existing features - it requires far less scrutiny/design as the features are already formalized/implemented/used in practice, etc.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>In the first case the consumer cannot know in which module to look for the type (other than by searching through all modules imported by the CU). In the second case it could use the decl context of the fwd decl (assuming that it would be the root of the DeclContext some way or another) to figure out the module. It also could load the type directly from the module using the info from the decl context.</div></div></div></blockquote><div><br>Using the fully qualified name built from the context/name? If that suits you guys, though I think we originally discussed having a type unit hash->module info type identifier table somewhere, but that's for the other review threads I think... which I'll eventually muster the courage to get back to. Sorry for the delay.<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- adrian</div></font></span><span class=""><br><blockquote type="cite"><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">and let dsymutil optimize them away.<br>
<span><font color="#888888"><br>
-- adrian<br>
</font></span><div><div><br>
><br>
>><br>
>> It seems like it's just being shoehorned in.<br>
>><br>
>> @David, if you still feel like they should be the same class, can you<br>
>> explain why?<br>
>><br>
>>> 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.<br>
>><br>
>><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>
</div></blockquote></span></div><br></div></blockquote></div><br></div></div>