<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 14, 2016, at 4:06 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Thu, Jan 14, 2016 at 4:00 PM, Adrian Prantl<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><br class=""><div class=""><div class=""><div class="h5"><blockquote type="cite" class=""><div class="">On Jan 8, 2016, at 10:18 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><br class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;">On Wed, Jan 6, 2016 at 10:38 AM, Adrian Prantl<span class=""> </span><span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span><span class=""> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><br class=""><div class=""><div class=""><div class=""><blockquote type="cite" class=""><div class="">On Jan 6, 2016, at 10:27 AM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank" class="">echristo@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Jan 6, 2016 at 10:04 AM Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank" class="">dexonsmith@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">+aprantl<br class=""><br class="">> On 2016-Jan-06, at 09:46, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank" class="">tejohnson@google.com</a>> wrote:<br class="">><br class="">> +dexonsmith,echristo,blaikie who are way more familiar with debug info than I<br class="">> +pcc who also found this and contacted me off-list with a possible solution<br class="">><br class="">> See also<span class=""> </span><a href="https://llvm.org/bugs/show_bug.cgi?id=26037" rel="noreferrer" target="_blank" class="">https://llvm.org/bugs/show_bug.cgi?id=26037</a><span class=""> </span>that I updated<br class="">> with a test case and more detailed analysis.<br class="">><br class="">> The issue is essentially that we no longer pull in a DISubprogram on<br class="">> an LTO link with this patch because the associated function was not<br class="">> linked in. However, it had been referenced by a DIImportedEntity which<br class="">> is leaving behind a DIImportedEntity with no entity:<br class="">><br class="">> !11 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !8, line: 2)<br class="">><br class="">> which in turn causes an assert during debug generation. The 3<br class="">> solutions I mention there are:<br class="">><br class="">> 1) Change findNeededSubprograms to look at entities and conservatively<br class="">> remove any subprograms reached from there from the UnneededSubprograms<br class="">> list<br class="">> 2) Remove any DIImportedEntity that no longer have an entity<br class="">> 3) Change the debug generation code to handle DIImportedEntity with a<br class="">> null entity<br class="">><br class="">> pcc has a tentative solution that implements 3) above.<br class="">><br class="">> Debug info experts - what is the right thing to do here? Is it legal<br class="">> to have a DIImportedEntity with no entity? If so, pcc's solution<br class="">> (solution 3 above) is the right one. Otherwise I should implement 1)<br class="">> or 2).<br class=""><br class="">My intuition says that (2) is the correct solution, but Adrian would know best.<br class=""><br class=""></blockquote><div class=""><br class=""></div><div class="">It's a bit iffy:</div><div class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class="">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></div></blockquote><div class=""><br class=""></div></div></div>Right. Dropping the imported entity because it’s not in the current translation unit would change the overload resolution in the debugger. </div></div></blockquote><div class=""><br class="">I'm not sure I follow here - if the function is never emitted (which it isn't in this translation unit) where would overload resolution come into it?<br class=""><br class=""></div></div></div></blockquote><div class=""><br class=""></div></div></div><div class=""><div class=""><div class="">The silly example I had in mind was something like:</div><div class=""><br class=""></div><div class="">namespace N {</div><div class=""> <span class="Apple-converted-space"> </span>void foo();</div><div class="">}</div><div class=""><br class=""></div><div class="">void foo() {</div><div class=""> <span class="Apple-converted-space"> </span>using N::foo;</div><div class=""> <span class="Apple-converted-space"> </span>// stop here and evaluate "foo()"</div></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Right - but in this module, there is no definition of 'foo', so there's no way to stop at it.<br class=""><br class="">In whatever other module has a definition of 'foo', we would expect the using declaration's debug info will be present there, no?<br class=""><br class="">If I'm understanding the situation correctly... </div></div></div></blockquote><div><br class=""></div><div>It almost looks like we’re thinking of different situations. My understanding of the problem was that there is an imported entity referencing a subprogram that was optimized away, and the question was whether it was safe to remove the imported entity if the subprogram doesn’t exist any more. The point I was trying to make was that it is never safe to remove an imported entity because it changes the name lookup at the site of the import (::foo in my example).</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><div class=""><div class=""><div class=""><div class=""> <span class="Apple-converted-space"> </span>if (false)</div><div class=""> <span class="Apple-converted-space"> </span>foo(); // optimized out</div><div class="">}</div><span class="HOEnZb"><font color="#888888" class=""><div class=""><br class=""></div><div class="">-- adrian</div></font></span></div></div><div class=""><div class="h5"><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><div class="">OK, /maybe/ if the subprogram had a local type that leaked out somewhere (eg: through a template instantiation that lead to a global variable that stayed alive despite the outer function being removed) and you were inside a member function of that local type you could rely on the imported declaration to provide name lookup. But in that case you'd still need to preserve the subprogram for the nested type to be the child of.<br class=""><br class="">Adrian: What name resolution situation did you have in mind?<br class=""></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><div class=""><br class="">Teresa: Have you looked at/hit this issue with local types? I might be able to conjure a specific example (using the aforementioned template scenario) that could tickle it in that direction if that'd be helpful.<br class=""><br class="">- Dave<br class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;" class=""><div class=""><span class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">In the meantime, if (3) isn't too intrusive, it would be worth doing temporarily to unbreak the bot...<br class=""></blockquote><div class=""><br class=""></div><div class="">This is absolutely ok as long as it's temporary while we figure this out.</div></div></div></div></blockquote><div class=""><br class=""></div></span>Agreed.<div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class="">-eric</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">><br class="">> Thanks,<br class="">> Teresa<br class="">><br class="">><br class="">> On Tue, Jan 5, 2016 at 4:24 PM, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com" target="_blank" class="">ahmed.bougacha@gmail.com</a>> wrote:<br class="">>> On Fri, Dec 18, 2015 at 9:51 AM, Teresa Johnson via llvm-commits<br class="">>> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class="">>>> Author: tejohnson<br class="">>>> Date: Fri Dec 18 11:51:37 2015<br class="">>>> New Revision: 256003<br class="">>>><br class="">>>> URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project?rev=256003&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=256003&view=rev</a><br class="">>>> Log:<br class="">>>> [ThinLTO/LTO] Don't link in unneeded metadata<br class="">>>><br class="">>>> Summary:<br class="">>>> Third patch split out from<span class=""> </span><a href="http://reviews.llvm.org/D14752" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D14752</a>.<br class="">>>><br class="">>>> Only map in needed DISubroutine metadata (imported or otherwise linked<br class="">>>> in functions and other DISubroutine referenced by inlined instructions).<br class="">>>> This is supported for ThinLTO, LTO and llvm-link --only-needed, with<br class="">>>> associated tests for each one.<br class="">>>><br class="">>>> Depends on D14838.<br class="">>>><br class="">>>> Reviewers: dexonsmith, joker.eph<br class="">>>><br class="">>>> Subscribers: davidxl, llvm-commits, joker.eph<br class="">>>><br class="">>>> Differential Revision:<span class=""> </span><a href="http://reviews.llvm.org/D14843" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D14843</a><br class="">>>><br class="">>>> Added:<br class="">>>> llvm/trunk/test/Linker/Inputs/only-needed-debug-metadata.ll<br class="">>>> llvm/trunk/test/Linker/only-needed-debug-metadata.ll<br class="">>>> Modified:<br class="">>>> llvm/trunk/include/llvm/Transforms/Utils/ValueMapper.h<br class="">>>> llvm/trunk/lib/Linker/IRMover.cpp<br class="">>>> llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp<br class="">>>> llvm/trunk/test/Linker/thinlto_funcimport_debug.ll<br class="">>>> llvm/trunk/test/tools/gold/X86/Inputs/linkonce-weak.ll<br class="">>>> llvm/trunk/test/tools/gold/X86/linkonce-weak.ll<br class="">>>><br class="">>>> Modified: llvm/trunk/lib/Linker/IRMover.cpp<br class="">>>> URL:<span class=""> </span><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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=256003&r1=256002&r2=256003&view=diff</a><br class="">>>> ==============================================================================<br class="">>>> --- llvm/trunk/lib/Linker/IRMover.cpp (original)<br class="">>>> +++ llvm/trunk/lib/Linker/IRMover.cpp Fri Dec 18 11:51:37 2015<br class="">>>> @@ -487,6 +495,16 @@ class IRLinker {<br class="">>>><br class="">>>> void linkNamedMDNodes();<br class="">>>><br class="">>>> + /// Populate the UnneededSubprograms set with the DISubprogram metadata<br class="">>>> + /// from the source module that we don't need to link into the dest module,<br class="">>>> + /// because the functions were not imported directly or via an inlined body<br class="">>>> + /// in an imported function.<br class="">>>> + void findNeededSubprograms(ValueToValueMapTy &ValueMap);<br class="">>><br class="">>> Hey Teresa,<br class="">>><br class="">>> This has been causing bootstrap assertion failures, I think because a<br class="">>> DISubprogram referenced by DIImportedEntity should also be considered<br class="">>> needed.<br class="">>><br class="">>> I filed PR26037, could you please have a look?<br class="">>><br class="">>> Thanks!<br class="">>> -Ahmed<br class="">><br class="">><br class="">><br class="">> --<br class="">> Teresa Johnson | Software Engineer |<span class=""> </span><a href="mailto:tejohnson@google.com" target="_blank" class="">tejohnson@google.com</a><span class=""> </span>|<span class=""> </span><a href="tel:408-460-2413" value="+14084602413" target="_blank" class="">408-460-2413</a></blockquote></div></div></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div><br class=""></body></html>