[PATCH] D15156: [ThinLTO] Appending linkage fixes

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Linker/LinkModules.cpp:1867
@@ -1865,1 +1866,3 @@
+       GV.hasAvailableExternallyLinkage() ||
+       (GV.hasAppendingLinkage() && isPerformingImport()))) {
     return false;
----------------
joker.eph wrote:
> Found another crash:
> 
> ModuleLinker::run() will start by iterating over all the variables in the module and call linkIfNeeded(GV). Your fix here only kicks in if there is no "DGV" (why?).
> 
> If the destination module already have a global_ctors, DGV won't be null. We may want to fix getLinkedToGlobal() as well.
> 
> All of this logic gets more and more complicate, this is a bit scary.
Right, for appending vars we shouldn't import regardless of DGV status. Fixed this, updated test, and removed the old handling in linkGlobalValueProto which is no longer needed as the check is moved here (consistent with other checks that moved from linkGlobalValueProto to linkIfNeeded in the restructuring earlier this week). I don't think the logic is getting more complicated, it has just moved as needed based on the restructuring.


http://reviews.llvm.org/D15156