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