<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 15, 2018 at 6:35 AM Teresa Johnson <<a href="mailto:tejohnson@google.com">tejohnson@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 15, 2018 at 6:22 AM Evgeny Leviant <<a href="mailto:eleviant@accesssoftek.com" target="_blank">eleviant@accesssoftek.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr" style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif">
<p>Thanks for making reproducer, Steven!<br>
</p>
<p><br>
</p>
<p>It turned out cache key is computed differently by newer API (LTO.cpp) and legacy API (ThinLTOCodeGenerator).<br></p></div></blockquote><div>Arg - such a simple reason! I didn't remember that they were using separate code. Looking at what appears to be the cache key computation for the old API (ModuleCacheEntry constructor), I don't see it hashing other things we compute during the thin link beyond import/exports/resolved linkage types. I.e. I don't see it hashing the isLive bit. So I'm surprised we haven't hit an issue here before. Wonder if this code and computeCacheKey can be refactored to do the same computation for most of the info?? Maybe as a follow on patch. But I'm really curious how things are working given e.g. the missing hash of the live bit etc.</div></div></div></blockquote><div><br></div><div>Answering my own question: The isLive bit is only used downstream of the thin link in the CFI/WPD case, which aren't supported via the old LTO API. Ditto for the DSOLocal bit, which is only set in the new LTO API.</div><div><br></div><div>Anyway, I think it would be good to refactor and call common code for the cache key computation, but this can be done as a follow on. Steven - is this something you would have time to do? I can do it, but can't test with ld64. If you want I can create a patch and you can test it there.</div><div><br></div><div>Teresa</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif"><p>
</p>
<p>I was always wondering what piece of software uses legacy API and finally found this out - it is Mac OS X linker ld64.<br></p></div></blockquote><div><br></div><div>Yep, along with at least one proprietary linker that I am aware of. </div><div><br></div><div>Teresa</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" style="font-size:12pt;color:#000000;background-color:#ffffff;font-family:Calibri,Arial,Helvetica,sans-serif"><p>
</p>
<p><br>
</p>
<p>On Linux lld and gold plugin use newer API, so the bug can't be reproduced.<br>
</p>
<div style="word-wrap:break-word;line-break:after-white-space">
<hr style="display:inline-block;width:98%">
<div id="m_-7194465250107477513m_3865917635723563813divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>От:</b> <a href="mailto:stevenwu@apple.com" target="_blank">stevenwu@apple.com</a> <<a href="mailto:stevenwu@apple.com" target="_blank">stevenwu@apple.com</a>> от имени Steven Wu <<a href="mailto:stevenwu@apple.com" target="_blank">stevenwu@apple.com</a>><br>
<b>Отправлено:</b> 15 ноября 2018 г. 3:34<br>
<b>Кому:</b> Teresa Johnson<br>
<b>Копия:</b> Evgeny Leviant; Mehdi AMINI; <a href="mailto:reviews%2BD49362%2Bpublic%2B6f0316d7b1456727@reviews.llvm.org" target="_blank">reviews+D49362+public+6f0316d7b1456727@reviews.llvm.org</a>; Xin Tong; Alex L; Daniel Grumberg; JF Bastien; Anton Korobeynikov; Bob Haarman; Easwaran Raman; Duncan Exon Smith; llvm-commits; George Rimar; Igor Kudrin; <a href="mailto:jun.l@samsung.com" target="_blank">jun.l@samsung.com</a><br>
<b>Тема:</b> Re: [PATCH] D49362: [ThinLTO] Internalize read only globals</font>
<div> </div>
</div>
<div>
<div style="font-size:9pt;font-family:'Calibri',sans-serif">
<h3 style="background-color:#ffffff;font-size:10pt;border:1px dotted #003333;padding:.8em">
<span style="color:#ff6600">CAUTION:<strong> </strong></span>This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.</h3>
</div>
<div><br>
<div><br>
<blockquote type="cite">
<div>On Nov 14, 2018, at 4:28 PM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> wrote:</div>
<br class="m_-7194465250107477513m_3865917635723563813Apple-interchange-newline">
<div>
<div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none">
<div dir="ltr"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Nov 14, 2018 at 1:47 PM Steven Wu <<a href="mailto:stevenwu@apple.com" target="_blank">stevenwu@apple.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<div>Here is a small reduced test case using the same main.c and foo.c from the review:
<div>cat main.c</div>
<div>
<div></div>
<blockquote type="cite">
<div>int foo();</div>
<div>int main() {</div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>return foo();</div>
<div>}</div>
</blockquote>
<div><br>
</div>
<div>
<div>cat foo.c</div>
<div></div>
</div>
<blockquote type="cite">
<div>
<div>#include <stdlib.h></div>
<div>static int gFoo = 1;</div>
<div>int foo() {</div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>return gFoo;</div>
<div>}</div>
<div>void bar() {</div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>gFoo = rand();</div>
<div>}</div>
</div>
</blockquote>
<div>
<div><br>
</div>
<div>cat test1.c</div>
<div>
<div>
<div></div>
</div>
<blockquote type="cite">
<div>
<div>int foo();</div>
</div>
<div>int test() {</div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>return foo();</div>
<div>}</div>
</blockquote>
<div>
<div><br>
</div>
<div>cat test2.c</div>
<div>
<div></div>
</div>
</div>
</div>
<blockquote type="cite">
<div>
<div>
<div>
<div>int foo();</div>
<div>int bar();</div>
</div>
<div>int test() {</div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>return foo() + bar();</div>
<div>}</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>To reproduce:</div>
<div>$ clang -O3 -flto=thin -c main.c test1.c test2.c foo.c</div>
<div>$ clang -O3 -flto=thin -Wl,-cache_path_lto,lto.cache main.o test2.o foo.o</div>
<div>$ clang -O3 -flto=thin -Wl,-cache_path_lto,lto.cache main.o test1.o foo.o</div>
<div>Undefined symbols for architecture x86_64:</div>
<div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>"_gFoo.llvm.17695746433417383459", referenced from:</div>
<div> <span class="m_-7194465250107477513m_3865917635723563813Apple-converted-space"> </span>_main in lto.o</div>
</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>I can't reproduce this, although I am using gold (with -Wl,-plugin-opt,cache-dir=lto.cache) since I don't have ld64. I notice that test() is dead, so with gold at least it is removed completely.</div>
<div><br>
</div>
<div>Do you have any temp files you can share (i.e. IR before and after importing)?</div>
</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Attach all the intermediate file. I haven't had time to look in detail yet.</div>
<div><br>
</div>
<div></div>
</div>
</div>
</div>
</div>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="m_-7194465250107477513gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><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:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"><br></td></tr></tbody></table></span></div></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><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:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"><br></td></tr></tbody></table></span></div></div></div></div>