<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 9 July 2014 11:29, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I am going on vacations tonight, so I don't think I will have time to<br>
finish this until I get back (20th), but I wanted to send out a<br>
preview of the patch so that others can take a look.<br>
<br>
The fundamental issue with that bug is the api mismatch between gold<br>
and our own LTO api. The gold api talks about "symbol foo in file<br>
bar". Our api talks about "symbol foo in the merged module".<br>
<br>
In that particular testcase, what happens is that gold asks us to keep<br>
a particular linkonce_odr symbol, but the IR linker doesn't copy it to<br>
the merged module and we never have a chance to ask lib/LTO to keep<br>
it.<br></blockquote><div><br></div><div>Why is that not the bug? The linker can't assume that all callers are visible, just that it's merging two modules. A flag for the linker that tells it that it can see the whole program and therefore may drop linkonce symbols sounds like a nice optimization, but not correct in general.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The fix, I think, is to have a more direct implementation of the gold<br>
api. If it asks us to keep a symbol, we change the linkage so it is<br>
not linkonce. If it says we can drop a symbol, we do so. All of this<br>
before we even send the module to lib/Linker.<br>
<br>
Another advantage is that we can drop some linkonce_odr symbols from<br>
the symbol table without having to read the entire module upfront,<br>
only after symbol resolution is done.<br>
<br>
The attached patched is quiet a hack. The final intention is to<br>
refactor lib/LTO to expose the API we need, not to copy and paste code<br>
as the patch does :-)<br>
<br>
The patch is also missing some option handling (setting codegen<br>
attributes for example) and lto logic (don't internalize memcpy for<br>
example).<br>
<br>
Some interesting properties:<br>
<br>
* During symmbol resolution we use a temporary LLVMContext and do lazy<br>
module loading. This allows us to keep the minimum possible amount of<br>
allocated memory around. This should also allow as much parallelism as<br>
we want, since there is no shared context.<br>
<br>
* During codegen, we can use a plain Module and trim it before<br>
linking. This will need some small changes to handle non-elf mangling,<br>
but I don't expect it to be too hard.<br>
<br>
* Since we know what to do with each global, it should be able to make<br>
the reading parallel, but in here llvmcontext synchronization would be<br>
an issue.<br>
<br>
A similar organization is hopefully possible in lld.<br>
<br>
I just coded enough for it to bootstrap with lto enabled. Next items on my list:<br>
<br>
* Vacations :-)<br>
* Feature parity and LTO refactoring to avoid code duplication. With<br>
this it should already be an strict improvement.<br>
* Add support for COMDATs now that we have them in the IR. This should<br>
allow us to use gcc style comdats for c++ constructors and<br>
destructors.<br>
* try to support bfd ld with COFF, as a testcase where mangling in relevant.<br>
* Try to improve how we represent lazy function bodies to make this<br>
more efficient.<br>
* Figure out a better interface with lib/Linker, since right now it is<br>
"fighting" with the plugin to see who is in charge of symbol<br>
resolution.<br>
* Implement lazy loading of metadata.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div>