<div dir="ltr">I've created a review for your patch Mehdi: <a href="https://reviews.llvm.org/D37961">https://reviews.llvm.org/D37961</a><div><br></div><div>First time using `arc`, so hope things went well.</div><div><br></div><div>- Johan</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 12, 2017 at 5:25 AM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Johan,<div class="gmail_extra"><br><div class="gmail_quote"><span class="">2017-09-11 14:21 GMT-07:00 Johan Engelen <span dir="ltr"><<a href="mailto:jbc.engelen@gmail.com" target="_blank">jbc.engelen@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Sep 8, 2017 at 9:04 PM, Johan Engelen <span dir="ltr"><<a href="mailto:jbc.engelen@gmail.com" target="_blank">jbc.engelen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Sep 7, 2017 at 5:44 PM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi Johan,<div><br></div><div>ld64 only calls functions from llvm/include/llvm-c/lto.h  (defined in llvm/tools/lto/lto.cpp)</div><div><br></div><div>For instance ThinLTOCodeGenerator::addModul<wbr>e is called through thinlto_codegen_add_mo<wbr>dule().<br></div><div><br></div><div>Apple hasn't released the code for ld64 in Xcode 9 yet, did you check if it is fixed in Xcode 9?</div><div>(I think I remember fixing it in ld64 but I'm not totally sure...).</div></div></blockquote><div><br></div></span><div>I haven't tried with Xcode 9.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>From what I can see with Xcode 8.2, the linker just passes the file name (prefixed with the archive name): <a href="https://github.com/michaelweiser/ld64/blob/master/src/ld/parsers/lto_file.cpp#L546" target="_blank">https://github.com/michaelweis<wbr>er/ld64/blob/master/src/ld/par<wbr>sers/lto_file.cpp#L546</a></div><div>(original here: <a href="https://opensource.apple.com/source/ld64/ld64-274.2/src/ld/parsers/lto_file.cpp.auto.html" target="_blank">https://opensource.apple<wbr>.com/source/ld64/ld64-274.2/sr<wbr>c/ld/parsers/lto_file.cpp.auto<wbr>.html</a> )</div><div><br></div><div>We could workaround this in ThinLTOCodeGenerator by adding a incremental suffix to every new buffer. Something like this diff:</div></div></blockquote><div><br></div></span><div><span style="font-size:12.8px">I was assuming that we do want to assert/error on calling addModule with the exact same module twice? Otherwise your diff is nice, thanks.</span><span class="m_4583211550549892208m_2430739262194129951HOEnZb"><font color="#888888"><br></font></span></div><span class="m_4583211550549892208m_2430739262194129951HOEnZb"><font color="#888888"><div> </div></font></span></div></div></div></blockquote><div><br></div></span><div>Hi Mehdi,</div><div>  Can you advise me?</div><div>Is it OK to not error upon the exact same module being added twice? (and thus your patch would be good)</div><div><br></div></div></div></div></blockquote><div><br></div></span><div>It is a matter of API contract, so I'm not sure I perceive a clear right/wrong here. If we want to enforce an API contract on the caller's responsibility to provide a unique ID, then the assert may be OK (however we disable assertions in production and it means random crashes which isn't super user-friendly).</div><div>On the other hand my patch here was trying to be resilient to API misuse for "little cost".</div><div><br></div><div>Note that the plan was that this API (and the ThinLTOCodegenerator.cpp) should just die, but unfortunately I haven't had time to work on the replacement (a C binding for the new LTO API).</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div></font></span></div></div></div>
</blockquote></div><br></div>