Debug info questy (was Re: [llvm] r256003 - [ThinLTO/LTO] Don't link in unneeded metadata)

Wed Jan 6 10:37:38 PST 2016

Thanks Duncan and Eric.

I should also be able to implement #1 very quickly and get it in today
if we want a workaround while we figure out what is the right approach
here. Will take a stab at that now.

(Somehow I feel the need to apologize for the overly cute subject - my
intended "question" got turned into "questy", presumably by fat
fingers. =(  =)  )

Teresa

On Wed, Jan 6, 2016 at 10:27 AM, Eric Christopher <echristo at gmail.com> wrote:
>
>
> On Wed, Jan 6, 2016 at 10:04 AM Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>>
>> +aprantl
>>
>> > On 2016-Jan-06, at 09:46, Teresa Johnson <tejohnson at google.com> wrote:
>> >
>> > +dexonsmith,echristo,blaikie who are way more familiar with debug info
>> > than I
>> > +pcc who also found this and contacted me off-list with a possible
>> > solution
>> >
>> > See also https://llvm.org/bugs/show_bug.cgi?id=26037 that I updated
>> > with a test case and more detailed analysis.
>> >
>> > The issue is essentially that we no longer pull in a DISubprogram on
>> > an LTO link with this patch because the associated function was not
>> > linked in. However, it had been referenced by a DIImportedEntity which
>> > is leaving behind a DIImportedEntity with no entity:
>> >
>> > !11 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !8,
>> > line: 2)
>> >
>> > which in turn causes an assert during debug generation. The 3
>> > solutions I mention there are:
>> >
>> > 1) Change findNeededSubprograms to look at entities and conservatively
>> > remove any subprograms reached from there from the UnneededSubprograms
>> > list
>> > 2) Remove any DIImportedEntity that no longer have an entity
>> > 3) Change the debug generation code to handle DIImportedEntity with a
>> > null entity
>> >
>> > pcc has a tentative solution that implements 3) above.
>> >
>> > Debug info experts - what is the right thing to do here? Is it legal
>> > to have a DIImportedEntity with no entity? If so, pcc's solution
>> > (solution 3 above) is the right one. Otherwise I should implement 1)
>> > or 2).
>>
>> My intuition says that (2) is the correct solution, but Adrian would know
>> best.
>>
>
> It's a bit iffy:
>
> 3.2.3 Imported (or Renamed) Declaration Entries Some languages support the
> concept of importing into or making accessible in a given unit declarations
> made in a different module or scope. An imported declaration may sometimes
> be given another name. An imported declaration is represented by one or more
> debugging information entries with the tag DW_TAG_imported_declaration. When
> an overloaded entity is imported, there is one imported declaration entry
> for each overloading.
>
> which means that we're probably looking at 1 as we care about declarations
> for entities rather than defining declarations. That said, unless we change
> the metadata to be a declaration of a subprogram rather than a defining
> declaration we're going to have a bad time further down the line.
>
>>
>> In the meantime, if (3) isn't too intrusive, it would be worth doing
>> temporarily to unbreak the bot...
>
>
> This is absolutely ok as long as it's temporary while we figure this out.
>
> -eric
>
>>
>>
>> >
>> > Thanks,
>> > Teresa
>> >
>> >
>> > On Tue, Jan 5, 2016 at 4:24 PM, Ahmed Bougacha
>> > <ahmed.bougacha at gmail.com> wrote:
>> >> On Fri, Dec 18, 2015 at 9:51 AM, Teresa Johnson via llvm-commits
>> >> <llvm-commits at lists.llvm.org> wrote:
>> >>> Author: tejohnson
>> >>> Date: Fri Dec 18 11:51:37 2015
>> >>> New Revision: 256003
>> >>>
>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=256003&view=rev
>> >>> Log:
>> >>> [ThinLTO/LTO] Don't link in unneeded metadata
>> >>>
>> >>> Summary:
>> >>> Third patch split out from http://reviews.llvm.org/D14752.
>> >>>
>> >>> Only map in needed DISubroutine metadata (imported or otherwise linked
>> >>> in functions and other DISubroutine referenced by inlined
>> >>> instructions).
>> >>> This is supported for ThinLTO, LTO and llvm-link --only-needed, with
>> >>> associated tests for each one.
>> >>>
>> >>> Depends on D14838.
>> >>>
>> >>> Reviewers: dexonsmith, joker.eph
>> >>>
>> >>> Subscribers: davidxl, llvm-commits, joker.eph
>> >>>
>> >>> Differential Revision: http://reviews.llvm.org/D14843
>> >>>
>> >>> Added:
>> >>>    llvm/trunk/test/Linker/Inputs/only-needed-debug-metadata.ll
>> >>>    llvm/trunk/test/Linker/only-needed-debug-metadata.ll
>> >>> Modified:
>> >>>    llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h
>> >>>    llvm/trunk/lib/Linker/IRMover.cpp
>> >>>    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
>> >>>    llvm/trunk/test/Linker/thinlto_funcimport_debug.ll
>> >>>    llvm/trunk/test/tools/gold/X86/Inputs/linkonce-weak.ll
>> >>>    llvm/trunk/test/tools/gold/X86/linkonce-weak.ll
>> >>>
>> >>> Modified: llvm/trunk/lib/Linker/IRMover.cpp
>> >>> URL:
>> >>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=256003&r1=256002&r2=256003&view=diff
>> >>>
>> >>> ==============================================================================
>> >>> --- llvm/trunk/lib/Linker/IRMover.cpp (original)
>> >>> +++ llvm/trunk/lib/Linker/IRMover.cpp Fri Dec 18 11:51:37 2015
>> >>> @@ -487,6 +495,16 @@ class IRLinker {
>> >>>
>> >>>   void linkNamedMDNodes();
>> >>>
>> >>> +  /// Populate the UnneededSubprograms set with the DISubprogram
>> >>> metadata
>> >>> +  /// from the source module that we don't need to link into the dest
>> >>> module,
>> >>> +  /// because the functions were not imported directly or via an
>> >>> inlined body
>> >>> +  /// in an imported function.
>> >>> +  void findNeededSubprograms(ValueToValueMapTy &ValueMap);
>> >>
>> >> Hey Teresa,
>> >>
>> >> This has been causing bootstrap assertion failures, I think because a
>> >> DISubprogram referenced by DIImportedEntity should also be considered
>> >> needed.
>> >>
>> >> I filed PR26037, could you please have a look?
>> >>
>> >> Thanks!
>> >> -Ahmed
>> >
>> >
>> >
>> > --
>> > Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
>>
>

-- 
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413