<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 20, 2016 at 4:27 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@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"><span>> +  if (ImportIndex) {<br>
> +    ThinLTOGlobalProcessing ThinLTOProcessing(SrcM, ImportIndex,<br>
> +                                              FunctionsToImport);<br>
> +    if (ThinLTOProcessing.run())<br>
> +      return true;<br>
> +    for (auto *GV : ThinLTOProcessing.getNewExportedValues())<br>
> +      ValuesToLink.insert(GV);<br>
> +  }<br>
><br>
<br>
</span>This patch is a nice improvement, but it should be possible to move<br>
this completely out of the linker, no? When we have an ImportIndex we<br>
also have a complete list of what to import.<br></blockquote><div><br></div><div>For the exporting module (the main module being compiled), we have to do the renaming/promotion and other linkage changes before IRMover, so that the new globals are correctly added to the ValuesToLink. That is straightforward, though, and I could require that this be done prior to invoking linkInModule in the client (llvm-link) via renameModuleForThinLTO.</div><div><br></div><div>However, it is trickier to split this out for imports. If it is done prior to the whole ModuleLinker::run, it causes issues because ModuleLinker::linkIfNeeded makes some decisions based on linkage that would do the wrong thing if the imported linkage had already been adjusted (for example skips available_externally). OTOH, if it is done after ModuleLinker::run, it won't work as written for several reasons. It would have to walk the dest module, which in addition to being increasingly larger as we perform more imports, has different Function pointers than the ones in the FunctionsToImport set. I could pass a different set with the equivalent Function pointers from the dest module, but the ValueMap is not available at that point to do the mapping. I suppose I could do it by name though. However, the renaming for promoted statics needs to be done based on the source module id, so it has to have access to both modules and know which to use to find the symbols and adjust them, and which to use for building the promoted name.</div><div><br></div><div>It's doable, but seemed cleaner to do it here since it works as is for both the exporting module and for imports.</div><div><br></div><div>Let me know what you think.</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">
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br><br clear="all"><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>
</div></div>