[PATCH] D21080: [ThinLTO/gold] Enable summary-based internalization

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:534
@@ +533,3 @@
+        // recorded in the index using the original name.
+        const auto &GS = DefinedGlobals.find(GlobalValue::getGUID(OrigName));
+        assert(GS != DefinedGlobals.end());
----------------
tejohnson wrote:
> mehdi_amini wrote:
> > Note: we plan to change the alias representation to https://llvm.org/bugs/show_bug.cgi?id=27866 ; which I believe will make this issue obsolete. It may even remove the need for the aliases representation in the summary (not sure what are the implications about comdats).
> > 
> > Also, this seems like a bug fix that could be exposed with llvm-lto, and thus be split out of this patch?
> Ok, I can add a FIXME to reevaluate this after that bug is fixed.
> 
> Let me see if I can provoke this with llvm-lto and split it out.
Can't reproduce with llvm-lto. I think it is specific to the way the gold plugin handles setting up the list of symbols to link in (it doesn't link in the preempted weak symbol).


http://reviews.llvm.org/D21080