[PATCH] D18415: [LTO] Basic support for internalize.

Rafael Ávila de Espíndola via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 24 12:18:34 PDT 2016


rafael added a comment.

This looks a lot better.

Testcase? :-)


================
Comment at: ELF/LTO.cpp:18
@@ -17,2 +17,3 @@
 #include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/LegacyPassManager.h"
----------------
Dead include?

================
Comment at: ELF/LTO.cpp:82
@@ +81,3 @@
+  Module &M = Obj->getModule();
+  collectUsedGlobalVariables(M, Used, /* CompilerUsed */ false);
+
----------------
This can also go in a future patch, no?

================
Comment at: ELF/LTO.cpp:113
@@ +112,3 @@
+          // symbols when creating a shared library.
+          if (!Config->Shared && !Used.count(GV) && !B->isUsedInRegularObj())
+            InternalizedSyms.insert(GV->getName());
----------------
ExportDynamic should have the same treatment as shared. Not sure if we already have a convenient predicate for either.

================
Comment at: ELF/LTO.cpp:126
@@ +125,3 @@
+static void internalize(GlobalValue &GV) {
+  if (!GV.hasLocalLinkage())
+    GV.setLinkage(GlobalValue::InternalLinkage);
----------------
You only add symbols that are not internal, I think you can assert this.

================
Comment at: ELF/LTO.cpp:136
@@ +135,3 @@
+    GlobalValue *GV = Combined.getNamedValue(Name.first());
+    if (GV)
+      internalize(*GV);
----------------
You can assert GV for now I think.

================
Comment at: ELF/SymbolTable.cpp:221
@@ +220,3 @@
+  if (auto *BC = dyn_cast<DefinedBitcode>(Existing))
+    if (auto *U = dyn_cast<Undefined>(New))
+      BC->setUsedInRegularObj();
----------------
I am not sure just Undefined is sufficient.

If a weak symbol in an object file is replaced with a strong symbol in a .bc file, there can be a reference from the strong symbol to the bit code one.



http://reviews.llvm.org/D18415





More information about the llvm-commits mailing list