[PATCH] D31963: ThinLTOBitcodeWriter: keep comdats together, rename if leader is renamed

[Bob Haarman via Phabricator via llvm-commits]

inglorion added inline comments.


================
Comment at: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:319
 
+  auto ShouldLiveInMergedM = [&](const GlobalValue *GV) -> bool {
+    if (auto *F = dyn_cast<Function>(GV))
----------------
pcc wrote:
> I don't think you need to extract this lambda. The appropriate test is just whether the GlobalValue is a GlobalVariable with type metadata. The canonical definitions of any virtual functions identified by this lambda live in the thin LTO module, and the copies in the merged module are used for optimization only and are not comdat members (see line 347 below). So your code for identifying comdats could be added to the loop above.
Yes. The way I wrote this actually introduces a bug where we can end up with functions available_externally in MergedM, and no definition in M. I'm working on a fix.


https://reviews.llvm.org/D31963