[PATCH] D14387: Move comdat setup after global value body linking in ModuleLinker

[Duncan P. N. Exon Smith via llvm-commits]

This seems reasonable to me, but we don't have comdats in Mach-O (I know they're similar to linkonce/etc., but don't really understand the full semantics) so I'm not sure I'm the best person to review this.

Rafael, maybe you could have a look?

> On 2015-Nov-05, at 10:03, Teresa Johnson <tejohnson at google.com> wrote:
> 
> tejohnson created this revision.
> tejohnson added reviewers: dexonsmith, joker.eph.
> tejohnson added a subscriber: llvm-commits.
> 
> The verifier complains if a declaration is in a comdat. However,
> the module linker speculatively adds linked global values that are
> in comdats in the source module into comdats in the destination module,
> which assumes their bodies will also be linked.
> 
> For ThinLTO we may not link the body if it isn't an imported function.
> Also, with currently reverted patch r251926 (D14195: Move metadata
> linking after lazy global materialization/linking), during an LTO build
> a function referenced only from metadata will not have its definition
> linked and will become a declaration. If it was in a comdat it will also
> hit the verifier error.
> 
> To handle this, delay setting up comdats on the Dest module until after
> all global value bodies have been linked, so that we know which are
> definitions vs declarations in Dest.
> 
> This change is an enabler for resubmitting r251926, and also for some
> follow-on fixes for ThinLTO comdat handling I discovered in the process
> of working on these changes.
> 
> http://reviews.llvm.org/D14387
> 
> Files:
>  lib/Linker/LinkModules.cpp
>  test/Linker/funcimport.ll
> 
> <D14387.39379.patch>