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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 24 09:50:30 PST 2016


But would it be OK to say that ThinLTO only works on Darwin if you use
dsymutil?

As for ThinLTO on non-Darwin platforms using LLDB. We could still consider
that a bug in LLDB... (works on other debuggers, etc) rather than one in
ThinLTO. But, yeah, not sure how the people who own those platforms feel
about it, etc.

Not sure if the FreeBSD folks have enabled -fstandalone-debug there by
default, for example.

On Wed, Feb 24, 2016 at 9:10 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Feb 24, 2016, at 9:04 AM, Teresa Johnson <tejohnson at google.com> wrote:
>
>
>
> On Wed, Feb 24, 2016 at 8:43 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>> On Feb 23, 2016, at 11:26 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Tue, Feb 23, 2016 at 11:17 AM, Adrian Prantl <aprantl at apple.com>
>> wrote:
>>
>>>
>>> > On Feb 23, 2016, at 11:12 AM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>> >
>>> >
>>> >
>>> > On Tue, Feb 23, 2016 at 11:03 AM, Mehdi Amini <mehdi.amini at apple.com>
>>> wrote:
>>> >
>>> >> On Feb 23, 2016, at 10:53 AM, Teresa Johnson <tejohnson at google.com>
>>> wrote:
>>> >>
>>> >> Ok, after looking at the merged module Mehdi just sent me I think I
>>> know what is going on. Xalancbmk which helped me work out most of the kinks
>>> on this must not have hit this particular permutation.
>>> >
>>> > Great!
>>> >
>>> >>
>>> >> Looking at the first error from the list Mehdi sent earlier:
>>> >> unresolved type ref
>>> >> !"_ZTSN3JSC14ScopeLabelInfoE"
>>> >> !121713 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType:
>>> !"_ZTSN3JSC14ScopeLabelInfoE", size: 64, align: 64)
>>> >>
>>> >> here is what we have and what I think is happening:
>>> >>
>>> >> !121500 = !DICompositeType(tag: DW_TAG_enumeration_type, name:
>>> "FunctionParsePhase", scope: !121501, file: !121486, line: 1305, size: 32,
>>> align: 32, flags: DIFlagFwdDecl, identifier:
>>> "_ZTSN3JSC6ParserINS_5LexerIhEEE18FunctionParsePhaseE")
>>> >> !121501 = !DICompositeType(tag: DW_TAG_class_type, name:
>>> "Parser<JSC::Lexer<unsigned char> >", scope: !121464, file: !121486, line:
>>> 670, size: 18880, align: 64, elements: !121502, templateParams: !121762,
>>> identifier: "_ZTSN3JSC6ParserINS_5LexerIhEEEE")
>>> >> !121502 = !{... !121710, ...}
>>> >> !121710 = !DISubprogram(name: "getLabel", linkageName:
>>> "_ZN3JSC6ParserINS_5LexerIhEEE8getLabelEPKNS_10IdentifierE", scope:
>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEEE", file: !121486, line: 1158, type:
>>> !121711, isLocal: false, isDefinition: false, scopeLine: 1158, flags:
>>> DIFlagPrototyped, isOptimized: true)
>>> >> !121711 = !DISubroutineType(types: !121712)
>>> >> !121712 = !{!121713, !121539, !23033}
>>> >> !121713 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType:
>>> !"_ZTSN3JSC14ScopeLabelInfoE", size: 64, align: 64)
>>> >>
>>> >> The DIComposite declaration !121500 was referenced by identifier
>>> indirectly via a !tbaa attachment, presumably from a function that was
>>> imported, which is why it was correctly identified as needed, and it is in
>>> the retained types list.
>>> >>
>>> >> Note that !121501 is the scope of !121500. Presumably !121501 and its
>>> descendants (all the stuff listed below it) were mapped in when we mapped
>>> the retained type !121500.
>>> >
>>> > Is it really needed to map all the methods and types for the class
>>> when we are interested in only one method from it?
>>> >
>>> > ish.
>>> >
>>> > (+Adrian)
>>> >
>>> > We do have a representation that works for a few special cases where
>>> partial representations of types are emitted. In many cases this doesn't
>>> work on Apple platforms due to tools (LLDB, some driver debug
>>> utilities/APIs) not being able to cope with some of these situations.
>>> >
>>> > Here are some examples:
>>> >
>>> > On all platforms, implicit special members, member function templates,
>>> and nested types are attached to their type scope lazily. But on MacOS this
>>> type scope is at least a full definition. It does mean that a debugger
>>> still needs to know that those certain kinds of members may be in one type
>>> DIE but not in another type DIE, and it may be necessary to search all type
>>> DIEs for those things.
>>> >
>>> > On non-Apple platforms, we aggressively emit type declarations where
>>> we can assume that the definition will be available elsewhere (the
>>> "limited" or "non-standalone" debug info size optimizations - if a type is
>>> defined, but only used in ways that would require a declaration, we emit a
>>> declaration. If the type has a vtable, we only emit the type definition
>>> where the vtable is emitted (knowing the vtable must be emitted somewhere).
>>> If the type is the subject of an explicit instantiation declaration, we
>>> only emit the definition where we see the explicit instantiation
>>> definition).
>>> >
>>> > This latter can produce the sort of debug info you're proposing - any
>>> member function defined in a translation unit where the type was determined
>>> to only need a debug info declaration, would produce a type
>>> declaration-with-partial member declaration list.
>>> >
>>> > I don't know if we propagate the necessary flags to make this
>>> determination in the backend so we could decide whether partial types were
>>> a good idea or not.
>>> >
>>>
>>> We don’t pass this information to LLVM at the moment:
>>>
>>> CGDebugInfo.cpp:401
>>>
>>>   TheCU = DBuilder.createCompileUnit(...
>>>           DebugKind <= codegenoptions::DebugLineTablesOnly
>>>           ? llvm::DIBuilder::LineTablesOnly
>>>           : llvm::DIBuilder::FullDebug,
>>>           ...);
>>>
>>
>> Adrian - perhaps you & Mehdi can have a chat about the ramifications of a
>> choice like this for your platform (I don't know the full list of issues
>> Apple has with these partial types - it sounds like it'd tickle the LLDB
>> problems, but maybe not the driver util/debugging problem, maybe...). But
>> it might make sense to just have the same check in the backend, rather than
>> relying on the frontend flag.
>>
>>
>> I discussed this Mehdi and Greg Clayton yesterday.
>>
>> - We think that *LLDB* could be extended to support this case with
>> moderate effort if we can guarantee that a .dSYM bundle is created when
>> doing ThinLTO. Without running dsymutil, debug info stays in the .o files
>> on Darwin, which means that LLDB would have to scan every .o file for the
>> definition of an -flimit-style type, which would be prohibitively
>> expensive. In a .dSYM bundle there is a global accelerator table, which
>> makes this lookup feasible.
>> - That said, this scheme would likely still break *ctfconvert* (the
>> utility that dtrace uses to ingest debug type info) and I don’t know what
>> needs to be done to fix that.
>> - *CoreSymbolication* (atos & friends) would hopefully be unaffected
>> because it primarily looks at the line table and the inline information in
>> the debug info.
>>
>> -- adrian
>>
>
> It sounds like there is some Apple-specific work that needs to be done,
> but currently this is gated on ThinLTO and we're definitely going to want
> it there. Assuming the latest version of the patch works in Mehdi's testing
> (or when we get it fully debugged and working in that context), I'm hoping
> it sounds reasonable to get this in.
>
>
> We can't break ThinLTO on Darwin, so we would need an option to turn
> on/off this optimization. I'm fine with a cl::opt (enabled by default).
> That way we keep the ability to make it conditional to the platform in the
> future (if !Triple.isOSDarwin() for instance).
>
> Thanks,
>
> --
> Mehdi
>
>
>
> Thanks,
> Teresa
>
>
>> My thinking is that the frontend flag is for "I am compiling this object
>> with debug info but don't assume I'm compiling anything else with debug
>> info" - whereas in the backend here, we have control & know the type is
>> elsewhere, it can't be missing. So it doesn't necessarily make sense to use
>> the frontend flag to guide our decision here.
>>
>> But yeah, I think the Apple issue aside, when importing a member function
>> and importing any types in its scope chain we can reasonably import those
>> types as declarations and omit any other members, etc. Relying on the debug
>> info consumer to find the real type definition in the origin module/object.
>> This debug info should look just like debug info we already produce for
>> -fno-standalone-debug.
>>
>>
>> But I don't have quite enough context in this thread to talk about teh
>> overall approach - perhaps there's some part I can read that describes all
>> the aspects of importing debug info from a module, with specific detail as
>> it pertains to types?
>>
>> - Dave
>>
>>
>>>
>>>
>>> > - Dave
>>> >
>>> >
>>> > --
>>> > Mehdi
>>> >
>>> >
>>> >
>>> >> This ultimately brought in the derived type !121713, which references
>>> _ZTSN3JSC14ScopeLabelInfoE. However, at this time we have already decided
>>> which retained types to bring in, and presumably that is why we miss it.
>>> >>
>>> >> To fix this I'll need to restructure things a bit to do the mapping
>>> earlier, and iteratively catch any newly-required retained types. Need to
>>> think about the best way to do this...I found the corresponding
>>> declarations in
>>> https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/parser/Parser.h
>>>  and will try to create a small test case from it as well.
>>> >>
>>> >> Thanks,
>>> >> Teresa
>>> >>
>>> >> On Tue, Feb 23, 2016 at 9:22 AM, Teresa Johnson <tejohnson at google.com>
>>> wrote:
>>> >>
>>> >>
>>> >> On Tue, Feb 23, 2016 at 9:12 AM, Mehdi Amini <mehdi.amini at apple.com>
>>> wrote:
>>> >> I think in the error output I sent, I noticed the issue seemed to
>>> happen on DIDerivedType metadata that have a "baseType".
>>> >>
>>> >> That's not really different than the below example, where the types
>>> were also reached via a DIDerivedType baseType. If we mapped in the
>>> DIDerivedType we should have mapped in the reached type identifier.
>>> >>
>>> >>
>>> >> --
>>> >> Mehdi
>>> >>
>>> >>> On Feb 23, 2016, at 6:59 AM, Teresa Johnson <tejohnson at google.com>
>>> wrote:
>>> >>>
>>> >>> Both of these cases work fine. The types are in fact
>>> DICompositeType, and are reached via the DISubroutineType for the imported
>>> function. E.g. for your second example:
>>> >>>
>>> >>> struct foo { };
>>> >>> struct bar { };
>>> >>> void f(foo (*)(bar)) {
>>> >>> }
>>> >>>
>>> >>> The original module looks like:
>>> >>>
>>> >>> !4 = !DICompositeType(tag: DW_TAG_structure_type, name: "foo", file:
>>> !1, line: 1, size: 8, align: 8, flags: DIFlagFwdDecl, identifier:
>>> "_ZTS3foo")
>>> >>> !5 = !DICompositeType(tag: DW_TAG_structure_type, name: "bar", file:
>>> !1, line: 2, size: 8, align: 8, flags: DIFlagFwdDecl, identifier:
>>> "_ZTS3bar")
>>> >>> ...
>>> >>> !7 = distinct !DISubprogram(name: "f", linkageName:
>>> "_Z1fPF3foo3barE", scope: !1, file: !1, line: 3, type: !8, isLocal: false,
>>> isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized:
>>> true, variables: !13)
>>> >>> !8 = !DISubroutineType(types: !9)
>>> >>> !9 = !{null, !10}
>>> >>> !10 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !11, size:
>>> 64, align: 64)
>>> >>> !11 = !DISubroutineType(types: !12)
>>> >>> !12 = !{!"_ZTS3foo", !"_ZTS3bar"}
>>> >>>
>>> >>> Because they are reached via the imported function's DISubprogram,
>>> they get imported properly.
>>> >>>
>>> >>> On Mon, Feb 22, 2016 at 11:21 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>> >>> Yeah - I figured you would've caught it already if it were this
>>> simple. What sort of testing have you done?
>>> >>>
>>> >>> Happy to go through a few things with you in person tomorrow as
>>> well. Perhaps I'm just not understanding the algorithm you're implementing
>>> here.
>>> >>>
>>> >>> On Mon, Feb 22, 2016 at 9:09 PM, Teresa Johnson <
>>> tejohnson at google.com> wrote:
>>> >>>
>>> >>>
>>> >>> On Mon, Feb 22, 2016 at 8:03 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>> >>> Have either of you tried creating a simple test case? Naively it
>>> looks like any use of a pointer-to-user-defined type would hit this in some
>>> way, no?
>>> >>>
>>> >>> import this function:
>>> >>>
>>> >>> struct foo { };
>>> >>> void bar(foo *f) {
>>> >>> }
>>> >>>
>>> >>> and I think the code will look at the type of 'f',
>>> getCompositeTypeToImport will immediately return null, because 'f' isn't a
>>> DICompositeType, and the type won't be retained.
>>> >>>
>>> >>> Note that the type could be worse, it could involve importing more
>>> than one type:
>>> >>>
>>> >>> struct foo { };
>>> >>> struct bar { };
>>> >>> void f(foo (*)(bar)) {
>>> >>> }
>>> >>>
>>> >>>
>>> >>> Hadn't tried that because I wasn't sure what to look for to be
>>> honest (especially since my testing is all fine at this point). Will give
>>> that a try to see how it behaves.
>>> >>>
>>> >>> On Mon, Feb 22, 2016 at 7:39 PM, Teresa Johnson via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>> >>> Unfortunately without seeing how the types were referenced in the
>>> original module I may not be able to deduce why they weren't pulled in. But
>>> go ahead and send me the IR after importing in the meantime and I will see
>>> what I can figure out.
>>> >>>
>>> >>>
>>> >>> On Mon, Feb 22, 2016 at 6:02 PM, Mehdi Amini <mehdi.amini at apple.com>
>>> wrote:
>>> >>> Unfortunately IIRC it involved 800 files, and I don't have them. I
>>> need to reproduce and it'll take some time. I can send you the IR *after*
>>> importing (the broken module) if it can help (not sure).
>>> >>>
>>> >>> --
>>> >>> Mehdi
>>> >>>
>>> >>>
>>> >>>
>>> >>>> On Feb 22, 2016, at 5:52 PM, Teresa Johnson <tejohnson at google.com>
>>> wrote:
>>> >>>>
>>> >>>> Can you give me a test case to reproduce, or at least the IR for
>>> the module we're importing from (where these presumably came from) and
>>> which function(s) were imported?
>>> >>>>
>>> >>>> Thanks,
>>> >>>> Teresa
>>> >>>>
>>> >>>> On Mon, Feb 22, 2016 at 5:37 PM, Mehdi Amini <mehdi.amini at apple.com>
>>> wrote:
>>> >>>> We still have an issue with this patch, when compiling this with
>>> thinlto and debug info:
>>> https://github.com/adobe/webkit/blob/master/Source/WebCore/inspector/InspectorRuntimeAgent.cpp
>>> >>>>
>>> >>>> I haven't had time to narrow it unfortunately, it seems that
>>> "baseType" for some DIDerivedType entries are not present.
>>> >>>> What we see is a broken LLVM Module straight after the
>>> FunctionImporter. The output looks like this:
>>> >>>>
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC14ScopeLabelInfoE"
>>> >>>> !121713 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType:
>>> !"_ZTSN3JSC14ScopeLabelInfoE", size: 64, align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC15DeclarationTypeE"
>>> >>>> !121577 = !DISubroutineType(types: !121578)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC17AssignmentContextE"
>>> >>>> !121580 = !DISubroutineType(types: !121581)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC17DestructuringKindE"
>>> >>>> !121577 = !DISubroutineType(types: !121578)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC21DeclarationImportTypeE"
>>> >>>> !121606 = !DISubroutineType(types: !121607)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC23SourceProviderCacheItemE"
>>> >>>> !121621 = !DIDerivedType(tag: DW_TAG_const_type, baseType:
>>> !"_ZTSN3JSC23SourceProviderCacheItemE")
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE10LexerStateE"
>>> >>>> !121743 = !DISubroutineType(types: !121744)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE15AutoPopScopeRefE"
>>> >>>> !121600 = !DIDerivedType(tag: DW_TAG_reference_type, baseType:
>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE15AutoPopScopeRefE", size: 64, align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE20ExpressionErrorClassE"
>>> >>>> !121571 = !DISubroutineType(types: !121572)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE23AutoCleanupLexicalScopeE"
>>> >>>> !121604 = !DIDerivedType(tag: DW_TAG_reference_type, baseType:
>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE23AutoCleanupLexicalScopeE", size: 64,
>>> align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE25ExpressionErrorClassifierE"
>>> >>>> !121535 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType:
>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE25ExpressionErrorClassifierE", size: 64,
>>> align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerIhEEE9SavePointE"
>>> >>>> !121751 = !DISubroutineType(types: !121752)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE10LexerStateE"
>>> >>>> !122000 = !DISubroutineType(types: !122001)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE15AutoPopScopeRefE"
>>> >>>> !121866 = !DIDerivedType(tag: DW_TAG_reference_type, baseType:
>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE15AutoPopScopeRefE", size: 64, align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE20ExpressionErrorClassE"
>>> >>>> !121838 = !DISubroutineType(types: !121839)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE23AutoCleanupLexicalScopeE"
>>> >>>> !121870 = !DIDerivedType(tag: DW_TAG_reference_type, baseType:
>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE23AutoCleanupLexicalScopeE", size: 64,
>>> align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE25ExpressionErrorClassifierE"
>>> >>>> !121803 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType:
>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE25ExpressionErrorClassifierE", size: 64,
>>> align: 64)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC6ParserINS_5LexerItEEE9SavePointE"
>>> >>>> !122008 = !DISubroutineType(types: !122009)
>>> >>>> unresolved type ref
>>> >>>> !"_ZTSN3JSC9ScopeNodeE"
>>> >>>> !121635 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType:
>>> !"_ZTSN3JSC9ScopeNodeE", size: 64, align: 64)
>>> >>>>
>>> >>>>
>>> >>>> --
>>> >>>> Mehdi
>>> >>>>
>>> >>>>
>>> >>>> > On Feb 22, 2016, at 2:20 PM, Teresa Johnson <tejohnson at google.com>
>>> wrote:
>>> >>>> >
>>> >>>> > tejohnson updated this revision to Diff 48732.
>>> >>>> > tejohnson added a comment.
>>> >>>> >
>>> >>>> > Handle a null MD passed to MapMetadata to address problem
>>> reported by
>>> >>>> > ahatanak.
>>> >>>> >
>>> >>>> >
>>> >>>> > http://reviews.llvm.org/D16440
>>> >>>> >
>>> >>>> > Files:
>>> >>>> >  include/llvm/Linker/IRMover.h
>>> >>>> >  lib/Linker/IRMover.cpp
>>> >>>> >  lib/Linker/LinkModules.cpp
>>> >>>> >  lib/Transforms/Utils/ValueMapper.cpp
>>> >>>> >  test/Linker/thinlto_funcimport_debug.ll
>>> >>>> >  test/Transforms/FunctionImport/Inputs/funcimport_debug.ll
>>> >>>> >  test/Transforms/FunctionImport/funcimport_debug.ll
>>> >>>> >
>>> >>>> > <D16440.48732.patch>
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> --
>>> >>>> Teresa Johnson |    Software Engineer |     tejohnson at google.com |
>>>  408-460-2413
>>> >>>
>>> >>>
>>> >>>
>>> >>>
>>> >>> --
>>> >>> Teresa Johnson |     Software Engineer |     tejohnson at google.com |
>>>  408-460-2413
>>> >>>
>>> >>> _______________________________________________
>>> >>> llvm-commits mailing list
>>> >>> llvm-commits at lists.llvm.org
>>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> >>>
>>> >>>
>>> >>>
>>> >>>
>>> >>>
>>> >>> --
>>> >>> 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
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> 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/20160224/b89f8882/attachment.html>


More information about the llvm-commits mailing list