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

Tue Jan 26 18:43:24 PST 2016

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)

- 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160126/2fdea3f5/attachment.html>