<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=koi8-r">
<style type="text/css" style="display:none"><!-- p { margin-top: 0px; margin-bottom: 0px; } .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: 2px solid rgb(128, 0, 0); }--></style>
</head>
<body dir="ltr" style="font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;">
<p>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);">347033. I'll create a follow-up path with assembly representation of ReadOnly
attribute next week, if this one goes well.</span><br>
</p>
<div style="color: rgb(33, 33, 33);">
<div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>От:</b> Teresa Johnson via Phabricator <reviews@reviews.llvm.org><br>
<b>Отправлено:</b> 16 ноября 2018 г. 3:28<br>
<b>Кому:</b> Evgeny Leviant; tejohnson@google.com; joker.eph@gmail.com<br>
<b>Копия:</b> trent.xin.tong@gmail.com; arphaman@gmail.com; dany.grumberg@gmail.com; jfbastien@apple.com; anton@korobeynikov.info; llvm@inglorion.net; eraman@google.com; stevenwu@apple.com; dexonsmith@apple.com; llvm-commits@lists.llvm.org; George Rimar; Igor
Kudrin; jun.l@samsung.com<br>
<b>Тема:</b> [PATCH] D49362: [ThinLTO] Internalize read only globals</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<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>
<br>
tejohnson added a comment.<br>
<br>
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>
<br>
<br>
<br>
================<br>
Comment at: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp:1048<br>
+ continue;<br>
+ if (auto *GVar = dyn_cast<GlobalVariable>(&GV))<br>
+ if (GVar->hasAttribute("thinlto-internalize")) {<br>
----------------<br>
This should always be true - globals() returns only GlobalVariables (you shouldn't even need to cast).<br>
<br>
<br>
================<br>
Comment at: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp:225<br>
+ // propagateConstants hasn't been run (may be because we're using<br>
+ // distriuted import. We can't internalize GV in such case.<br>
+ if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {<br>
----------------<br>
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>
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D49362">https://reviews.llvm.org/D49362</a><br>
<br>
<br>
<br>
</div>
</span></font></div>
</body>
</html>