<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Nov 16, 2018, at 2:38 AM, Evgeny Leviant <<a href="mailto:eleviant@accesssoftek.com" class="">eleviant@accesssoftek.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="margin-top: 0px; margin-bottom: 0px; caret-color: rgb(0, 0, 0); font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 16px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration: none;" class="">See r<span style="color: rgb(33, 33, 33); font-family: "Segoe UI", "Segoe WP", "Segoe UI WPC", Tahoma, Arial, sans-serif; font-size: 13.3333px; background-color: rgb(255, 255, 255);" class="">347033. I'll create a follow-up path with assembly representation of ReadOnly attribute next week, if this one goes well.</span><br class=""></div></div></blockquote><div><br class=""></div>Thanks. I ran some simple examples through the new version and it indeed fix the issue. I will keep an eye on our CI as well.</div><div><br class=""></div><div>Steven</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 16px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration: none; color: rgb(33, 33, 33);" class=""><div class=""><hr tabindex="-1" style="display: inline-block; width: 1010.375px;" class=""><div id="x_divRplyFwdMsg" dir="ltr" class=""><font face="Calibri, sans-serif" style="font-size: 11pt;" class=""><b class="">От:</b><span class="Apple-converted-space"> </span>Teresa Johnson via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>><br class=""><b class="">Отправлено:</b><span class="Apple-converted-space"> </span>16 ноября 2018 г. 3:28<br class=""><b class="">Кому:</b><span class="Apple-converted-space"> </span>Evgeny Leviant;<span class="Apple-converted-space"> </span><a href="mailto:tejohnson@google.com" class="">tejohnson@google.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:joker.eph@gmail.com" class="">joker.eph@gmail.com</a><br class=""><b class="">Копия:</b><span class="Apple-converted-space"> </span><a href="mailto:trent.xin.tong@gmail.com" class="">trent.xin.tong@gmail.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:arphaman@gmail.com" class="">arphaman@gmail.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:dany.grumberg@gmail.com" class="">dany.grumberg@gmail.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:jfbastien@apple.com" class="">jfbastien@apple.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:anton@korobeynikov.info" class="">anton@korobeynikov.info</a>;<span class="Apple-converted-space"> </span><a href="mailto:llvm@inglorion.net" class="">llvm@inglorion.net</a>;<span class="Apple-converted-space"> </span><a href="mailto:eraman@google.com" class="">eraman@google.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:stevenwu@apple.com" class="">stevenwu@apple.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>;<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>; George Rimar; Igor Kudrin;<span class="Apple-converted-space"> </span><a href="mailto:jun.l@samsung.com" class="">jun.l@samsung.com</a><br class=""><b class="">Тема:</b><span class="Apple-converted-space"> </span>[PATCH] D49362: [ThinLTO] Internalize read only globals</font><div class=""> </div></div></div><font size="2" class=""><span style="font-size: 10pt;" class=""><div class="PlainText">CAUTION: 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.<br class=""><br class="">tejohnson added a comment.<br class=""><br class="">I was trying this out internally and noticed a couple of minor things that can be fixed when you recommit the patch with the caching fix.<br class=""><br class=""><br class=""><br class="">================<br class="">Comment at: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp:1048<br class="">+ continue;<br class="">+ if (auto *GVar = dyn_cast<GlobalVariable>(&GV))<br class="">+ if (GVar->hasAttribute("thinlto-internalize")) {<br class="">----------------<br class="">This should always be true - globals() returns only GlobalVariables (you shouldn't even need to cast).<br class=""><br class=""><br class="">================<br class="">Comment at: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp:225<br class="">+ // propagateConstants hasn't been run (may be because we're using<br class="">+ // distriuted import. We can't internalize GV in such case.<br class="">+ if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {<br class="">----------------<br class="">As we discussed earlier in the review thread, there should not be any issue with doing this for a distributed import (I just checked a small test case and confirmed it works fine). Please update the comment (it was only in certain testing contexts that you wouldn't have dead stripping at this point).<br class=""><br class=""><br class="">Repository:<br class=""> rL LLVM<br class=""><br class=""><a href="https://reviews.llvm.org/D49362" class="">https://reviews.llvm.org/D49362</a><br class=""><br class=""><br class=""><br class=""></div></span></font></div></div></blockquote></div><br class=""></body></html>