[PATCH] D20290: [ThinLTO] Change ODR resolution and internalization to be index-based

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Tue May 24 08:58:45 PDT 2016


tejohnson added inline comments.

================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:339
@@ +338,3 @@
+        S->setLinkage(GlobalValue::InternalLinkage);
+    }
+  }
----------------
tejohnson wrote:
> joker.eph wrote:
> > Can we internalize other than ExternalLinkage?
> Good question. Looking at the rest of the linkage types they are either available externally (skipped by internalizeModule), various types of weak (including linkonce), and other special types such as common and appending.
> 
> I'm not sure about the weak/linkonce types - I don't see anything in internalizeModule or elsewhere in HEAD that prevents internalizing these symbols, but I would have thought that internalizing them could be problematic since they have weak linkage and some can be overridden by external definitions (isInterposableLinkage). I assume that with ld64 any that need to be preserved because they are overridden are added to the preserved set? In gold we know which one is prevailing (which is used to guide current internalization decisions in gold-plugin) and can use that info to guide the internalization via the callback when I call this from there.
> 
> I removed the "if (GlobalValue::isExternalLinkage(S->linkage()))" guard and the regression tests run fine - do you want to try your bootstrap with that change?
Actually, changed this to "if (!GlobalValue::isLocalLinkage(S->linkage()))" so we don't change private -> internal.


http://reviews.llvm.org/D20290





More information about the llvm-commits mailing list