[PATCH] D16440: [ThinLTO] Link in only necessary DICompileUnit operands

Tue Jan 26 22:07:29 PST 2016

On Tue, Jan 26, 2016 at 6:43 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Jan 26, 2016 at 4:05 PM, Teresa Johnson <tejohnson at google.com>
> wrote:
>
>>
>>
>> On Tue, Jan 26, 2016 at 9:47 AM, Teresa Johnson <tejohnson at google.com>
>> wrote:
>>
>>>
>>>
>>> On Tue, Jan 26, 2016 at 9:09 AM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jan 26, 2016 at 8:53 AM, Teresa Johnson via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> tejohnson added a comment.
>>>>>
>>>>> > !78 = !DICompositeType(tag: DW_TAG_class_type, name:
>>>>> "XMLPlatformUtils", scope: !17, file: !16, line: 104, size: 8, align: 8,
>>>>> elements: !79, identifier: "_ZTSN11xercesc_2_516XMLPlatformUtilsE")
>>>>>
>>>>> >  ...
>>>>>
>>>>> >  !93 = !DISubprogram(name: "Initialize", linkageName:
>>>>> "_ZN11xercesc_2_516XMLPlatformUtils10InitializeEPKcS2_PNS_12PanicHandlerEPNS_13MemoryManagerE",
>>>>> scope: !"_ZTSN11xercesc_2_516XMLPlatformUtilsE", file: !16, line: 204,
>>>>> type: !94, isLocal: false, isDefinition: false, scopeLine: 204, flags:
>>>>> DIFlagPublic | DIFlagPrototyped, isOptimized: true)
>>>>>
>>>>>
>>>>> Correction, the DISubprogram that references the composite type (and
>>>>> was pulled in due to an import) is:
>>>>> !103 = distinct !DISubprogram(name:
>>>>> "alignPointerForNewBlockAllocation", linkageName:
>>>>> "_ZN11xercesc_2_516XMLPlatformUtils33alignPointerForNewBlockAllocationEm",
>>>>> scope: !"_ZTSN11xercesc_2_516XMLPlatformUtilsE", file: !104, line: 851,
>>>>> type: !105, isLocal: false, isDefinition: true, scopeLine: 852, flags:
>>>>> DIFlagPrototyped, isOptimized: true, declaration: !107, variables: !108)
>>>>>
>>>>> > Why aren't these correlated by the metadata ID instead? I.e. !93
>>>>> instead of !"_ZTSN11xercesc_2_516XMLPlatformUtilsE" in the DISubprogram
>>>>> scope? Then no TypeIdentifierMap would be needed.
>>>>>
>>>>> >
>>>>>
>>>>> > In any case, I added some handling for this, so that we detect and
>>>>> map in retained types onto the new DICompileUnit if they correspond to a
>>>>> DISubprogram that is being mapped in. However, I just hit another similar
>>>>> issue. In this case, the retained type DICompositeType identifier is the
>>>>> base type of a DIDerivedType needed by a DISubprogram that was mapped in.
>>>>> E.g.:
>>>>>
>>>>> >
>>>>>
>>>>> > !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name:
>>>>> "PanicReasons", scope: !"_ZTSN11xercesc_2_512PanicHandlerE", file: !4,
>>>>> line: 116, size: 32, align: 32, elements: !5, identifier:
>>>>> "_ZTSN11xercesc_2_512PanicHandler12PanicReasonsE")
>>>>>
>>>>> >  ...
>>>>>
>>>>> >  !107 = !DIDerivedType(tag: DW_TAG_const_type, baseType:
>>>>> !"_ZTSN11xercesc_2_512PanicHandler12PanicReasonsE")
>>>>>
>>>>> >
>>>>>
>>>>> > where the DIDerivedType is the DISubroutineType for a DISubprogram
>>>>> that was mapped in. I'll need to extend my handling a bit to get this case.
>>>>> Same question here though, why doesn't it just use "baseType: !3"?
>>>>>
>>>>>
>>>>> It turns out the second problem was caused by the fix for the first.
>>>>> The subprogram that referenced the DIDerivedType !107 was only mapped in
>>>>> because it is in the list of elements for the DICompositeType !78 that was
>>>>> the scope for the function needed due to importing. So the set of what
>>>>> debug metadata needs to be imported keeps exploding as the problem is
>>>>> addressed.
>>>>>
>>>>> Is there any way to avoid this? Does the complete DICompositeType !78
>>>>> need to be recreated in the importing module? It isn't clear to me which
>>>>> debug type metadata needs to be fully recreated in the importing module, I
>>>>> know the original premise behind this patch was that it was sufficient for
>>>>> it to exist in the original module. But getOrCreateSubprogramDIE wants to
>>>>> get the ContextDIE for the subprogram.
>>>>>
>>>>
>>>> Subprogram DIEs will need the right context for the DWARF to be
>>>> correct/valid, so some amount of DWARF is going to need to be cloned into
>>>> any module where the subprogram/function is emitted (even if it is only
>>>> emitted inline)
>>>>
>>>> Essentially, the minimal DWARF you would need for a subprogram that
>>>> might be a member of a type would be:
>>>>
>>>> Declaration of outer type
>>>>   Declaration of member function
>>>> Definition of member function (referencing the declaration)
>>>>
>>>> So you still need to clone the metadata declaration to match the
>>>> definition, and the scope chains of both declaration and definition (but I
>>>> don't think we put the definition inside any scopes if it's an implicit
>>>> definition of a member (that's vague wording, I can be more specific)), but
>>>> you can avoid importing lots of stuff if you demote any definitions of user
>>>> defined types (structs, classes, enums, etc) to declarations.
>>>>
>>>
>>> Thanks, this is helpful.
>>>
>>> Just talked to David Blaikie offline, he pointed me to the composite
>>> type declarations being marked with "flags: DIFlagFwdDecl" instead of
>>> having an element list. He pointed out that I need to follow the scope
>>> chains of the imported Subprogram definition and declaration all the way up
>>> but can make them declarations instead of importing all the member
>>> elements. And ditto for the types hanging off the imported subprogram.
>>>
>>
>> What about the vtableHolder? With my new changes I getting the following
>> verification error:
>>
>> unresolved type ref
>> !"_ZTSN11xercesc_2_57LocatorE"
>> !4554 = !DICompositeType(tag: DW_TAG_class_type, name: "ReaderMgr",
>> scope: !985, file: !2308, line: 177, size: 640, align: 64, flags:
>> DIFlagFwdDecl, vtableHolder: !"_ZTSN11xercesc_2_57LocatorE", identifier:
>> "_ZTSN11xercesc_2_59ReaderMgrE")
>>
>> I am following the scope chains and bringing in referenced type decls, do
>> I need to do the same for the vtableHolder if it exists? Any others?
>>
>
> No, I shouldn't think so - a declaration should be much simpler than that.
>
> I'd suggest finding a type with a vtable holder - just make a small
> example:
>
> struct foo {
>   virtual ~foo() {}
> } f;
>
> & look at the type description in metadata - I assume it'll have a
> vtableHolder.
>
> Now try a declaration instead:
>
> struct foo {
>   virtual ~foo() {}
> } *f;
>
> Should produce just the declaration of 'foo', and probably won't have a
> vtableHolder. You should probably have/not have whatever the declaration of
> 'foo' has. Which should be nothing but name and the decl flag, really (&
> scope). It probably doesn't need anything else. (I guess some parts of the
> debug info emission might need/expect file/line, but it shouldn't really be
> needed)
>

Ok, thanks. Confirmed via similar example comparison that template params
also should be dropped when demoting to declaration. The patch seems to be
working now for xalancbmk, but needs some cleanup before I send it out.

Teresa

> - Dave
>
>
>>
>> Thanks,
>> Teresa
>>
>>
>>
>>>
>>> Will work on that.
>>>
>>> Teresa
>>>
>>>
>>>> So long as the types stick around as definitions somewhere.
>>>>
>>>> The DWARF produced here will look similar to other things we produce
>>>> with -fno-standalone-debug (this is the default on non-Apple platforms) -
>>>> types in DWARF that are only a declaration and have arbitrary subsets of
>>>> members, but there will be an authoritative definition somewhere in the
>>>> LTO'd program (wherever the types were drawn from). This shouldn't hit the
>>>> problematic cases Apple has on their platform that necessitated them
>>>> turning to -fstandalone-debug. It probably won't even hit the LLDB bugs
>>>> that are exposed by -fno-standalone-debug either (because the debug info
>>>> will be standalone in the destination module before we import anything, so
>>>> we won't create the weird situations that LLDB fails on)
>>>>
>>>> - David
>>>>
>>>
>>>
>>>
>>> --
>>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>>> 408-460-2413
>>>
>>
>>
>>
>> --
>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>> 408-460-2413
>>
>
>

-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160126/92ef82e8/attachment.html>