<div dir="ltr">This is a typical error:<div><a href="http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/35542/testReport/junit/LLVM/ThinLTO_X86/funcimport_tbaa_ll/">http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/35542/testReport/junit/LLVM/ThinLTO_X86/funcimport_tbaa_ll/</a><br></div><div><br></div><div>"<pre style="box-sizing:border-box;white-space:pre-wrap;word-wrap:break-word;margin-top:0px;margin-bottom:0px;color:rgb(51,51,51);font-size:13px">llvm-lto: error loading file '/Users/buildslave/jenkins/sharedspace/phase1@2/clang-build/test/ThinLTO/X86/Output/funcimport-tbaa.ll.tmp.bc_0': No such file or directory<br></pre>"</div><div><br></div><div>Note the "_0" suffix. I think what happens is that the suffixed module identifier is used as filename when writing out the thinlto index, and in a subsequent step that file can (of course) not be found.</div><div><br></div><div>Other lit errors have to do with symbols not being found correctly across modules, leading e.g. to wrong internalization/optimizations after importing.</div><div><br></div><div>List of failures: <a href="http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/35542/testReport/junit/LLVM/">http://green.lab.llvm.org/green/job/clang-stage1-configure-RA_check/35542/testReport/junit/LLVM/</a></div><div><br></div><div>regards,</div><div>  Johan</div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 19, 2017 at 1:43 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">It is expected and not unusual to need to update the lit test in such case. I'd need to see exactly which test breaks and how to know though.<div><br></div><div>Best,<span class="HOEnZb"><font color="#888888"><br><div><br></div><div>-- </div><div>Mehdi</div></font></span><div><div class="h5"><div><div class="gmail_extra"><br><div class="gmail_quote">2017-09-18 13:17 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">The fix (<a href="https://reviews.llvm.org/D37961" style="font-size:12.8px" target="_blank">https://reviews.llvm.org/D379<wbr>61</a>) does not work.  From what I have learned thusfar, the module identifier is used as filename sometimes (I think when writing an intermediate module index summary), and so a bunch of lit tests fail with the "fix".<div><br></div><div>I'll look further into fixing this, any help is appreciated.<br></div><div><br></div><div>( One thing that may be important is to have a deterministic suffix. The counter solution of <a href="https://reviews.llvm.org/D37961" style="font-size:12.8px" target="_blank">https://reviews.llvm.org/D3<wbr>7961</a> would name the same module differently depending on order, which may happen when doing thinlto in several different steps. Adding the size of the module (or its offset) would be the same in each invocation. Regardless, changing the suffix to a size suffix does not fix the lit test problems. )</div><div><br></div><div>regards,</div><div>  Johan</div><div><br></div><div><br></div></div><div class="m_-6609349195101040447HOEnZb"><div class="m_-6609349195101040447h5"><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Sep 17, 2017 at 6:32 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">I've created a review for your patch Mehdi: <a href="https://reviews.llvm.org/D37961" target="_blank">https://reviews.llvm.or<wbr>g/D37961</a><div><br></div><div>First time using `arc`, so hope things went well.</div><span class="m_-6609349195101040447m_-511235697632853010HOEnZb"><font color="#888888"><div><br></div><div>- Johan</div><div><br></div></font></span></div><div class="m_-6609349195101040447m_-511235697632853010HOEnZb"><div class="m_-6609349195101040447m_-511235697632853010h5"><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>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_-6609349195101040447m_-511235697632853010m_-4826935965825266947m_4583211550549892208m_2430739262194129951HOEnZb"><font color="#888888"><br></font></span></div><span class="m_-6609349195101040447m_-511235697632853010m_-4826935965825266947m_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="m_-6609349195101040447m_-511235697632853010m_-4826935965825266947HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div></font></span></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div></div></div></div>
</blockquote></div><br></div>