<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 25, 2016 at 9:06 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><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 dir="ltr">Sounds like one of the two ways for doing importing is going away - should I wait for this patch to be updated once that removal is complete?</div></blockquote><div><br></div><div>The patch won't be affected by the removal of post-pass metadata linking, other than removing the new comment I added above the computation of NewSubprograms:</div><div><br></div><div>    // Map in any needed subprograms before walking the retained types list</div><div>    // below. This ensures that in the case of postpass metadata linking we</div><div>    // have already mapped in any composite types reached via direct reference.</div><div><br></div><div>So fine to review as is if you ignore that comment (the subprograms are always mapped in as the function bodies are linked).</div><div><br></div><div>Thanks,</div><div>Teresa</div><div><br></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 class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Wed, Mar 23, 2016 at 6:39 AM, Teresa Johnson via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br></div></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><div class="h5"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Mar 22, 2016 at 4:36 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><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">Hey Teresa,<br>
<br>
How will the changes we were discussing with Adrian yesterday on IRC will affect this patch?<br></blockquote><div><br></div></span><div>Yes, not from the moving of metadata into the function block where possible, but with the reversing of the DICompileUnit -> DISubprogram links. </div><div><br></div><div>With the link reversal, the DICompileUnit will automatically be fully mapped in when the function bodies are mapped and the associated metadata including the DISubprogram are mapped in, thus pulling in all the retained types and other fields on the CU we are trying to drop here for imported modules. So we'd either need to figure out how to stop the metadata mapping that happens in MapMetadata when we hit a DICompileUnit, so that we could map it manually as we do here, or figure out what to map via some callbacks from the ValueMapper (similar to how we prevent unneeded DISubprogram from getting mapped into the DISubprogram list hanging of the DICompileUnit currently).</div><div><br></div><div>If the retained types list is also dropped from the DICompileUnit, which I see is also on Duncan's list, then at least the above mapping would not automatically pull in all types. But if we don't do that before reversing the CU->SP link then it needs to be handled.</div><div><br></div><div>So what we want to do with this patch will depend on how fast some of these changes under discussion will go in. It results in a greatly reduced amount of type data being pulled in with -flto=thin -g, so I'm not sure we want to wait until all the debug info changes under discussion are in.</div><div><br></div><div>One way to handle this with the current patch if the link reversal goes in before the retained types etc are removed from the CU is to put back the findReachedCompositeTypes helper that I just removed with my latest update and use it to find the needed types and manually create/map the new pruned DICompileUnit *before* we do any function body linking. Then when the DISubprogram are mapped during function body linking the mapper will find the new DICompileUnit in the ValueMap and use it. I *think* I can do this now with some restructuring of this patch, or we could do that just after the link reversal goes in (we would temporarily lose out on the benefits of this patch in between when the link reversal and the change to this handling goes in).</div><span><font color="#888888"><div><br></div><div>Teresa</div></font></span><span><div><br></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>
--<br>
Mehdi<br>
<div><div><br>
> On Mar 22, 2016, at 2:35 PM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> wrote:<br>
><br>
> tejohnson updated this revision to Diff 51343.<br>
> tejohnson added a comment.<br>
><br>
> - Address review comments from dblaikie.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D16440" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16440</a><br>
><br>
> Files:<br>
>  include/llvm/Linker/IRMover.h<br>
>  lib/Linker/IRMover.cpp<br>
>  lib/Linker/LinkModules.cpp<br>
>  lib/Transforms/Utils/ValueMapper.cpp<br>
>  test/Linker/thinlto_funcimport_debug.ll<br>
>  test/Transforms/FunctionImport/Inputs/funcimport_debug.ll<br>
>  test/Transforms/FunctionImport/funcimport_debug.ll<br>
><br>
</div></div>> <D16440.51343.patch><br>
<br>
</blockquote></span></div><br><br clear="all"><span><div><br></div>-- <br><div><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> <a href="tel:408-460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</span></div></div>
<br></div></div><span class="">_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></span></blockquote></div><br></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>