[PATCH] D14623: [ThinLTO] Comdat importing fixes and related cleanup

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Linker/LinkModules.cpp:944
@@ +943,3 @@
+  // ValueMap for SGV.
+  if (DGV && NewGV != DGV) {
+    DGV->replaceAllUsesWith(ConstantExpr::getBitCast(NewGV, DGV->getType()));
----------------
davidxl wrote:
> A more general question -- why can't the attributes etc from Src GV be merged into Dst GV instead of creating a clone?
Probably more complicated than necessary when we create a NewGV that has a blend of attributes from the dest and source GV copies - we need to keep the dest copy around long enough to create the NewGV anyway. Note this is how the existing handling in linkGlobalValueProto works.

================
Comment at: lib/Linker/LinkModules.cpp:1704
@@ +1703,3 @@
+           "Found remaining member of stripped comdat");
+  for (Function &DF : *DstM)
+    assert(!StrippedComdats.count(DF.getComdat()) &&
----------------
davidxl wrote:
> This brings up the same question why the clone is needed in the first place..
See above answer. Also, because we import the declaration (prototype) before we can determine whether the definition needs to be linked in, it is much simpler to do this as a post-pass.


http://reviews.llvm.org/D14623