[PATCH] D49362: [ThinLTO] Internalize read only globals

[Evgeny Leviant via llvm-commits]

See r347033. I'll create a follow-up path with assembly representation of ReadOnly attribute next week, if this one goes well.

________________________________
От: Teresa Johnson via Phabricator <reviews at reviews.llvm.org>
Отправлено: 16 ноября 2018 г. 3:28
Кому: Evgeny Leviant; tejohnson at google.com; joker.eph at gmail.com
Копия: trent.xin.tong at gmail.com; arphaman at gmail.com; dany.grumberg at gmail.com; jfbastien at apple.com; anton at korobeynikov.info; llvm at inglorion.net; eraman at google.com; stevenwu at apple.com; dexonsmith at apple.com; llvm-commits at lists.llvm.org; George Rimar; Igor Kudrin; jun.l at samsung.com
Тема: [PATCH] D49362: [ThinLTO] Internalize read only globals

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.

tejohnson added a comment.

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.



================
Comment at: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp:1048
+      continue;
+    if (auto *GVar = dyn_cast<GlobalVariable>(&GV))
+      if (GVar->hasAttribute("thinlto-internalize")) {
----------------
This should always be true - globals() returns only GlobalVariables (you shouldn't even need to cast).


================
Comment at: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp:225
+  // propagateConstants hasn't been run (may be because we're using
+  // distriuted import. We can't internalize GV in such case.
+  if (!GV.isDeclaration() && VI && ImportIndex.withGlobalValueDeadStripping()) {
----------------
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).


Repository:
  rL LLVM

https://reviews.llvm.org/D49362



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181116/a9a14f11/attachment.html>