[PATCH] D16120: [ThinLTO] Remove imported available externally defs from comdats.

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Linker/LinkModules.cpp:733
@@ +732,3 @@
+  // Remove functions imported as available externally defs from comdats,
+  // as this is a declaration for the linker, and will be dropped eventually.
+  auto *GO = dyn_cast_or_null<GlobalObject>(&GV);
----------------
joker.eph wrote:
> I don't understand why we need to this?
Declarations (more generally declarations for the linker, so including available_externally) may not be in a comdat. It is illegal (and checked by Verifier::visitGlobalValue).

================
Comment at: lib/Linker/LinkModules.cpp:737
@@ +736,3 @@
+    // The IRMover should not have placed any imported declarations in
+    // a comdat.
+    assert(GO->hasAvailableExternallyLinkage() &&
----------------
joker.eph wrote:
> This comment looks like it is a bug in the IRMover that you're working around here?
No, it is describing the below assert. I.e. the only declarations for the linker that are in a comdat at this point would be imported definitions, which are available externally. I will try to clarify the comment a bit.


http://reviews.llvm.org/D16120