[PATCH] D16440: [ThinLTO] Link in only necessary DICompileUnit operands
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 23 12:59:11 PST 2016
On Tue, 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.
>
> 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.
>
Ok this is a good point you and Mehdi have raised about only needing the
declaration (Apple specific issues aside) - in fact currently with this
patch, when importing the scope chain linked via identifiers I am already
only creating a type declaration for the visited composite/retained types.
It should be straightforward to do the same for scope chains linked via
references like this case. I think I'll still have to do some work so that
the actual mapping of retained types doesn't pull in the definition.
>
>
> 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?
>
I don't have a writeup. Basically it will map in anything reached via the
imported function and its instructions. And before this patch anything
reached from named metadata like the dbg.cu.
Teresa
>
> - 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/20160223/af86506e/attachment-0001.html>
More information about the llvm-commits
mailing list